Commit Graph
939 Commits
Author SHA1 Message Date
austinried a4149690e5 wire the sync fetch concurrency to a Settings option
The bound was only a constructor default with no production consumer. Add a
"Sync concurrency" preference (Settings -> Network, default 4) and pass it
through SourceRepository to SubsonicSource, so sync parallelism is tunable
without a rebuild. Both the app-scoped source and the on-demand
`activeMusicSource()` used by sync read the preference.

- UserPreferences.syncConcurrency()/setSyncConcurrency() (clamped >= 1)
- SourceRepository combines the preference into toMusicSource
- Settings row + dialog; SettingsViewModel state and setter
- Preferences test for default/round-trip/clamp
2026-10-04 00:30:02 +00:00
austinried adcdc442e4 address review: mark the fetch bound test-only; fix the concurrency test
- The concurrency bound already existed; this parameter only exists so tests
  can force the sequential path, and no production path passes it. Mark it
  `@VisibleForTesting` so that is explicit rather than reading as a wired
  setting.
- `playlistSongsUseTheSameRowsAtAnyFetchBound` could not fail (bound has no
  effect on the rows) — delete it and its `normalized` helper.
- `playlistSongsRespectsASmallerFetchBound` relied on a 300ms overlap
  window; use latches so the bound is proven deterministically (fails when
  the semaphore ignores the bound).
2026-10-04 00:30:01 +00:00
austinried dd092ed7ad make the sync fetch concurrency bound configurable 2026-10-04 00:30:01 +00:00
austinried 8ba9e92fa6 export the v19 schema after the queue keyset index migration 2026-10-04 00:30:01 +00:00
austinried 729737895f address review: offset fallback for far backward album seeks; pin the playlist keyset
- The NULL-aware backward album predicate is not sargable, so a backward seek
  from a far cursor scans the whole album partition. Fall back to the offset
  query when the anchor is more than twice the offset away; near-cursor
  backward reads still use keyset.
- Cover the playlist keyset path: a DAO test for `playlistSongsBefore` and a
  repository test scanning a playlist forward and backward. Both fail if the
  playlist order key stops driving the seek.
2026-10-04 00:30:01 +00:00
austinried 87dc4499ca split the song projections; the queue carries the playlist key
`SongListItem` carried a nullable `position` that only playlist rows have,
so the album/song queries projected a fake `NULL AS position` just to map
the shared type — a leak of the playlist concept into unrelated reads.

Split it into `AlbumSongItem` (song, coverArt) and `PlaylistSongItem` (song,
coverArt, position), plus a `SongItem` interface so the queue stays generic
over kinds. The queue's `QueueWindowItem` now carries the optional
`playlistPosition` for the keyset cursor, so no projection fabricates it.
Playlist detail and its paging use `PlaylistSongItem`.
2026-10-04 00:30:01 +00:00
austinried 649528e338 address review: NULL disc/track in backward album seeks; drop redundant index
- `albumSongsBefore` used the null-intolerant row-value `<`, so an album
  with NULL `disc`/`track` (untagged files) dropped exactly the rows a
  backward window needs, returning the wrong track at a position. Use a
  NULL-aware predicate (NULL sorts first, so it belongs to any `before`
  set); `albumSongsFrom` is unaffected.
- The new `(sourceId, albumId, disc, track, id)` index has the old
  `(sourceId, albumId)` index as a prefix, so drop the latter in the
  migration and from the entity.
- Tests: mixed NULL disc/track album for the before-window and a full
  backward scan; both fail on the previous commit.
2026-10-04 00:30:01 +00:00
austinried 4aaeb4871b resolve queue rows with keyset cursor seeks instead of offsets 2026-10-04 00:30:01 +00:00
austinried d710a3185c clear the compile warnings the downloads work left behind
- ArtworkStore's fetcher still did `response.body ?: throw` from before the body
  size limit landed; OkHttp's `body` is not nullable, so the elvis was dead.
- DownloadRepository's `downloadStates` uses `flatMapLatest` without the opt-in
  its callers annotate.
- HeroHeader's subtitle tap invoked `onSubtitleClick?.invoke()` inside the branch
  where it is already known to be non-null.
- MediaItemTest used a safe call on `clippingConfiguration`, which never returns
  null.

No behaviour change: a clean full rebuild now compiles without warnings.
2026-10-04 00:30:01 +00:00
austinried 6a311638c2 finish the queue view test row race fix
#71 converted only two reads; the rest of `QueueViewModelTest` still read
`viewModel.rows` — a snapshot list mutated on the main dispatcher — from the
test thread, so `removeAndUndoKeepTheBoundedWindowConsistent` could map a torn
list (the removed position missing) under load. Route every test-thread read
through `rowsSnapshot()` and run the synchronous `reorder` mutations on the
dispatcher too.
2026-10-04 00:30:01 +00:00
austinried 65ed658433 address the artwork review: snapshots, atomic writes, bounded reads
- SourceRepository.coverArt and networkCoverArt read the source id and the
  source object separately, so a source switch between the two could pair one
  source's cache key with another source's URL; for the store that meant writing
  the wrong server's artwork into the per-source file for good. Both now resolve
  from a single active snapshot.
- ArtworkStore wrote through a fixed `<name>.part`, so two downloads of songs
  from one album could interleave: one publishing the file while the other still
  had the shared inode open, which leaves a torn image for Coil to decode. Each
  write gets its own partial now.
- OkHttpArtworkFetcher read the whole body into memory whatever its size. It
  refuses anything past 8 MB, checked by declared length and again while reading.
- Artwork whose library rows could not all be resolved is no longer swept:
  pruning an album while its songs remain used to delete art a download still
  held. A source with no download rows still sweeps everything.
- fetchArtworkSafely and storeArtwork no longer swallow CancellationException,
  and the poll loop fetches a resumed row's artwork off the tick instead of
  holding up the next one.
- Reconcile reads the download rows once rather than twice, and download() no
  longer repeats the check enqueueIfAbsent makes under the lock.
2026-10-04 00:30:01 +00:00
austinried 6942798bf2 start the download before fetching its artwork
- The artwork step no longer holds up the media: the song goes to the platform
  first and the art is fetched afterwards, in the same foreground window. A
  stalled artwork server can no longer delay a download, and a process death
  during the fetch can no longer lose the download entirely.
- Resuming a queued download after a restart fetches its artwork now too; only a
  fresh download did before.
- The artwork budget is a minute rather than ten seconds, which costs nothing but
  a live coroutine now that nothing waits on it for the media.
- OkHttpArtworkFetcher derives a client with a per-call timeout. A blocking
  OkHttp call is not interrupted by a coroutine timeout, so without it the outer
  budget bounded our wait but not the socket.
2026-10-04 00:30:01 +00:00
austinried 11cc5e9ab8 download a song's artwork with it and use it before the network
Second block of #65.

- ArtworkStore keeps the album's and the artist's cover art, thumbnail and full,
  in downloads/<sourceId>/art, keyed by the cache key the rest of the app already
  uses for Coil and for palettes. Keys are base64url encoded because cover art ids
  come from the server and must not reach outside the art directory.
- Downloading a song fetches that artwork first, in our process and while the app
  is in the foreground, under a timeout, and hands the media to the platform
  afterwards whether or not the artwork arrived. A song whose album or artist has
  no art, or whose art fetch fails, still downloads.
- SourceRepository.coverArt resolves a stored file before the server, at the one
  point every CoverArt, the notification bitmap loader and the palette prefetch go
  through, so downloaded artwork works without the network.
- The sweep prunes art that no download references, so removing the last download
  of an album drops its art while art another download still needs stays.
2026-10-04 00:30:01 +00:00
austinried 033ac56cf1 read queue view rows on the main dispatcher in the view model tests
`QueueViewModelTest` read `viewModel.rows` (a `mutableStateListOf` mutated
on the main dispatcher) from the test thread, racing the view model: CI saw
`ConcurrentModificationException` in `removingWhileShuffledReconcilesTheRows`
and a torn read in `removingATrackUnderTheBlockReloadsRows`. Copy the rows
on the dispatcher via a `rowsSnapshot()` helper before asserting.
2026-10-04 00:30:01 +00:00
austinried 7ee7dc474a drop the indicator placement test
It measured pixel bounds to assert which line the indicator sits on and which
side of the artist name it lands. That is a presentational choice with exactly
one place in the code that makes it, so the test could only fail when someone
deliberately changed that line, and would break on unrelated layout changes.
The state-to-indicator tests and the accessibility descriptions stay.
2026-10-04 00:30:01 +00:00
austinried b9e16d8377 put the download indicator on the artist line, tight to its left
The glyph and the progress ring sat in the title row after the star, spaced 8dp;
they now lead the subtitle row, 4dp from the artist name, which is where the
artist line has room for them. `theDownloadedIndicatorSitsOnTheArtistLineToItsLeft`
measures the unmerged bounds and fails if the indicator moves back up to the
title line or to the artist's right.
2026-10-04 00:30:01 +00:00
austinried 9cdbc92975 bind the download engine by its interface in Koin
`single { SystemDownloadEngine(...) }` registers the concrete class, but
DownloadRepository asks for the DownloadEngine interface, so resolving
PlaybackController threw NoDefinitionFoundException and the app died on launch
with a Koin InstanceCreationException. It is now `single<DownloadEngine>`, the
way the other interface singletons in the module are declared.

Nothing in the suite ever started Koin — the repository tests build the
repository directly with a fake — which is why a launch-crashing binding got
through. AppModuleTest now resolves the playback graph from the real module, and
it failed with the same NoDefinitionFoundException before this fix.
2026-10-04 00:30:01 +00:00
austinried 2520c60f5a publish the download map before signalling it is loaded
Second review round on the downloads block:

- `loaded` was completed upstream of the state flow that publishes the map, so
  `awaitLoaded()` could return while the map was still the initial empty one and
  the first window after a cold start would stream a track that is already on
  disk. The collector now assigns the map before completing, and the map is a
  plain state flow instead of a lazy eager `stateIn` with a bare touch to start
  it.
- A failed download now deletes its partial file, in both the engine-failure
  and the recovery paths. A failed row used to protect its own partial bytes
  from the sweep, and the UI offers no delete for it.
- A queued row is left queued when the download URL cannot be resolved yet,
  instead of being failed. At launch the repository's reconcile can run before
  the source repository has built its client, and failing there turned a
  resumable download into a dead one.
- The poller only considers the active source's rows, so a queued row belonging
  to another source no longer keeps it polling the database every second.
- `SourceRepository.downloadUri` takes the expected source id and returns null
  unless it is the active one, so a download cannot be enqueued against the
  previous server's URL during a source switch.
- Tests: a stream with no content length still gets its declared length through
  the production data source factory (the http half of the chain, which the
  first round left uncovered), and an engine failure is asserted to leave no
  partial file. The engine's status mapping now runs as a plain unit test.
2026-10-04 00:30:01 +00:00
austinried 31c1fec322 fix downloaded playback and tighten the download state machine
Review findings on the first downloads block:

- ExoPlayer could not open a downloaded file at all: the service's data source
  factory was OkHttp only, so a file:// URI went to the HTTP stack and failed
  with a malformed URL. Wrap it in DefaultDataSource so the file, asset and
  content schemes are resolved before the stream chain, and cover it with a
  test that reads a local file through the production factory.
- Reconcile no longer restarts a failed download, or a running one whose engine
  entry vanished: only a queued row is started again, and the rest become failed
  so the explicit Retry in the UI means something.
- song_downloads also references songs with CASCADE now, so a song pruned from
  the mirror takes its download row with it and the sweep can reclaim the file.
- The mutating methods run on the injected dispatcher rather than the caller's,
  which was the main thread for every call site, and the repository exposes one
  shared state flow instead of a second map plus five subscriptions.
- Playback stops when the active source disappears, including deleting the last
  source while a downloaded track is playing.
- Dropped the unused request mime type, the queued-at column nothing read, the
  metered constructor parameter nobody varied, and a dead index guard.
- The running indicator is asserted positively, the engine's status and reason
  mapping has a test, and the failure paths use the fake's failure and forget
  helpers.
2026-10-04 00:30:01 +00:00
austinried 2b104f1387 download a song to the device and play it from there
First block of #65: the state machine, the platform download engine, a
single-song download from the song context menu, the downloaded indicator in
song lists, and local-first playback.

- `song_downloads`, keyed `(sourceId, songId)` with CASCADE on the source, and
  the 16 -> 17 migration.
- `DownloadEngine` over `android.app.DownloadManager`, with a fake for tests.
  Files land in app-specific external storage at
  `.../Music/downloads/<sourceId>/<songId>`, plus a `.nomedia` so media scanners
  on pre-Android-10 devices leave them alone.
- `DownloadRepository` enqueues, mirrors progress by polling, cancels, deletes
  and reconciles on start and on source change: a queued row whose engine entry
  is gone is started again, a completed row whose file is gone becomes failed
  rather than being resurrected, and files with no row are swept.
- Playback prefers the downloaded file: the media item build takes the local URI
  when the download is complete and leaves it unclipped.
- Deleting a download reloads the player window at its position, so the item
  falls back to streaming instead of failing mid-read.
- Deleting a source cancels its downloads and removes their files.

Downloads are the system's own notifications and stay off metered networks for
now. Artwork, bulk downloads and the management screen are later blocks.
2026-10-04 00:30:01 +00:00
austinried b21c297964 keep the block in place when the anchor is removed while a queued track plays
`adjustAnchorOnRemove` moved the anchor to the canonical predecessor of the
removed track, which in a shuffled queue is an unrelated play position, so
removing the context anchor while playing a queued track teleported the
up-next block somewhere random (and out of view).

When the removed context track is the anchor, move the anchor to the track
before it in play order (or -1), so the block stays where it sits. This also
covers removing the playing context anchor, so the controller's separate
re-anchor is no longer needed and is removed.
2026-10-04 00:30:01 +00:00
austinried 72330387c8 fix resuming after removing the playing track in a shuffled queue
The "play whatever now fills the removed track's slot" branch was written
for the old re-derive model: it mapped the removed canonical slot through
the new permutation, which now (with the fixed shuffle order) picks an
unrelated track. Removing the playing track should just resume at the same
slot, which holds whatever was next.

- Drop the special slot branch: `position == current` resumes at `current`
  for shuffled and unshuffled alike. Without a block that is the next
  shuffled track; with one, re-anchor the block from the removed track onto
  the track now preceding that slot so the block takes the slot and plays
  next (previously it was stranded behind a random track).
- Tests assert the immediate playing track, not just what is upcoming, which
  is why they missed this. Both new tests fail on the previous commit.
- Cover advancing out of a block track and moving a block track while
  shuffled (the last items from the QA review).
2026-10-04 00:30:01 +00:00
austinried eb6b08ff56 replace the seed-flaky block test and cover clearing the block while shuffled
`removingThePlayingTrackWhileShuffledLeavesTheBlockReachable` looped 30
unseeded shuffles and only checked a weak invariant; it is now a single
deterministic case (fixed seed) that asserts the cursor resolves to the
playing track and the queued track still plays next. Add clearing the block
while shuffled, asserting the cursor stays on the playing track.
2026-10-04 00:30:01 +00:00
austinried 7b1a2a8f95 cover the queue/shuffle invariants the QA review flagged
The shuffled-removal tests mostly removed the *current* track (a path that
was already correct) and asserted `state.item`, which `rebuildWindow` keeps
regardless of the computed cursor, so a wrong position went unnoticed. Add:

- deterministic shuffle setup (`seedShuffledQueue`, or replace + setShuffle
  + setCursor + connect) so shuffle tests assert exact behaviour instead of
  looping over random seeds;
- controller tests that assert the queue position still resolves to the
  playing track after removing a non-current track, with and without a block,
  and that undo restores the shuffled order;
- a repository property test removing every position with a block present,
  checking the play/flat bijection, the combined/contextPlay inverse, the
  block span and that no track repeats;
- a `QueueRow.isPlaying` predicate (position AND song id) extracted and
  unit-tested, and QueueViewModel tests for starting a shuffled queue at the
  top and reconciling after a shuffled removal;
- Shuffle degenerate/out-of-range/MAX_SIZE coverage.
2026-10-04 00:30:01 +00:00
austinried 736e59fec6 keep the shuffle order stable when removing a track
Removing a track re-derived the permutation from (seed, size), so every
remaining track moved to a new position and the queue visibly reshuffled
(in an album-heavy playlist, the removed row and its neighbours were
replaced by other tracks from the same album).

Keep the permutation fixed over the domain it was created with and subtract
removals from it instead: `ShuffleState` holds the seed, the domain size and
the removed domain slots, kept in memory only. `flatContext`/`playContext`
skip those slots so the surviving tracks keep their positions; the DB still
stores just the seed and domain, so the old per-track `shuffle_order` rows
stay gone. A new shuffle or a library change clears the removal list; on
restart the removals are gone, so a queue that was edited falls back to
unshuffled rather than guessing an order.
2026-10-04 00:30:01 +00:00
austinried 6d25b31a9a fix shuffled queue edits: flat remap, pinned shuffle start, stable highlight
The previous commit assumed a removed/moved track only shifts play
positions by one, but a shuffled context removal re-derives the
permutation, so the current track's play position is a remap of its flat
index. `removeAt` now maps the current's flat through the updated order
(shuffled) or shifts it (unshuffled), and re-anchors the up-next block to
the track the cursor lands on. This fixes removing any track while
shuffled picking a random track and the anchor removal not advancing to
the block.

Restore "shuffle starts at the top": pick the seed so the requested start
track maps to play position 0 (`pinnedSeed`), instead of leaving the
current at its derived position. Applies to shuffle-play, tapping a track
while shuffled, toggling shuffle on, and restore.

The queue view's playing highlight now requires both the combined position
and the song id, so a transient position mismatch no longer flashes the
row above as playing.
2026-10-04 00:30:01 +00:00
austinried e142148b5a stop locating the playing queue entry by song id
The queue can hold the same song more than once (a song in both the context
and the up-next block, a duplicate album track, ...), so resolving the
playing entry by song id could match the wrong copy.

- `PlaybackController.move` and `removeAt` now derive the current track's
  new position from the edit (block index / context play, read back through
  the updated snapshot so a moved anchor is accounted for) instead of
  `combinedIndexOf`/`combinedContextIndexOf`.
- the queue view's playing highlight compares the row position to the
  playback position instead of the song id, so duplicates no longer both
  light up.

Regression tests cover a move and a remove with a duplicate song in the
context; both fail without the change.

Still id-based, deliberately: `undo`'s canMirror guard (already position-
guarded), the shuffle-state-changed branch of `shiftWindowLocked` (rare,
needs the id to find the track after an arbitrary reorder) and the view's
LazyColumn keys (cosmetic only).
2026-10-04 00:30:01 +00:00
austinried 902f556fe6 advance past a duplicate song when the playing queued track is deleted
Removing the currently playing track resolved its id against the updated
queue afterwards; if the same song also exists in the context, the lookup
matched that copy and jumped the cursor to it instead of advancing. When the
removed position was the playing one, use the fallback (next in the block /
slot fill) rather than a by-id lookup.

Pre-existing on main, exposed while testing the shuffle rework. Regression
test fails without the fix.
2026-10-04 00:30:01 +00:00
austinried 7f674f04be address review simplifications: setShuffle(seed?) and a nullable playContext
- `QueueRepository.setShuffle(seed: Long?)` derives `shuffleEnabled` from the
  seed, so disabled callers drop the meaningless sentinel `0L`.
- `playContext` returns `Long?` for symmetry with `flatContext`; `anchorPlay`
  treats an out-of-range anchor as -1 instead of silently clamping to the last
  track, and `combinedContextIndexOf` returns null.
2026-10-04 00:30:01 +00:00
austinried e6386ebddd keep the up-next block reachable when the playing context track is removed while shuffled
Review found that removing the playing context track while shuffled with a
non-empty up-next block stranded the block behind the cursor: the
repository moves `upNextAnchor` to the preceding track, but the cursor's
re-derived slot position can land after the block, so the queued tracks
were skipped. Re-anchor the block to the track the cursor lands on, same
as `playAt`/`toggleShuffle`.

- regression test loops the sequence over fresh random seeds and asserts
  the block stays reachable; fails without the fix ("stranded behind the
  cursor").
- `removingTheCurrentTrackWhileShuffled...` now asserts the canonical slot
  the current track occupies instead of recomputing the production position
  formula (which only checked the implementation against itself).
- `QueueRepository.removeAt` returns Unit; every caller discarded the
  snapshot and re-read it, so it no longer builds one.
2026-10-04 00:30:01 +00:00
austinried 93742e8584 rework: derive the shuffle order from a seed on top of the up-next queue
Replaces the per-track `shuffle_order` table and the retained whole-order
arrays with `playback_cursor.shuffleSeed`/`shuffleSize` and a seekable
Feistel permutation of `[0, size)` (`Shuffle.kt`). Positions map through
`flatContext`/`playContext`; a snapshot is shuffled only while the stored
size matches the context, so a stale size is ignored. Queue migration
16->17 adds `shuffleSize` and drops `shuffle_order`.

Drops the "dragging a context row while shuffled reorders the canonical
queue" behaviour: it cannot be made WYSIWYG without materialising the
order. The context is playback-only while shuffled (the view already
disables context drags) and `QueueRepository.move` no-ops for a context
move while shuffled; manual ordering goes through the up-next block.

Removing a context track while shuffled re-derives the permutation at the
smaller size, keeping the same seed and the playhead on the current track,
or on whatever now fills the removed track's slot when it was playing.
2026-10-04 00:30:01 +00:00
austinried 0a11585205 Revert the downloads block 2026-10-04 00:30:01 +00:00
austinried ae41b0e66a publish the download map before signalling it is loaded
Second review round on the downloads block:

- `loaded` was completed upstream of the state flow that publishes the map, so
  `awaitLoaded()` could return while the map was still the initial empty one and
  the first window after a cold start would stream a track that is already on
  disk. The collector now assigns the map before completing, and the map is a
  plain state flow instead of a lazy eager `stateIn` with a bare touch to start
  it.
- A failed download now deletes its partial file, in both the engine-failure
  and the recovery paths. A failed row used to protect its own partial bytes
  from the sweep, and the UI offers no delete for it.
- A queued row is left queued when the download URL cannot be resolved yet,
  instead of being failed. At launch the repository's reconcile can run before
  the source repository has built its client, and failing there turned a
  resumable download into a dead one.
- The poller only considers the active source's rows, so a queued row belonging
  to another source no longer keeps it polling the database every second.
- `SourceRepository.downloadUri` takes the expected source id and returns null
  unless it is the active one, so a download cannot be enqueued against the
  previous server's URL during a source switch.
- Tests: a stream with no content length still gets its declared length through
  the production data source factory (the http half of the chain, which the
  first round left uncovered), and an engine failure is asserted to leave no
  partial file. The engine's status mapping now runs as a plain unit test.
2026-10-04 00:30:01 +00:00
austinried e037f3ef37 fix downloaded playback and tighten the download state machine
Review findings on the first downloads block:

- ExoPlayer could not open a downloaded file at all: the service's data source
  factory was OkHttp only, so a file:// URI went to the HTTP stack and failed
  with a malformed URL. Wrap it in DefaultDataSource so the file, asset and
  content schemes are resolved before the stream chain, and cover it with a
  test that reads a local file through the production factory.
- Reconcile no longer restarts a failed download, or a running one whose engine
  entry vanished: only a queued row is started again, and the rest become failed
  so the explicit Retry in the UI means something.
- song_downloads also references songs with CASCADE now, so a song pruned from
  the mirror takes its download row with it and the sweep can reclaim the file.
- The mutating methods run on the injected dispatcher rather than the caller's,
  which was the main thread for every call site, and the repository exposes one
  shared state flow instead of a second map plus five subscriptions.
- Playback stops when the active source disappears, including deleting the last
  source while a downloaded track is playing.
- Dropped the unused request mime type, the queued-at column nothing read, the
  metered constructor parameter nobody varied, and a dead index guard.
- The running indicator is asserted positively, the engine's status and reason
  mapping has a test, and the failure paths use the fake's failure and forget
  helpers.
2026-10-04 00:30:01 +00:00
austinried 7ac79288b3 download a song to the device and play it from there
First block of #65: the state machine, the platform download engine, a
single-song download from the song context menu, the downloaded indicator in
song lists, and local-first playback.

- `song_downloads`, keyed `(sourceId, songId)` with CASCADE on the source, and
  the 16 -> 17 migration.
- `DownloadEngine` over `android.app.DownloadManager`, with a fake for tests.
  Files land in app-specific external storage at
  `.../Music/downloads/<sourceId>/<songId>`, plus a `.nomedia` so media scanners
  on pre-Android-10 devices leave them alone.
- `DownloadRepository` enqueues, mirrors progress by polling, cancels, deletes
  and reconciles on start and on source change: a queued row whose engine entry
  is gone is started again, a completed row whose file is gone becomes failed
  rather than being resurrected, and files with no row are swept.
- Playback prefers the downloaded file: the media item build takes the local URI
  when the download is complete and leaves it unclipped.
- Deleting a download reloads the player window at its position, so the item
  falls back to streaming instead of failing mid-read.
- Deleting a source cancels its downloads and removes their files.

Downloads are the system's own notifications and stay off metered networks for
now. Artwork, bulk downloads and the management screen are later blocks.
2026-10-04 00:30:01 +00:00
austinried 07fb07762c keep the artist targets narrow and clear of the controls
Both targets were too greedy. They now hug the text instead of spanning the row, so a tap
beside the line does nothing, and the album detail target stops short of the controls
instead of reaching down to them.

Album detail: the line plus its 4dp lead-in and 8dp below (32dp), with the original
spacing kept, so the layout is exactly where it was. Now playing keeps the title+artist
block (58dp) but is no longer full width.

Both are shorter than the 48dp recommendation on purpose, to stay clear of the slider and
the play/shuffle buttons. Tests assert the measured geometry: no full-width target, a
12dp gap above the controls, and the now playing block staying at its own 58dp.
2026-10-04 00:30:01 +00:00
austinried 17a41fd579 give the album detail artist line a full touch target
The subtitle was a bare bodyMedium line: 20dp tall and only as wide as the text. It now
sits in a full-width target spanning the 4dp lead-in and the 20dp gap below it, so the
row clears 48dp while the text stays where it was. The album title is deliberately not
part of it, unlike now playing: it can wrap to two lines, so tapping it should not jump
to the artist. The controls row moves down 4dp, the only layout delta.

AlbumDetailTapsTest covers the tap, the 48dp minimum (fails on a 20dp node) and that the
title carries no click action.
2026-10-04 00:30:01 +00:00
austinried a296443109 make the now playing artist target the whole title block
The artist line is a single bodyMedium line, 20dp, well under the 48dp minimum touch
target. A 48dp artist-only target cannot fit next to the 32dp title inside the 58dp
block without borrowing from the title's bottom edge or from the slider's top padding,
so the block itself becomes the target: 58dp and full width, with no layout change and
nothing to overlap (the slider keeps its own 20dp of top padding before the track).

The trade-off is that tapping the song title now opens the artist too. The test asserts
the measured target is at least 48dp, which fails on a 20dp node.
2026-10-04 00:30:00 +00:00
austinried 7b1696da5e address the review of the tap targets
The "stays inert" test was tautological: the render helper always passed non-null no-op
lambdas, so its assertion held for any implementation. It now passes nullable callbacks
and asserts the targets are disabled, which fails if the gate is removed.

Accessibility: both taps carry onClickLabel ("Open album"/"Open artist") and the artist
box spans the full row, and the now playing cover passes showPlaceholder = coverArt ==
null so the always-present placeholder letter is not merged into the tap target's label
(the same flash the artist detail cells had).

Left as-is: the artist target is still one line tall (~20dp), and the album detail
subtitle tap covers the year too. Both are layout/scope trade-offs.
2026-10-04 00:30:00 +00:00
austinried b55e5cc5e8 make the now playing art and artist names tap targets
Tapping the cover art on now playing opens its album and tapping the artist name opens
the artist, matching the context menu actions; the album detail subtitle does the same
for its artist. Both are inert when the queue item has no album/artist id.

Artist detail's album cells also stop drawing the letter placeholder underneath: with a
thumbnail underlay and the full image on top, the letter only showed because the default
`showPlaceholder = true` was left on, so a cold cell flashed a letter before the art.
They now match the hero's `showPlaceholder = art == null`.
2026-10-04 00:30:00 +00:00
austinried edd6d850dd drop add to queue / play next from the now playing context menu 2026-10-04 00:30:00 +00:00
austinried b9e5e07506 use the delete-sweep icon for clearing the up-next queue 2026-10-04 00:30:00 +00:00
austinried 06cf49f4e8 queue view: disable drag auto-scroll; queue plays next when its anchor is removed
- removing the context track the up-next block sits behind now moves the
  block back behind the preceding track (or to the front when the anchor
  was first), so the queued track plays next instead of the following
  context track.
- disable sh.calvin.reorderable auto-scroll for every run
  (scrollThreshold 0). It keeps scrolling past a run's last valid target
  and can strand the dragged item off-screen; scroll first, then drag.
2026-10-04 00:30:00 +00:00
austinried 42840cd9ad queue view: stop context drags crossing the block, stabilise the title
- split the context into a reorder state either side of the up-next block.
  A context drag can no longer target across the block, so dragging from
  the context over a long queued block stops at the block instead of
  jumping over it and force-dropping.
- the context header's fallback is now the playing track's album, so it
  no longer flashes "Next from here" while the async name lookup runs
  when tapping between manually queued and context tracks.
2026-10-04 00:30:00 +00:00
austinried 718dcf31bd allow queue loading during drag; keep the album title after a reorder
- the mid-drag load guard also stopped the window extending while
  auto-scrolling, so dragging a context row over a long up-next block hit
  the loaded edge and dropped. Removed it: loadOlder/loadNewer now extend
  from the min/max loaded position, which is what prevents the duplicates.
- a reordered track becomes an explicit song entry, so the context kind
  switches to Song and the queue title fell back to "Next from here".
  sourceName now resolves a song entry to its album so the title keeps
  the album name.
2026-10-04 00:30:00 +00:00
austinried b189ae0cc5 stop the queue view extending itself mid-drag; restore context drag scroll
- the visible-items flow fired loadOlder/loadNewer during a drag, and the
  live reorder leaves row positions out of order, so the window re-added
  rows it already had — the queue duplicated under the dragged item.
  Skip loading while a drag is active, and extend from the min/max loaded
  position rather than the first/last row.
- auto-scroll is back for the context reorder state (a normal queue
  scrolls while dragging again); the up-next state keeps it disabled
  because its block is followed by context rows the scroller would run
  past with nowhere to drop.
2026-10-04 00:30:00 +00:00
austinried 5abc48848b name the reorderable library in the queue view comment 2026-10-04 00:30:00 +00:00
austinried eb5f5703f4 document why the queue view keeps a per-section reorder state and index-keyed headers 2026-10-04 00:30:00 +00:00
austinried 34f0e15a7a address review: anchor relocation and empty-context positions
- clamp anchorPlay to the context size, so an empty context with a queued
  block puts the block at combined 0 (previously position 0 was
  unresolvable and the queue view emptied).
- move/removeAt now derive the playing track's new position from its id
  after an edit, instead of assuming a fixed list delta; moving or
  removing the anchor relocates the whole block, which movedCursor
  ignored.
- removing the currently playing track reloads at the model's next
  position (the anchor may have relocated) rather than removing it from
  the player in place.
- the queue view's remove reconciles its rows from the repository instead
  of decrementing positions by one, so a block relocation stays in sync.
2026-10-04 00:30:00 +00:00
austinried 996b0f8f48 avoid reloading the window on queue edits; add clear up next
- moves that straddle the player window, and deletes that are not the
  current track, rebuilt the playlist with setWindow, restarting the
  playing track (a pause). They now reuse rebuildWindow, which re-lays
  the window around the playing item without replacing it.
- deleting the currently playing queued track now removes it from the
  player in place so it advances to the next track without a reload.
- the Up next section header has a clear button that drops the whole
  queued block and resumes the context after the anchor.
2026-10-04 00:30:00 +00:00