Post-merge review fixes for Songs tab Compose migration

.editorconfig

@@ -8,7 +8,6 @@ ktlint_standard_property-naming=disabled
 ktlint_standard_filename=disabled
 ktlint_standard_package-name=disabled
 ktlint_code_style=android_studio
-ktlint_function_naming_ignore_when_annotated_with = Composable
+ktlint_function_naming_ignore_when_annotated_with=Composable
 ktlint_standard_trailing-comma-on-declaration-site=disabled
 ktlint_standard_trailing-comma-on-call-site=disabled
-ktlint_function_naming_ignore_when_annotated_with=Composable

CLAUDE.md

@@ -0,0 +1,170 @@
+# CLAUDE.md
+
+This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
+
+## Project Overview
+
+S2 Music Player — an Android app for local music playback and streaming via Jellyfin, Emby, and Plex. Features Android Auto, Chromecast, custom EQ, replay gain, sleep timer, batch tag editing, and Material 3 theming.
+
+## Build Commands
+
+All commands run from the repository root.
+
+```bash
+# Build debug APK
+./gradlew :android:app:assembleDebug
+
+# Run all unit tests
+./gradlew testDebugUnitTest
+# Or via script:
+./support/scripts/unit-test
+
+# Run a single module's tests
+./gradlew :android:playback:testDebugUnitTest
+
+# Run instrumented tests (requires device/emulator)
+./gradlew :android:app:connectedCheck
+
+# Lint (KTLint)
+./support/scripts/lint
+
+# Build release bundle
+./gradlew :android:app:bundleRelease
+```
+
+## Architecture
+
+### Module Structure
+
+- **`:android:app`** — Main application: UI screens, DI setup, presenters, navigation
+- **`:android:playback`** — ExoPlayer wrapper, PlaybackManager, PlaybackService, queue management, audio focus
+- **`:android:mediaprovider:core`** — MediaProvider interface, MediaImporter, repository interfaces (Song, Album, Playlist, Genre)
+- **`:android:mediaprovider:local`** — Local MediaStore/TagLib provider implementation
+- **`:android:mediaprovider:jellyfin|emby|plex`** — Remote streaming provider implementations
+- **`:android:data`** — Room database, Parcelable data models
+- **`:android:core`** — Shared utilities, logging, Hilt setup
+- **`:android:networking`** — Retrofit + OkHttp + Moshi network layer
+- **`:android:imageloader`** — Glide image loading
+- **`:android:trial`** — Trial/subscription management via Play Billing
+- **`:android:remote-config`** — Firebase Remote Config wrapper
+
+### UI Patterns
+
+The app uses **MVP (Model-View-Presenter)** with Fragments. Most screens are Fragment-based with custom `ViewBinder` pattern for RecyclerView items. Compose is used in newer screens (onboarding, genre list, shared components). Navigation uses Android Navigation Component with Safe Args.
+
+Key MVP base classes:
+- `BasePresenter<T : View>` — coroutine-scoped presenter with SupervisorJob
+- `BaseContract` — defines View/Presenter interfaces per screen
+
+### Playback Flow
+
+PlaybackManager orchestrates playback. It coordinates QueueManager, ExoPlayerPlayback, AudioFocusHelper, and PlaybackService (foreground service with MediaBrowserServiceCompat). State changes propagate via PlaybackWatcher callbacks.
+
+### Data Layer
+
+Repository pattern backed by Room database. MediaProvider implementations (local, Jellyfin, Emby, Plex) return `Flow<FlowEvent>` for reactive updates. MediaImporter coordinates discovery across providers.
+
+### DI
+
+Hilt with `@HiltAndroidApp`, `@AndroidEntryPoint`. DI modules in `app/di/`: AppModule, RepositoryModule, MediaProviderModule, ImageLoaderModule.
+
+## Build Configuration
+
+- **Kotlin 2.x**, **Java 17** (with core library desugaring for API 23+)
+- **Min SDK 23**, Target/Compile SDK 36
+- **ExoPlayer**: Custom build (`2.14.2-shuttle-16kb`) with FLAC/Opus extensions as local AARs
+- **Version catalog**: `gradle/libs.versions.toml`
+- **Versioning**: Defined in `buildSrc/src/main/kotlin/AppVersion.kt`
+- Debug builds use `.dev` app ID suffix
+- R8 full mode is disabled (Retrofit compatibility)
+
+## Code Style
+
+- KTLint with `android_studio` style (`.editorconfig`)
+- Composable functions exempt from naming rules
+- Property naming, filename, and package-name rules disabled
+- Git hooks available in `support/scripts/git/` for pre-commit lint and pre-push tests
+
+## Branch Conventions
+
+- Base branch: `dev`
+- Branch prefixes: `feature/`, `fix/`, `tech/`, `doc/`
+- All changes via PR to `main`; pushes to `main` trigger deployment to Google Play (internal track)
+
+## Testing
+
+### Compose UI Characterisation Tests
+
+Robolectric-based Compose tests that verify observable UI behaviour. These allow safe rearchitecting of Compose screens and ViewModels — if the UI still looks right, the tests pass.
+
+**Run them:**
+```bash
+./gradlew :android:app:testDebugUnitTest --tests "com.simplecityapps.shuttle.ui.screens.library.songs.SongListTest"
+./gradlew :android:app:testDebugUnitTest --tests "com.simplecityapps.shuttle.ui.screens.library.genres.GenreListTest"
+```
+
+**Configuration:** `android/app/src/test/resources/robolectric.properties` sets `sdk=34`, `graphics=NATIVE`, and `application=android.app.Application` (bypasses Hilt app init for fast, isolated tests).
+
+### Robot Pattern
+
+Each Compose screen has a **robot** that encapsulates selectors and interaction mechanics. Tests express *what* they verify, not *how* to find Compose nodes. When the implementation changes (test tags, content descriptions, layout structure), update the robot — not every test.
+
+**Files per screen:**
+```
+songs/
+  SongListTest.kt        # test cases
+  SongListRobot.kt       # selector/interaction encapsulation
+  SongListScenarios.kt   # ViewState factories
+```
+
+**Robot responsibilities:**
+- `setContent(viewState)` — renders the composable with callback captures
+- `assertTextDisplayed(text)` / `assertTextNotDisplayed(text)` — hides node selectors
+- `openContextMenu()` — hides content description selectors
+- `clickText(text)` / `clickMenuItem(text)` — interaction primitives
+- Callback capture fields (`lastAddedToQueue`, `lastDeleted`, etc.) — avoid verbose lambda setup in tests
+
+**Robot boundaries — keep it thin:**
+- The robot hides *selectors* (content descriptions, test tags, node matchers)
+- The robot does NOT hide *behaviour* — tests compose primitives to describe what they verify
+- Assertions use user-visible text, not implementation details
+- No screen-specific compound assertions like `assertContextMenuComplete()` — tests list what they expect
+
+**Example test:**
+```kotlin
+@Test
+fun `context menu invokes onAddToQueue`() {
+    val song = createSong(name = "Queue Me")
+    robot.setContent(readySongList(songs = listOf(song)))
+    robot.openContextMenu()
+    robot.clickMenuItem("Add to Queue")
+    robot.lastAddedToQueue shouldBe song
+}
+```
+
+### Scenario Factories
+
+Top-level functions that construct `ViewState` with sensible defaults. Reduce boilerplate without hiding what matters.
+
+```kotlin
+// SongListScenarios.kt
+readySongList(songs = listOf(createSong(name = "My Song")))
+scanningSongList(Progress(50, 200))
+emptySongList()
+loadingSongList  // val, not a function — Loading has no parameters
+```
+
+### Model Factories
+
+`createSong()`, `createGenre()`, `createPlaylist()` in `app/src/test/.../creationFunctions.kt`. All parameters have defaults — override only what matters for the test.
+
+### Adding a New Screen's Tests
+
+1. Create `*Robot.kt` — constructor takes `ComposeContentTestRule`, provides `setContent()`, selectors, callback captures
+2. Create `*Scenarios.kt` — top-level functions for each ViewState variant
+3. Create `*Test.kt` — `@RunWith(RobolectricTestRunner::class)`, instantiate robot from `composeTestRule`
+4. Add model factories to `creationFunctions.kt` if needed
+
+### Known Robolectric Limitations
+
+- **FastScroller + DropdownMenu:** The `FastScroller` overlay causes `DropdownMenu` popups to be immediately dismissed under Robolectric. Context menu tests that need dropdowns should render the list *item* composable directly (e.g. `GenreListItem`) rather than the full list. The robot encapsulates this — see `GenreListRobot.setItemContent()`.

android/app/src/main/java/com/simplecityapps/shuttle/ui/common/ComposeContextualToolbarHelper.kt

@@ -2,40 +2,15 @@ package com.simplecityapps.shuttle.ui.common
 
 import androidx.appcompat.widget.Toolbar
 import androidx.core.view.isVisible
-import com.simplecityapps.shuttle.model.MediaProviderType
-import com.simplecityapps.shuttle.model.Song
-import kotlin.collections.distinct
-import kotlin.collections.map
-import kotlinx.coroutines.flow.MutableStateFlow
-import kotlinx.coroutines.flow.asStateFlow
-import kotlinx.coroutines.flow.map
 import timber.log.Timber
 
-class ComposeContextualToolbarHelper {
+class ComposeContextualToolbarHelper<T>(
+    private val selectionState: SelectionState<T>,
+) {
 
     var toolbar: Toolbar? = null
     var contextualToolbar: Toolbar? = null
 
-    private val _selectedSongsState = MutableStateFlow(emptySet<Song>())
-    val selectedSongsState = _selectedSongsState.asStateFlow()
-    val selectedSongCountState = _selectedSongsState.asStateFlow()
-        .map { selectedSongs -> selectedSongs.size }
-
-    fun toggleSongSelection(song: Song) {
-        Timber.d("foo: toggleSongSelection: ${hashCode()}")
-        _selectedSongsState.value = if (_selectedSongsState.value.contains(song)) {
-            _selectedSongsState.value - song
-        } else {
-            _selectedSongsState.value + song
-        }
-    }
-
-    fun clearSelection() {
-        _selectedSongsState.value = emptySet()
-    }
-
-    fun isSelecting() = _selectedSongsState.value.isNotEmpty()
-
     fun show() {
         contextualToolbar?.let { contextualToolbar ->
             toolbar?.isVisible = false
@@ -50,11 +25,6 @@ class ComposeContextualToolbarHelper {
         toolbar?.isVisible = true
         contextualToolbar?.isVisible = false
         contextualToolbar?.setNavigationOnClickListener(null)
-        clearSelection()
+        selectionState.clear()
     }
-
-    fun selectedSongsMediaProviders(): List<MediaProviderType> = selectedSongsState
-        .value
-        .map { it.mediaProvider }
-        .distinct()
 }

android/app/src/main/java/com/simplecityapps/shuttle/ui/common/SelectionState.kt

@@ -0,0 +1,26 @@
+package com.simplecityapps.shuttle.ui.common
+
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.asStateFlow
+import kotlinx.coroutines.flow.map
+
+class SelectionState<T> {
+
+    private val _selectedItems = MutableStateFlow(emptySet<T>())
+    val selectedItems = _selectedItems.asStateFlow()
+    val selectedCount = _selectedItems.map { it.size }
+
+    fun toggle(item: T) {
+        _selectedItems.value = if (_selectedItems.value.contains(item)) {
+            _selectedItems.value - item
+        } else {
+            _selectedItems.value + item
+        }
+    }
+
+    fun clear() {
+        _selectedItems.value = emptySet()
+    }
+
+    fun isActive() = _selectedItems.value.isNotEmpty()
+}

android/app/src/main/java/com/simplecityapps/shuttle/ui/screens/library/songs/SongList.kt

@@ -151,7 +151,7 @@ private fun SongList(
         LazyColumn(
             modifier = Modifier
                 .fillMaxWidth()
-                .testTag("genres-list-lazy-column"),
+                .testTag("songs-list-lazy-column"),
             verticalArrangement = Arrangement.spacedBy(16.dp),
             contentPadding = PaddingValues(vertical = 16.dp, horizontal = 8.dp),
             state = state,