Fix/performance ux stats - #2
Merged
Merged
Conversation
…e app
The reported bug was that automatic detection freezes the whole app. The
chunking everyone assumed was missing was already there — both scanner stages
loop in batches and yield between them. The freeze was inside a single batch.
saveEnumerated looped over the page doing five awaited round trips per track:
insert artist, select artist, insert album, select album, upsert track. At the
default page size of 500 that is ~2,500 sequential queries with no yield
anywhere in between. The fingerprint skip already in the code only helps a
re-scan, where most rows are unchanged; the first scan — the one a new user
sees — skipped nothing and paid all of it.
Measured on the Pixel_7 AVD, 528-file library, first scan, one page of 500:
before after
longest JS block 859ms 20-31ms
total CPU 859ms 126-150ms
859ms is a frozen screen: no render, no touch handled. 24ms is under two
frames. Verified by switching tabs mid-scan — Stats painted within a second
with the tap ripple still on screen, and the library list renders and scrolls
while "Reading tags..." is still running.
Now: one insert + one select resolves every artist in the page, one select +
one insert every album, and one statement per sixty tracks. Sixty because a
track binds fifteen columns and SQLITE_MAX_VARIABLE_NUMBER is 999 on older
builds — 900 parameters is under the pessimistic ceiling.
resolveAlbums reads before writing rather than leaning on onConflictDoNothing,
because the unique index is on (name, artist_id) and SQLite treats each NULL as
distinct in a unique index. An album with no artist never conflicts with itself,
so a blind upsert would have added a row for it on every scan, forever.
Two things the measurement changed my mind about:
Wall-clock is now the wrong metric and the instrumentation says so. It went
859ms -> 4151ms, because it counts the deliberate yields — the time the UI is
free. What freezes a screen is the longest stretch between two yields, so that
is what is recorded now.
And the yield is setTimeout(0), not requestIdleCallback. An idle callback is the
correct primitive and it was the first choice, but on this emulator each one
took ~700ms to fire even with { timeout: 50 }: a saturated thread never reports
idle and the timeout is not honoured tightly. Nine of them turned a 150ms write
into 6.4 seconds. Yielding on a 50ms budget instead of once per chunk cut the
number of yields by two thirds.
saveEnriched is now one transaction per batch rather than twenty-five implicit
ones — 25 commits to write 25 rows. It measures 17-23ms per batch of 25.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ppens Removes the automatic launch sweep. Nothing now begins a MediaStore enumeration without a user pressing something. The freeze this was blamed for is fixed separately and would have been worth fixing either way. This is the other half, and it is not a performance argument: an unannounced scan is the app deciding on the user's behalf to read every audio file on the device, on launch, without saying so. For an app whose whole proposition is that it does not do things behind your back, that was a strange exception to be making. - A Scan button lives permanently in the Library header, not only in the empty state — someone who copies an album across next month needs to reach it without emptying their library first. - It confirms before starting, and the copy says what will happen: every indexed audio file gets read, a large library takes a while, it can be stopped, and whatever was found is kept. No duration is promised, because MediaStore will not report a count until it has been asked. - The folder picker moves to its own icon and stays independent, as does pull-to-refresh. - The empty state now offers Scan rather than the folder picker: scanning is the answer for most people, and picking a folder is the answer for the ones MediaStore fails. Also fixes the enrich progress bar, which was permanently full. Stage two set total to however many rows it had already done, so the ratio was always 1 and the label read "N / N" throughout. There is a countUnenriched port now, called before the first progress report so no frame ever shows "0 / 0", and total is never revised below processed — a resumed scan starts partway through, and a file that will not open still counts as done. Verified on the Pixel_7 AVD: the library painted 528 tracks with no scan running at all, which is the ADR's claim that an already-scanned library comes from SQLite and needs MediaStore for nothing. The confirm dialog renders with its copy and both buttons. A rescan of an unchanged library now measures 20ms and 1ms for the two pages, written=0 — the incremental path doing its job. Cancellation is wired (banner Stop -> controller -> checked at the top of every batch in both stages) and covered by three unit tests including the one that matters most, that a cancelled sweep retires nothing. I could not exercise the button on device: an unchanged rescan finishes in 20ms, so the banner is gone before a tap can land on it. Decision recorded in docs/adr/010-scanning-is-user-initiated.md, including the cost — new files are no longer discovered on their own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Selection mode froze the list. Measured on the Pixel_7 AVD by counting row
reconciliations: 47 rows rebuilt per checkbox tap, and in non-selection mode
each rebuild also constructed a Gesture.Pan(). After: 1 row per tap.
Four causes, all the same shape — something stable was being rebuilt:
- useSelection returned a fresh object literal every render, so anything
holding it had a dependency that changed on every unrelated parent render.
It is memoized now; its functions were already useCallback([]).
- LibraryScreen's row callbacks listed the whole selection object as a
dependency. They take isActive and toggle instead, which are a boolean and a
stable function.
- TrackList took the selection object and called selection.has(id) per row.
It takes a ReadonlySet built once per change.
- SwipeableRow built its Pan gesture inline on every render. Memoized on
onSwipe, with the shared values deliberately absent from the deps — a
useSharedValue handle never changes identity, and listing one on a hook that
writes to it is what the compiler's immutability rule rejects.
The row tree moved into a memoized LibraryRow that takes only primitives and
stable callbacks. The old inline version passed
onSwipe={() => onSwipeToQueue(item.id)} — a fresh closure per row per render,
which defeated every memo under it.
Entering selection mode still rebuilds all 47 rows, and that is correct: every
row genuinely changes shape when the checkboxes appear. It is the per-tap cost
that mattered.
Worth recording a wrong turn: the first metric was renderItem invocations,
which stayed at exactly 47 before and after. It counts element creation, not
reconciliation — React bails out at the memo boundary afterwards. Only once the
counter moved inside the component body did the fix show up at all.
Also adds toasts, the second half of the same report. A swipe that queues a
track has no visible result — the queue is on another screen — so the gesture
was indistinguishable from a scroll that did nothing. A module-level store
rather than a context, so a toast re-renders the Toaster and nothing else, and
mounted directly above the mini player rather than at the root: positioning it
from the root needed an offset clearing both the transport and the tab bar,
and those are not design-system spacing values. The first attempt used pb-32,
which is not on the scale and therefore compiled to nothing — the toast sat on
top of the tab bar. Stacking removes the number entirely.
Verified on device: toast reads "Added to queue", clears the tab bar and the
mini player, and has a dismiss control.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The report was that the EmptyState icon does not disappear during an import. It does disappear — FlashList drops ListEmptyComponent the moment data is non-empty, and the source says so. The real fault was that it should never have been on screen in the first place. Scanning an empty library put the list's empty state — "No music found yet. Scan the device, or point Mufify at a folder", with a Scan button — directly underneath a banner reporting a scan in progress. Two halves of one screen disagreeing about whether anything was happening, and the half with the button was wrong. On the old slow scan that state was up for a long time, which is what made it look stuck. The list now shows skeleton rows whenever the query is in flight *or* a scan is running with no rows yet. Rows arriving is the only thing that ends it. Generalises the skeleton into the family the States rule wants: <Skeleton /> for one block, <SkeletonRows /> for a list, <SkeletonCards /> for the album and artist shelves. TrackListSkeleton is now a named wrapper so the row geometry stays in one place next to TrackRow. The pulse is opacity in a Reanimated worklet and never touches the JS thread — a loading indicator competing with the work it indicates is worse than none — and it stops dead under reduce-motion, which is exactly the kind of looping animation that setting exists to stop. No new colour token: a skeleton is an empty panel, and bg-surface-elevated already means "something goes here". Also stops the banner printing "0 / 0". Both stages report a total only after counting, and counting is itself a query; until it returns, saying nothing is honest and "0 / 0" reads as a scan that found nothing rather than one that has not looked yet. The label and Stop still appear instantly, so the press is still confirmed immediately. Verified on device: clearing the library and pressing Scan now shows "Finding files..." with Stop above pulsing skeleton rows, and no empty state anywhere. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three reported faults, one rewrite. The mini player gains a previous button. Skipping back is the commonest correction there is — you skip one too many and want it back — and having only "next" made the strip a one-way control. Swiping the strip up now opens Now Playing. It did not before, and the reason was the shared TransportSwipe wrapper: a generic pan over a container made entirely of Pressables, with activeOffsetX and activeOffsetY both set. Two axes of activation on a surface of touch targets meant the pan routinely lost the race to a child's press responder, so the gesture worked sometimes. The replacement activates on vertical movement only and reads horizontal travel as a secondary decision once it already owns the gesture, so it never competes with a tap. Distance or velocity commits, so a short flick opens it as readily as a deliberate drag. TransportSwipe is deleted. The Now Playing artwork is a three-slot carousel. Previous, current and next are all mounted and the strip translates under the finger, so the neighbour is a real decoded image sliding in rather than a blank square filling in after the fact — which is what made the old version, one image with its source swapped on release, look amateur. Release commits on distance or velocity; at the ends of the queue the strip still moves, at a quarter rate, and springs back. Refusing to move reads as a dropped gesture; a rubber band says "nothing here" in the language the gesture is already speaking. Three bugs found by testing on the device, none of which would have shown up in review: The axis lock compared both translations on the first onUpdate, when both are still zero — and Math.abs(0) >= Math.abs(0) is true, so every gesture locked to horizontal and vertical ones were silently discarded. Swipe-down did nothing. It waits for 6px of real movement now. The dismiss thresholds were inherited from the track-change ones and were too strict. A firm downward drag measured translationY 285 and velocityY 762 against limits of 302 and 800: it missed both by a hair and sprang back. Dismiss has its own, far more forgiving numbers, which is right anyway — throwing a screen away is coarse and costs one tap to undo, changing track is precise and interrupts the music. And the one that wasted the most time: the player was being pushed twice. The drag fires openPlayer while the underlying Pressable still registers its press, so router.push stacked two identical screens. The symptom was that swipe-down appeared broken *and* the close button appeared broken, because each was correctly dismissing one of two. router.navigate reuses the route instead. The gestures here are built inline rather than memoized, matching Scrubber and unlike SwipeableRow. SwipeableRow has forty live instances in a list and needs the memo; these have one each, and keeping the shared values out of a hook's closure avoids the React Compiler immutability rule, which is right about ordinary values and simply does not model Reanimated. Verified on the Pixel_7 AVD end to end: swipe up opens, swipe left advances MUSE -> nested-4 with the spec strip following, swipe down returns to the library with playback still running, swipe up reopens at 1:05. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every row now has a line under it explaining the setting, and the shuffle picker is a single column with a sentence per algorithm rather than five squeezed segments. The segmented control was the wrong control, not just a badly sized one. "Discovery" and "Favourites" are names; the entire argument for offering five shuffle algorithms is that a user can tell them apart, and a control with no room for the explanation cannot let them. Wrapping at three per row — the previous fix — made it fit without making it clear. A column has as much room as it needs in any language and at any font scale, which also disposes of the Turkish-is-20%-longer problem permanently rather than by measuring. Selection is a tick, not a filled row: with a description under every option, filling the selected one would put body text on indigo and cost the contrast the tokens guarantee. The shuffle hints are rewritten now that there is room for a real sentence — "An unplayed track comes up twice as often as one you have heard once" says more than "Favours tracks you have played least". Theme and language stay segmented: three short self-evident options each, where a column would be pomp. Adds the Playback and short-file settings, both wired: haptics already drives `src/services/haptics`, and "ignore short files" now actually feeds the scanner's minDurationMs, read at scan time rather than captured at mount because the switch lives on a different screen from the button. Deliberately *not* added: a "resume on launch" switch. The preference exists in the store from an earlier commit, but nothing persists or restores a queue yet, so the control would have changed nothing while claiming to. An absent feature is better than a lying one. It goes in when the queue persistence does. Verified on the Pixel_7 AVD: the shuffle column reads properly, both switches render with their descriptions, and the folder group keeps its existing copy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The suspicion was that only track rollups were being written. That turned out to be wrong: rollupDeltas already fans out to track, artist, album and playlist, and it is tested. Verified on device by reading the database — playing a track that has an artist took artist rollups from 0 rows to 3, one per period, with album and track moving in step. The zero-artist-rollups reading that prompted the check was real and innocent: 526 of the 528 test files have no artist tag at all, so there was no artist to roll up. All 528 have an album, which is why album rows existed and artist rows did not. What *was* broken is upstream of the fan-out. listenRecorder hardcoded sourceType: 'library' for every listen, so: - No listen was ever attributed to a playlist. stats_rollups has an entity type for playlists and could never gain a single row, and the failure is invisible — an empty top-playlists list looks exactly like a user who does not play playlists. - play_events.shuffle_algorithm has existed since Phase 1 with nothing writing to it, so the question it exists to answer — which shuffle produces listens people finish — was unanswerable. docs/shuffle.md claimed it was recorded. Both are attributes of the *queue*, not of a track: the same track played from a playlist and from the library is two different listens. So QueueSource lives on the engine, set by setQueue and defaulted to the library, and rides out with each finished listen alongside the active shuffle algorithm. PlaylistDetail passes its own source from all three of its entry points. Verified on device, reading play_events directly: event 456 has shuffle_algorithm NULL, then shuffle is switched on and 457-459 read "balanced". Exactly the right boundary. One thing worth writing down for the next person: reading this database over adb needs the -wal file too. WAL mode means recent writes are not in the main file at all, and copying only mufify.db shows an empty play_events table and a stale rollup count — which looks precisely like the bug you are hunting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A layer on top of ADR 005, which is untouched — min(30s, duration*0.5) is still what makes a listen count. This decides where one listen ends and the next begins while the track never changes, which was never in doubt before playback existed: the engine closed a listen only when the loaded track changed. Dragging the scrubber back to the start and listening again produced no second event. The counts were not wrong about what qualified, only about how many times it happened. The rule, checked each status tick against the previous one: 1. the current listen has already earned a play, and 2. the position jumped back to at or below 25% of the track. The first condition is what stops seeking from shredding history. Without it, scrubbing inside the first thirty seconds splits one listen into fragments too short to count as anything, turning a real play into a pile of skips. With it, a rewind can only ever add a listen. 25% has to separate "start it again" from "go back a bit" using the only signal available, which is how far back the position went. Tighter counts a scrub over the final chorus as a replay; looser misses a restart on a nearly-finished track. The boundary deliberately does not require the *new* listen to pass the threshold too. It fires on the rewind and the new listen is classified by the ordinary rule when it ends, so rewind-then-leave records a play for what was heard and a skip for what was abandoned — both true — instead of merging the fragment into the completed play. Worth recording, because it changes what this commit actually adds: repeat-one was already producing one event per loop before this. didJustFinish fires at the end of a track whether or not the queue advances, and the engine already flushed the listen there. The genuinely new case is the manual seek backwards. Decided in docs/adr/011-repeat-listen-detection.md, referencing ADR 005 without reopening it, with a section in docs/stats.md that replaces a stale "not yet implemented" note about exactly this. Verification is unit tests rather than the device, and deliberately so. Ten tests pin the individual decision at every threshold boundary, and five more walk realistic runs of status ticks and count how many listens a session produces — three loops give three, a drag to the start gives one, hunting around in the first few seconds gives none, replaying the last chorus four times gives none. That is the behaviour the feature was asked for, and it is deterministic; the emulator stopped accepting input partway through this work and a six-second file looping on a starved AVD is a worse oracle than the sequence itself. One thing the sequence tests caught that a device never would have: the first version of the harness advanced accumulated playback by a fixed amount per tick regardless of how far the position moved, which made every position jump free and produced a failure that looked exactly like a bug in the rule. It credits forward movement only now, because seeking is not listening. Still owed: on-device confirmation of the seek-back path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…pped card stats_rollups has carried ms_played since Phase 1 and the screen never showed it. Every ranked row now reports both numbers, because they disagree constantly — a three-minute song played twice beats a forty-minute mix on one measure and loses badly on the other, and showing only the count was hiding half of what the table already knew. Adds top albums and top playlists. The rollup fan-out has always written all four entity types; only two were ever read. Playlists stay empty until something is played from one, which the previous commit made possible. Artwork resolves through correlated subqueries. albums.artwork_path exists in the schema and the scanner never fills it — artwork is extracted per file, so the cover lives on the tracks — and a join would multiply the rollup row by every track on the record. Bounded by the outer limit, so it runs ten times rather than once per album in the library. The Wrapped card leads the screen. What makes a summary worth screenshotting is a single sentence someone would repeat, so it leads with the listening time in the display face at a size nothing else uses and follows with two facts. Deliberately not a gradient, a collage, or a share sheet: the design direction rules out the first two by name, and the third needs an outward-facing intent in an app whose promise is that nothing leaves the device. A screenshot is already the share mechanism and needs no permission. Also fixes formatListeningTime below a minute, which the new per-row totals exposed: a handful of six-second tracks all read "0m", which tells the reader nothing and looks like a value that failed to load. It reports seconds under a minute and stops there — "4h 37m 12s" is not how anyone reports listening time, and once there are minutes the seconds are noise. Verified on the Pixel_7 AVD: Wrapped reads "This week / 48m / 461 plays across 459 tracks" with most-played and most-heard beneath it; rows show "1 play / 6s" where they used to show "0m"; Top artists resolves Muse's cover from its tracks; Top albums is populated and Top playlists correctly renders nothing at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three segments now: Tracks, Artists, Albums. The grids are two-column square cards; tapping one opens a detail screen with its tracks and Play/Shuffle. No schema change — artists and albums have been populated since Phase 2 and nobody was reading them. The detail screen reuses LibraryTracks wholesale rather than growing a second, thinner list. A track on an album screen gets exactly the same verbs as one in the library — swipe to queue, long-press sheet, multi-select — and there is one place to change them. Playing from a shelf attributes the listen to that artist or album, which is what fills those entity types in stats_rollups. No genre shelf, though the tech stack doc lists one. Genre comes from MediaStore and on real libraries it is close to useless: many files carry none, sources disagree on spelling and case, and most of a collection lands in one bucket called Unknown. A shelf whose biggest card is "Unknown" and whose next four are spellings of the same word is a way to discover your tags are a mess, not a way to find music. It needs normalisation to be worth having, and that is a feature with a design rather than a fourth useLiveQuery. Recorded in ADR 012. LibraryScreen was at exactly the 300-line limit, so it was split before anything was added: it owns the library — scanning, searching, which view is showing — and LibraryTracks owns tracks. Forced by a line count, right boundary anyway. 184 and 221 lines now. Then the part that matters more than the feature. The detail header's cover did not appear. The path was in the database and the same query fed the grid, where the cover rendered fine. The class was `h-32 w-32`, and 128px is not on the spacing scale — which tailwind.config.js overrides rather than extends, so the class produced no CSS at all and the image drew at zero by zero. No warning, no error. Grepping for the pattern found four more that had already shipped invisible: the playlist mosaic at size="lg", `w-24` on the swipe-to-queue reveal strip — which is why that action's icon had never once been seen — `h-7 w-7` on the track picker's checkbox, and `max-h-96` on two sheets. AGENTS.md names this exact trap and it caught us anyway, because the failure mode is silence. src/theme/scale.test.ts now walks every source file and fails on any spacing class outside the scale. It caught its first offender immediately: my own comment explaining the bug, which still contained the dead class. Verified on the Pixel_7 AVD: Albums grid renders with real covers and track counts, the search field correctly disappears on the card views, and the album detail header shows its artwork. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Section 1a of the brief asked for a diagnosis before a fix, with numbers. Most of the checklist turned out to be already correct, and saying so with evidence is worth as much as a change would have been: - No JS-thread animation anywhere. Every animation is a Reanimated worklet; there is no `Animated` from react-native, no LayoutAnimation, and no setInterval or rAF driving a visual. - List rows are memoized with stable callbacks — 47 reconciliations per checkbox tap before, 1 after. - Opening a modal reconciles 1 of 47 visible rows, not all of them. No chain. - There is no React context in the app at all. Shared state is module-level stores read through useSyncExternalStore, so a change notifies only its own subscribers. - All nine expo-image call sites pass cachePolicy and recyclingKey. The one real defect the audit turned up was the spacing-scale bug, already fixed and now guarded by a test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both gates green — lint, typecheck, 292 JS tests across 20 suites, and the Kotlin unit tests forced with --rerun-tasks. Verified on device: theme both ways, language both ways with every string translated, all four tabs clean with no JS error in logcat, scanning, playback, shuffle persistence, and the statistics screen. One honest gap recorded rather than glossed: the playlist create -> add -> play chain was not exercised in a single pass. Its pieces were each verified earlier, but adb input became unreliable on an emulator nine hours into the session. That is missing evidence, not a known fault, and it is first on the list for a fresh device. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The repository had no README, no CONTRIBUTING, and no LICENSE despite package.json declaring MIT. docs/components.md was referenced by AGENTS.md and had never been written. README covers what the app is, the offline promise and how it is enforced rather than merely intended, the toolchain requirements, setup, the commands, the MIUI install and input restrictions, the architecture rules, and an index of every doc. CONTRIBUTING is the process around AGENTS.md rather than a summary of it: the four-part gate, what a commit message is for, what a performance or UI claim has to come with, and the list of things that will fail review — each of which now has a test behind it. docs/components.md maps all 56 components. Where one has a constraint that is not obvious from reading it — SwipeableRow being transient because it lives in a recycling list, MiniPlayer subscribing to phase and track but never position, TrackList's drawDistance — that reason is repeated there, because it is what someone needs before touching the file. app.sh and app.bat check the toolchain, install dependencies if the lockfile moved, find a device, wire the Metro reverse tunnel and start the dev server. They deliberately never set JAVA_HOME, ANDROID_HOME or PATH: silently rewriting a developer's toolchain for the duration of one command produces a build that works on one machine and nowhere else, and the failure surfaces hours later somewhere unrelated. Every check names exactly what is missing and stops. Writing the check found a stale requirement. AGENTS.md has said "SDK Platform 35+" since Phase 0, and app.sh duly failed on this machine — which builds the project every day. Expo SDK 57 compiles against 36, confirmed by asking Gradle directly (`:app:properties` reports compileSdkVersion 36, targetSdkVersion 36, minSdkVersion 26), and only android-36 and android-36.1 are installed here. Corrected in AGENTS.md, README and both scripts, and the check now compares the highest installed major rather than looking for one exact directory, because the platform directories carry point releases. Verified by running app.sh: every check passes, it finds the emulator, and it stops cleanly before Metro when told to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…asured
Two per-render counters removed: LibraryRow.body and useTracks.render.
They were temporary probes for two specific investigations, both of which are
closed and whose numbers are already recorded in docs/performance.md — 47 row
reconciliations per checkbox tap before the memo boundary and 1 after.
They had to go because of where they ended up being harmful. On the Mi 9T,
LibraryRow.body fires 45+ times per mount, and MIUI's logcat rate limiter
("chatty") responds by discarding lines — including useTracks.firstRows, which
is the cold-start measurement only this device can give. A probe that hides the
thing it is measuring is worse than no probe.
What stays is the measurements rather than the counters: firstRows, the
saveEnumerated busy/longest-block figures, and the mount markers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Written because two consecutive sessions opened with the same wrong premise — "branch from main" — when main holds 3 files and the project holds 225. That is now the first thing the document says, with the command to check it. Beyond state and what is left, it carries the traps that have already cost time: the spacing scale compiling to nothing, axis-locking a pan on the first update when both translations are zero, router.push stacking two screens behind a gesture, the compiler's immutability rule versus Reanimated, MIUI blocking input and silently rate-limiting logcat, and the WAL file that must be copied or the database looks empty. Also records two things found in the phone's database today and deliberately not chased: stage two is 446 of 521 rows short, which is consistent with a cancelled scan resuming correctly but has never been watched; and codec is null for every row while bitrate is populated, which may be API 29 versus 35 or may be the test files being untagged. Both are stated as unexplained rather than as defects, because nobody has checked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two sessions recorded "codec is null on all 521 rows of the Mi 9T" as an unexplained finding worth investigating at the native layer. It is codecOf working as specified. codecOf returns null whenever the MIME subtype is already a container name, because audio/mpeg otherwise renders the spec strip as "MP3 · mpeg" — the identical fact spelled worse. flac, mp4, mpeg and wav are all in CONTAINER_NAMES, so a library of mainstream formats has a null codec on every row, by design, always. The premise was wrong twice over. codec is derived in TS from the MIME type and never comes from the native reader, so "saveEnriched returned one field and not the other" described a path that does not exist. And the emulator's "FLAC · 44.1 kHz · 16-bit" that the phone was compared against is the container column, not the codec column. Pulled the database from the device to settle it. The 75 enriched rows read container=FLAC|M4A, codec=NULL, bitrate=143, sample_rate=44100, bit_depth=16, channels=2. The phone renders exactly what the emulator renders. No API 29 versus 35 difference, nothing to fix. bit_depth is null on the single M4A row, also correct — AAC is lossy and has no bit depth to report. Pins it with a test naming the device case, so the next person who reads a null codec column resolves it in a test file instead of a device session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 10 listed it as missing with the README's architecture section as a seed. The README says what the directories are; this says how the pieces meet, which is the part a new reader actually gets wrong. Four things it records that were not written down anywhere: The three places state lives, chosen by lifetime — SQLite for anything persistent, the AudioEngine singleton for playback because it outlives every screen, and MMKV for settings because the reads are synchronous and have to happen before the first frame. There is no global state library; zustand is in package.json and imported by nothing, which reads as an oversight until you know the engine is subscribed through useSyncExternalStore on purpose. The startup ordering in app/_layout.tsx is load-bearing rather than incidental, and each step has a specific failure if moved: a frame in the fallback font, a component that paints before its cssInterop registration, a screen querying an unmigrated schema. Why the two-stage scan is resumable — last_scanned_at being null IS the queue, so there is no separate progress state to keep honest. Why a finished listen crosses a boundary to become a statistic instead of the engine writing it directly. Also corrects the offline-only reference to ADR 009, which is where the merged manifests were actually generated and compared. Linked from the README's docs table and its architecture section. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The app has no global state library, by design. `AudioEngine` is a singleton that lives outside React and outlives every screen, and the player hooks subscribe to it through `useSyncExternalStore` — see the comment in src/features/player/hooks/usePlayback.ts for why that is the right primitive: it is exactly what the hook is for, and it gets tearing right during concurrent renders, which a hand-rolled subscription does not. QueueScreen and the Toaster use the same pattern against their own external stores. So zustand was not pending work, just an unused entry left over in the dependency list. Nothing under src/ imports it, and `npm uninstall` removed exactly one package, so nothing reached it transitively either. Gate after removal: lint clean, typecheck clean, 293 tests / 20 suites passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Tail of 09c6225, which dropped the dependency but left four files still describing it. AGENTS.md rule 5 and the tech-stack table both named Zustand as where transient player/UI state lives. It never did — the AudioEngine singleton holds it and React reads it through useSyncExternalStore. Both now say so, and rule 5 adds "there is no global state library; don't add one" so the next person does not reintroduce one to fill an apparent gap. The eslint and jest configs ignore .claude/worktrees/*. Those hold full checkouts of other branches, so lint reported the root-level src/ exemptions as violations (the path-based rules only match src/ at the repo root) and jest collected every suite a second time. Note that overriding testPathIgnorePatterns drops its default, so /node_modules/ is repeated there deliberately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three of the four things the handoff listed as owed are now verified on a device. The fourth, frame timing, is the only one automation cannot reach. The previous session blamed flaky taps on the emulator being up nine hours. That was true but not the whole reason: every control in this app carries an accessibilityLabel, so driving the UI from `uiautomator dump` by label works where guessed pixel coordinates do not. Rebooted the AVD and did the whole pass that way. Playlist chain in one pass — create, name, add 3 tracks, drag-reorder, play. The reorder needs `input motionevent` DOWN/hold/MOVE/UP, because the handle is Gesture.Pan().activateAfterLongPress(120) and a plain `input swipe` never activates it; that is why earlier attempts saw nothing move. Moving perf-001 from position 2 to 0 renumbered all three rows correctly. QueueSource reaches the database: source_type=playlist, source_id=1, and three matching stats_rollups rows for week, month and year. Repeat-listen produced two distinct outcome=play rows for one track, 211171 ms and 144145 ms, each closed by a manual scrubber drag to the start. track_stats and all three rollup periods agree. Release APK installed and run for the first time in the project's life, with `adb reverse --remove-all` first so it could not quietly fall back to Metro. aapt2 confirms no INTERNET permission before installing. Boots clean under Hermes and minification, all four tabs render. Functional only — no numbers taken, so the debug-build caveat stands. Two incidental confirmations: background audio survives the app going to the launcher, and the spec strip on a real MP3 reads "MP3 · 44.1 kHz · 138 kbps · Stereo · 5.1 MB" — no codec field, per the finding closed in 255cacc, and 138 kbps from SpecMath computing over the retriever's bogus reported 32. One gap found: MiniPlayer renders only in app/(tabs)/_layout.tsx, so pushed stack screens have no transport control. Recorded as a design question rather than fixed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR improves performance and UX across the Library, Player, and Stats features, while expanding on-device statistics attribution (queue source + shuffle algorithm) and adding supporting UI infrastructure (toasts, skeletons, confirmations) and documentation/tests to lock behavior in.
Changes:
- Add queue attribution + repeat-listen boundary detection so statistics reflect playlist/album/artist sources, shuffle algorithm, and repeat-one / rewind-to-start listens.
- Rework Library UI into Tracks/Artists/Albums views with better scan UX (user-initiated scan + confirmation), improved list performance, and new skeleton components.
- Enhance Stats and Settings UX (Wrapped summary, top albums/playlists, richer setting explanations, toasts) plus add regression-preventing tests and updated docs.
Reviewed changes
Copilot reviewed 73 out of 76 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/theme/scale.test.ts | Adds a test to prevent non-existent numeric spacing utilities from silently compiling to no CSS. |
| src/services/toast/index.ts | Introduces a module-level toast store for transient confirmations. |
| src/services/stats/repeatListen.ts | Adds pure logic to detect “rewind/loop to restart” as a new listen boundary. |
| src/services/stats/repeatListen.test.ts | Pins repeat-listen thresholds and validates behavior over realistic tick sequences. |
| src/services/scanner/trackMapping.test.ts | Adds coverage clarifying codec/container null behavior for common MIME types. |
| src/services/scanner/scanner.ts | Fixes stage-two progress total reporting via a pre-count denominator. |
| src/services/scanner/scanner.test.ts | Updates scanner harness/tests for new unenriched-count port and progress semantics. |
| src/services/format/listeningTime.ts | Improves formatting for <1 minute by showing seconds instead of “0m”. |
| src/services/format/listeningTime.test.ts | Adds tests for the new seconds behavior and rounding rules. |
| src/services/audio/types.ts | Adds QueueSource and shuffleAlgorithm fields to finished listens for stats attribution. |
| src/services/audio/AudioEngine.ts | Records listen source/shuffle, and adds repeat-listen boundary handling based on rewind detection. |
| src/i18n/locales/tr.json | Updates Turkish strings for new Library/Stats/Settings UI and toast messages. |
| src/i18n/locales/en.json | Updates English strings for new Library/Stats/Settings UI and toast messages. |
| src/features/stats/StatsScreen.tsx | Adds Wrapped summary card and expands ranked lists to albums and playlists. |
| src/features/stats/components/Wrapped.tsx | New “Wrapped”-style summary card for a period’s headline stats. |
| src/features/stats/components/TopList.tsx | Enhances ranked lists with artwork/icon fallback and adds listening-time per row. |
| src/features/settings/SettingsScreen.tsx | Reworks settings layout with explanations, option list shuffle picker, and new switches. |
| src/features/playlists/PlaylistDetailScreen.tsx | Ensures playback from playlists sets queue source for stats attribution. |
| src/features/playlists/components/PlaylistMosaic.tsx | Fixes large mosaic sizing to avoid invalid spacing classes and invisible UI. |
| src/features/playlists/components/AddTracksSheet.tsx | Adjusts checkbox sizing to stay within the spacing scale. |
| src/features/playlists/components/AddToPlaylistSheet.tsx | Replaces invalid max-h-* spacing with max-h-full. |
| src/features/player/PlayerScreen.tsx | Switches to ArtworkCarousel, uses router.navigate, and updates layout padding. |
| src/features/player/listenRecorder.ts | Persists listen source + shuffle algorithm into play-event recording. |
| src/features/player/hooks/useQueueNeighbours.ts | Adds a queue-neighbour hook to support the artwork carousel without 2Hz churn. |
| src/features/player/components/TransportSwipe.tsx | Removes the old transport swipe wrapper (replaced by carousel). |
| src/features/player/components/ArtworkCarousel.tsx | Adds a 3-slot artwork carousel with axis lock, rubber-banding, and dismiss gesture. |
| src/features/library/LibraryTracks.tsx | Extracts track-list behaviors (selection/sheets/play) from LibraryScreen for size/perf. |
| src/features/library/LibraryScreen.tsx | Adds Tracks/Artists/Albums segmented views and user-confirmed scanning flow. |
| src/features/library/hooks/useTrackActions.ts | Adds translated toasts for queueing/playing-next/favourite actions. |
| src/features/library/hooks/useSelection.ts | Memoizes the selection object to reduce renders in large virtualized lists. |
| src/features/library/hooks/useScan.ts | Removes automatic sweep; adds ignore-short-files option and stage-two count port. |
| src/features/library/hooks/useCollectionRouting.ts | Adds stable navigation callbacks for artist/album collection routes. |
| src/features/library/components/TrackListSkeleton.tsx | Replaces bespoke skeleton rows with shared SkeletonRows. |
| src/features/library/components/TrackList.tsx | Refactors row rendering for stable props and moves swipe logic into LibraryRow. |
| src/features/library/components/TrackInfoSheet.tsx | Uses fractional label width to avoid invalid spacing classes. |
| src/features/library/components/ScanBanner.tsx | Hides “0 / 0” until a total is known for scanning progress. |
| src/features/library/components/LibraryRow.tsx | Adds a memoized wrapper combining TrackRow + SwipeableRow for perf. |
| src/features/library/components/LibraryHeader.tsx | Adds explicit Scan/Add Folder buttons and updates the header controls. |
| src/features/library/components/CollectionHeader.tsx | Adds shared header for artist/album detail screens with cover + metadata. |
| src/features/library/components/CollectionGrid.tsx | Adds FlashList-based grid for artist/album shelves. |
| src/features/library/components/CollectionCard.tsx | Adds memoized artist/album card UI with artwork fallback and accessibility. |
| src/features/library/CollectionDetailScreen.tsx | New detail screen for an artist/album, reusing LibraryTracks and attributing source. |
| src/db/queries/tracks.ts | Adds artist/album card queries and collection track query; adjusts perf instrumentation. |
| src/db/queries/stats.ts | Adds top albums/playlists queries and artwork selection via bounded subqueries. |
| src/components/ui/Toaster.tsx | Adds UI component subscribing to toast store and rendering animated toast pill. |
| src/components/ui/SwipeableRow.tsx | Memoizes the Pan gesture and fixes reveal width to a valid spacing class. |
| src/components/ui/Skeleton.tsx | Adds shared skeleton primitives (block/rows/cards) driven by Reanimated. |
| src/components/ui/SettingSwitch.tsx | Adds a themed native Switch row with label + description. |
| src/components/ui/SettingRow.tsx | Adds optional descriptions and adjusts layout to support explanatory text. |
| src/components/ui/OptionList.tsx | Adds a radio-like option list with per-option descriptions (used for shuffle). |
| src/components/ui/ConfirmDialog.tsx | Adds themed confirm modal for slow/irreversible actions (used for scanning). |
| src/components/ui/ActionSheet.tsx | Replaces invalid max-h-* spacing with max-h-full. |
| README.md | Adds a full project README describing offline-only promise, architecture, and workflow. |
| package.json | Removes Zustand dependency (aligns with module-store + useSyncExternalStore approach). |
| package-lock.json | Removes Zustand lockfile entries. |
| LICENSE | Adds MIT license file. |
| jest.config.js | Ignores .claude worktrees to avoid double-collecting tests. |
| HANDOFF.md | Adds a detailed project handoff/status doc with known traps and remaining work. |
| eslint.config.js | Ignores .claude/** to prevent linting other-branch worktrees. |
| docs/stats.md | Documents repeat-listen detection and its rationale. |
| docs/performance.md | Adds transition/list performance investigation notes and updated regression/device checks. |
| docs/components.md | Adds a component map and repository UI rules summary. |
| docs/architecture.md | Adds a high-level architecture map, state locations, and flow descriptions. |
| docs/adr/012-artist-and-album-shelves.md | ADR for artist/album shelves and rationale for no genre shelf. |
| docs/adr/011-repeat-listen-detection.md | ADR for repeat-listen boundary detection. |
| docs/adr/010-scanning-is-user-initiated.md | ADR for making scanning explicitly user-initiated. |
| docs/01-TECH-STACK.md | Updates state management approach away from Zustand to module stores + useSyncExternalStore. |
| CONTRIBUTING.md | Adds contributor workflow and gating requirements. |
| app/collection/[kind]/[id].tsx | Adds collection route that reads params and renders the detail screen. |
| app/(tabs)/_layout.tsx | Mounts Toaster above MiniPlayer in the tab stack layout. |
| app/_layout.tsx | Minor layout formatting update. |
| app.sh | Adds a toolchain/device bootstrap script for macOS/Linux. |
| app.bat | Adds a toolchain/device bootstrap script for Windows. |
| AGENTS.md | Updates setup requirements and clarifies the “no global state library” rule. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+353
to
+356
| private beginNextCycle(): void { | ||
| this.flushListen(true); | ||
| this.startedAt = new Date(); | ||
| } |
Comment on lines
+52
to
+56
| const [view, setView] = useState<LibraryView>('tracks'); | ||
| const [search, setSearch] = useState(''); | ||
| // The field stays instant; only the query waits. | ||
| const { tracks, isLoading } = useTracks(useDebounced(search)); | ||
| /* | ||
| * The launch sweep waits for the list to be on screen. Firing it on mount put | ||
| * a MediaStore enumeration on the JS thread alongside the first library query | ||
| * — see `useScan` and docs/performance.md. | ||
| */ | ||
| const { progress, isScanning, isRefreshing, scanLibrary, addFolder, rescan, cancel } = | ||
| useScan(!isLoading); | ||
| const { progress, isScanning, isRefreshing, scanLibrary, addFolder, rescan, cancel } = useScan(); |
Comment on lines
91
to
95
| isScanning={isScanning} | ||
| onAddMusic={addFolder} | ||
| onStartSelecting={selection.activate} | ||
| onScan={askToScan} | ||
| onAddFolder={addFolder} | ||
| onStartSelecting={() => setView('tracks')} | ||
| /> |
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.
No description provided.