fix(music): review feedback round

- isPublic() now always returns !isPrivate() so a playlist tagged with
  both public=true and private=true is consistently reported as private,
  matching the 'isPrivate wins' contract documented on isPrivate().
- AddToMusicPlaylistViewModel + NewMusicPlaylistFab: hop to
  Dispatchers.Main.immediate for every Compose State write. The wrapping
  coroutines (rescan loop, launchSigner) run on Dispatchers.IO; Snapshot
  tolerates off-main writes but the codebase convention is main-only.
- MusicTracksSubAssembler + MusicPlaylistsSubAssembler: the single REQ
  asks both kinds 36787+34139, so the since cursor must be the min of
  both feeds' lastNoteCreatedAt to avoid over-fetching the lagging kind.
  Both assemblers now also listen to the other feed's cursor flow.
- Extract formatTrackDuration into MusicFormatting.kt; MusicTrack and
  MusicPlaylist share it.
- syntheticWaveformFor: replace inline FQN
  com.vitorpamplona.amethyst.service.playback.composable.WaveformData
  with an import.
- NewMusicPlaylistFab: drop the second .trim() — the dialog's confirm
  button already trims before invoking onCreate.

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/MusicFormatting.kt

@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2025 Vitor Pamplona
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+ * Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+package com.vitorpamplona.amethyst.ui.note.types
+
+/** `m:ss` formatting for a track length in seconds, shared by both MusicTrack and MusicPlaylist UI. */
+internal fun formatTrackDuration(seconds: Int): String {
+    val minutes = seconds / 60
+    val secs = seconds % 60
+    return "%d:%02d".format(minutes, secs)
+}

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/MusicPlaylist.kt

@@ -555,12 +555,6 @@ private fun PlaylistTag(text: String) {
     }
 }
 
-private fun formatTrackDuration(seconds: Int): String {
-    val minutes = seconds / 60
-    val secs = seconds % 60
-    return "%d:%02d".format(minutes, secs)
-}
-
 // ---------------------------------------------------------------------------
 // @Preview composables. Constructs a MusicPlaylistEvent plus a handful of
 // MusicTrackEvents it references, pushes them through LocalCache, then

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/MusicTrack.kt

@@ -62,6 +62,7 @@ import com.vitorpamplona.amethyst.service.playback.composable.GetVideoController
 import com.vitorpamplona.amethyst.service.playback.composable.LoadThumbAndThenVideoView
 import com.vitorpamplona.amethyst.service.playback.composable.PauseControllerWhenInBackground
 import com.vitorpamplona.amethyst.service.playback.composable.VideoView
+import com.vitorpamplona.amethyst.service.playback.composable.WaveformData
 import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.GetMediaItem
 import com.vitorpamplona.amethyst.ui.components.LoadNote
 import com.vitorpamplona.amethyst.ui.components.MyAsyncImage
@@ -297,7 +298,7 @@ fun MusicTrackHeader(
                     if (album != null && (released != null || duration != null || isExplicit)) MetaSeparator()
                     released?.let { MetaText(text = it) }
                     if (released != null && (duration != null || isExplicit)) MetaSeparator()
-                    duration?.let { MetaText(text = formatDuration(it)) }
+                    duration?.let { MetaText(text = formatTrackDuration(it)) }
                     if (duration != null && isExplicit) MetaSeparator()
                     if (isExplicit) ExplicitBadge()
                 }
@@ -432,12 +433,6 @@ private fun TopicChip(
     )
 }
 
-private fun formatDuration(seconds: Int): String {
-    val minutes = seconds / 60
-    val secs = seconds % 60
-    return "%d:%02d".format(minutes, secs)
-}
-
 private const val SYNTHETIC_WAVEFORM_SAMPLES = 96
 private const val TWO_PI = (Math.PI * 2).toFloat()
 
@@ -452,7 +447,7 @@ private const val TWO_PI = (Math.PI * 2).toFloat()
  * only varied by per-bar noise, which made every track look like the same slow-fade sine with
  * minor wiggle.
  */
-private fun syntheticWaveformFor(seed: String): com.vitorpamplona.amethyst.service.playback.composable.WaveformData {
+private fun syntheticWaveformFor(seed: String): WaveformData {
     // Fold the seed's 32-bit hash into a Long so two ids whose hashCode happens to collide
     // (rare for hex addresses but cheap to defend against) still differ via the bit shuffle.
     val rng = kotlin.random.Random((seed.hashCode().toLong() * 0x9E3779B97F4A7C15uL.toLong()) xor seed.length.toLong())
@@ -475,8 +470,7 @@ private fun syntheticWaveformFor(seed: String): com.vitorpamplona.amethyst.servi
             val noise = (rng.nextFloat() - 0.5f) * 2f * noiseStrength
             ((baseline + carrier + noise) * envelope).coerceIn(0.05f, 1.0f)
         }
-    return com.vitorpamplona.amethyst.service.playback.composable
-        .WaveformData(bars)
+    return WaveformData(bars)
 }
 
 // ---------------------------------------------------------------------------

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/music/AddToMusicPlaylistViewModel.kt

@@ -97,7 +97,7 @@ class AddToMusicPlaylistViewModel : ViewModel() {
         super.onCleared()
     }
 
-    private fun rescan() {
+    private suspend fun rescan() {
         val mePubKey = account.userProfile().pubkeyHex
         val target = trackAddress
         val results =
@@ -114,7 +114,12 @@ class AddToMusicPlaylistViewModel : ViewModel() {
                         containsTrack = target != null && tracks.any { it == target },
                     )
                 }.sortedByDescending { it.containsTrack }
-        ownedPlaylists.value = results
+        // Hop back to the main thread to publish the new list — Compose State writes go
+        // through Snapshot.apply, and doing them off-main is the anti-pattern flagged in
+        // prior reviews of this codebase even though Snapshot makes it not-yet-broken.
+        withContext(Dispatchers.Main.immediate) {
+            ownedPlaylists.value = results
+        }
     }
 
     /**
@@ -127,7 +132,7 @@ class AddToMusicPlaylistViewModel : ViewModel() {
         val targetTrack = trackAddress ?: return false
         return mutationLock.withLock {
             if (isWorking.value) return@withLock false
-            isWorking.value = true
+            setWorking(true)
             try {
                 val existing = LocalCache.addressables.get(playlistAddress)?.event as? MusicPlaylistEvent ?: return@withLock false
 
@@ -139,10 +144,10 @@ class AddToMusicPlaylistViewModel : ViewModel() {
                     }
 
                 account.signAndComputeBroadcast(template)
-                withContext(Dispatchers.IO) { rescan() }
+                rescan()
                 true
             } finally {
-                isWorking.value = false
+                setWorking(false)
             }
         }
     }
@@ -153,7 +158,7 @@ class AddToMusicPlaylistViewModel : ViewModel() {
         if (title.isBlank()) return null
         return mutationLock.withLock {
             if (isWorking.value) return@withLock null
-            isWorking.value = true
+            setWorking(true)
             try {
                 val event =
                     account.signAndComputeBroadcast(
@@ -162,11 +167,23 @@ class AddToMusicPlaylistViewModel : ViewModel() {
                             tracks = listOf(targetTrack),
                         ),
                     )
-                withContext(Dispatchers.IO) { rescan() }
+                rescan()
                 event.address()
             } finally {
-                isWorking.value = false
+                setWorking(false)
             }
         }
     }
+
+    /**
+     * Publish the [isWorking] flag on the main thread. See the comment in [rescan]: this
+     * coroutine is launched on [Dispatchers.IO] (via `launchSigner`), and Compose State
+     * writes belong on the main dispatcher even when Snapshot would technically tolerate
+     * them off-main.
+     */
+    private suspend fun setWorking(working: Boolean) {
+        withContext(Dispatchers.Main.immediate) {
+            isWorking.value = working
+        }
+    }
 }

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/music/NewMusicPlaylistFab.kt

@@ -48,6 +48,8 @@ import com.vitorpamplona.amethyst.ui.theme.Size26Modifier
 import com.vitorpamplona.amethyst.ui.theme.Size55Modifier
 import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonRow
 import com.vitorpamplona.quartz.experimental.music.playlist.MusicPlaylistEvent
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
 
 @Composable
 fun NewMusicPlaylistFab(accountViewModel: AccountViewModel) {
@@ -72,12 +74,17 @@ fun NewMusicPlaylistFab(accountViewModel: AccountViewModel) {
             onDismiss = { dialogOpen = false },
             onCreate = { name ->
                 // Empty playlist: user adds tracks later via the "Add to playlist" sheet on
-                // any music-track note.
+                // any music-track note. `name` is already trimmed by the dialog's confirm
+                // button, no need to .trim() again here.
                 accountViewModel.launchSigner {
                     accountViewModel.account.signAndComputeBroadcast(
-                        MusicPlaylistEvent.build(title = name.trim()),
+                        MusicPlaylistEvent.build(title = name),
                     )
-                    dialogOpen = false
+                    // Compose state writes belong on the main dispatcher — launchSigner is
+                    // Dispatchers.IO, so we hop back before flipping the dialog flag.
+                    withContext(Dispatchers.Main.immediate) {
+                        dialogOpen = false
+                    }
                 }
             },
         )

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/music/datasource/MusicPlaylistsSubAssembler.kt

@@ -53,7 +53,15 @@ class MusicPlaylistsSubAssembler(
         since: SincePerRelayMap?,
     ): List<RelayBasedFilter> {
         val feedSettings = key.followsPerRelay()
-        return makeMusicTracksFilter(feedSettings, since, key.feedStates.musicPlaylistsFeed.lastNoteCreatedAtIfFilled())
+        // Same REQ asks both kinds (36787 + 34139), so the `since` cursor has to be the
+        // older of the two feeds' cursors — otherwise the kind whose feed lags would
+        // miss older events.
+        val defaultSince =
+            listOfNotNull(
+                key.feedStates.musicTracksFeed.lastNoteCreatedAtIfFilled(),
+                key.feedStates.musicPlaylistsFeed.lastNoteCreatedAtIfFilled(),
+            ).minOrNull()
+        return makeMusicTracksFilter(feedSettings, since, defaultSince)
     }
 
     override fun user(key: MusicPlaylistsQueryState) = key.account.userProfile()
@@ -91,6 +99,13 @@ class MusicPlaylistsSubAssembler(
                         invalidateFilters()
                     }
                 },
+                key.account.scope.launch(Dispatchers.IO) {
+                    // REQ also asks for tracks (kind 36787), so pagination needs to react
+                    // to the tracks feed's cursor advancing too.
+                    key.feedStates.musicTracksFeed.lastNoteCreatedAtWhenFullyLoaded.sample(5000).collectLatest {
+                        invalidateFilters()
+                    }
+                },
             )
 
         return super.newSub(key)

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/music/datasource/MusicTracksSubAssembler.kt

@@ -43,7 +43,15 @@ class MusicTracksSubAssembler(
         since: SincePerRelayMap?,
     ): List<RelayBasedFilter> {
         val feedSettings = key.followsPerRelay()
-        return makeMusicTracksFilter(feedSettings, since, key.feedStates.musicTracksFeed.lastNoteCreatedAtIfFilled())
+        // One REQ asks both kinds (36787 + 34139), so the `since` cursor must cover both
+        // feeds — pick the older of the two `lastNoteCreatedAt`s so neither feed misses
+        // older events. If either side hasn't paged yet (null), the union is null too.
+        val defaultSince =
+            listOfNotNull(
+                key.feedStates.musicTracksFeed.lastNoteCreatedAtIfFilled(),
+                key.feedStates.musicPlaylistsFeed.lastNoteCreatedAtIfFilled(),
+            ).minOrNull()
+        return makeMusicTracksFilter(feedSettings, since, defaultSince)
     }
 
     override fun user(key: MusicTracksQueryState) = key.account.userProfile()
@@ -81,6 +89,14 @@ class MusicTracksSubAssembler(
                         invalidateFilters()
                     }
                 },
+                key.account.scope.launch(Dispatchers.IO) {
+                    // The REQ also asks for playlists (kind 34139), so pagination needs to
+                    // listen to the playlists feed's cursor as well — otherwise an older
+                    // playlist that should be fetched stays out of view.
+                    key.feedStates.musicPlaylistsFeed.lastNoteCreatedAtWhenFullyLoaded.sample(5000).collectLatest {
+                        invalidateFilters()
+                    }
+                },
             )
 
         return super.newSub(key)

quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/music/playlist/MusicPlaylistEvent.kt

@@ -25,7 +25,6 @@ import com.vitorpamplona.quartz.experimental.music.playlist.tags.CollaborativeTa
 import com.vitorpamplona.quartz.experimental.music.playlist.tags.DescriptionTag
 import com.vitorpamplona.quartz.experimental.music.playlist.tags.ImageTag
 import com.vitorpamplona.quartz.experimental.music.playlist.tags.PrivateTag
-import com.vitorpamplona.quartz.experimental.music.playlist.tags.PublicTag
 import com.vitorpamplona.quartz.experimental.music.playlist.tags.TitleTag
 import com.vitorpamplona.quartz.experimental.music.track.MusicTrackEvent
 import com.vitorpamplona.quartz.nip01Core.core.Address
@@ -68,7 +67,13 @@ class MusicPlaylistEvent(
      */
     fun isPrivate() = tags.firstNotNullOfOrNull(PrivateTag::parse) ?: false
 
-    fun isPublic() = tags.firstNotNullOfOrNull(PublicTag::parse) ?: !isPrivate()
+    /**
+     * Always the inverse of [isPrivate] so a playlist that tags both `public=true` AND
+     * `private=true` still gets reported as private — matching the doc on [isPrivate]
+     * ("isPrivate wins"). If we honored a standalone `public` tag here, the two methods
+     * would contradict each other when both tags are present.
+     */
+    fun isPublic() = !isPrivate()
 
     fun isCollaborative() = tags.firstNotNullOfOrNull(CollaborativeTag::parse) ?: false