From 96b15b75e6b51f37db176ab39bb568130483720f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 30 Jun 2026 12:42:25 -0400 Subject: [PATCH 1/4] feat(web/diagnostics): default to recent 500, move time window to Advanced The diagnostics view already defaulted to the most recent 500 events (no window); make that the obvious path. Device/Kind stay primary; the start/end window + row cap move into a collapsed "Advanced filters" disclosure (auto-opens when a window is active) with a "Reset to recent 500" action. Caption now states whether you're seeing the recent default or a windowed slice. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01K55iTxn95BtshocgdE1shW --- web/src/routes/admin/diagnostics/+page.svelte | 181 +++++++++++------- 1 file changed, 110 insertions(+), 71 deletions(-) diff --git a/web/src/routes/admin/diagnostics/+page.svelte b/web/src/routes/admin/diagnostics/+page.svelte index f6fb0e34..12379cb9 100644 --- a/web/src/routes/admin/diagnostics/+page.svelte +++ b/web/src/routes/admin/diagnostics/+page.svelte @@ -36,13 +36,26 @@ } } + // Default view = the most recent N events, no time window. The + // start/end window is an Advanced affordance. + const DEFAULT_LIMIT = 500; + // Filter state. let accountId = $state(''); let clientId = $state(''); let kind = $state(''); let fromLocal = $state(''); let toLocal = $state(''); - let limit = $state(500); + let limit = $state(DEFAULT_LIMIT); + + // True when an explicit time window is set (drives the caption wording). + const hasWindow = $derived(Boolean(fromLocal || toLocal)); + + function resetWindow() { + fromLocal = ''; + toLocal = ''; + limit = DEFAULT_LIMIT; + } // datetime-local (browser-local, no tz) → RFC3339 UTC the API accepts. function toRfc(v: string): string | undefined { @@ -191,77 +204,97 @@ - -
- - - - - + +
+
+ + -
- - +
+ + +
+ + +
+ + Advanced filters{hasWindow ? ' · window active' : ''} + +
+ + + + +
+
@@ -276,7 +309,13 @@

{:else}
- {rows.length} events (oldest first) + + {#if hasWindow} + {rows.length} events in window (oldest first) + {:else} + Most recent {rows.length} events (oldest first) · set a time window under Advanced + {/if} +
    {#each rows as r (r.id)} From 79f2d79a2ed02660f0cd80dc33d5b9aaa58d6e66 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 30 Jun 2026 16:33:01 -0400 Subject: [PATCH 2/4] feat(diagnostics): 'playback' kind, newest-first sort, fix active-route subtitle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Relabel (#1204): route + player_state events fire for every output route, not just UPnP — split them into a new 'playback' kind; 'upnp_sync' now means genuinely UPnP/Sonos signal (drops, resync). Migration 0037 adds 'playback' to the kind CHECK; server whitelist, Android reporter labels, and the web kind filter updated. Web sort: the diagnostics list gains a Newest/Oldest-first sort (default newest at top); export follows the displayed order. Fix (#1205): OutputRoute.isConnected was derived from RouteInfo.connectionState, which stays DISCONNECTED for local SYSTEM routes even when active — so a connected Bluetooth device showed "Available" and reported connected:false. The picker subtitle now uses isSelected (route == selected route); the dead isConnected field is removed and the misleading `connected` field dropped from the diagnostics route event (it only ever logs the active route). Refs Scribe M9 (#119), tasks #1204 #1205. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01K55iTxn95BtshocgdE1shW --- .../diagnostics/DiagnosticsReporter.kt | 9 +++-- .../player/output/OutputPickerSheet.kt | 9 +++-- .../minstrel/player/output/OutputRoute.kt | 21 +++++------ internal/api/diagnostics.go | 3 +- .../0037_diagnostics_playback_kind.down.sql | 9 +++++ .../0037_diagnostics_playback_kind.up.sql | 19 ++++++++++ web/src/routes/admin/diagnostics/+page.svelte | 36 ++++++++++++++++--- 7 files changed, 82 insertions(+), 24 deletions(-) create mode 100644 internal/db/migrations/0037_diagnostics_playback_kind.down.sql create mode 100644 internal/db/migrations/0037_diagnostics_playback_kind.up.sql diff --git a/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt b/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt index fe827015..d804b348 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt @@ -147,11 +147,13 @@ class DiagnosticsReporter @Inject constructor( } private suspend fun collectPlayerState() { + // 'playback', not 'upnp_sync' — player state applies to every output + // route (phone speaker, Bluetooth, UPnP), not just casting. playerController.uiState .map { Triple(it.currentSource, it.isUpnpLoading, it.playbackError) } .distinctUntilChanged() .collect { (source, upnpLoading, err) -> - record("upnp_sync", buildJsonObject { + record("playback", buildJsonObject { put("event", "player_state") put("source", source ?: "") put("upnp_loading", upnpLoading) @@ -161,14 +163,15 @@ class DiagnosticsReporter @Inject constructor( } private suspend fun collectRoutes() { + // 'playback' — route changes happen for all outputs. This only ever + // logs the ACTIVE route (routesState.current), so no "connected" flag. outputPicker.routesState.map { it.current }.distinctUntilChanged().collect { r -> - record("upnp_sync", buildJsonObject { + record("playback", buildJsonObject { put("event", "route") put("id", r.id) put("name", r.name) put("kind", r.kind.name) put("protocol", r.protocol.name) - put("connected", r.isConnected) }) } } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerSheet.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerSheet.kt index 4a9039c3..5f31d464 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerSheet.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerSheet.kt @@ -133,7 +133,7 @@ private fun RouteRow( maxLines = 2, overflow = TextOverflow.Ellipsis, ) - val subtitle = route.description ?: defaultSubtitle(route) + val subtitle = route.description ?: defaultSubtitle(route, isSelected) Text( text = subtitle, style = MaterialTheme.typography.bodySmall, @@ -198,10 +198,13 @@ private fun MulticastHintRow() { } } -private fun defaultSubtitle(route: OutputRoute): String = when (route.kind) { +// isSelected (route == the active/selected route) drives the Bluetooth +// "Connected" subtitle. RouteInfo.connectionState can't — it stays +// DISCONNECTED for local SYSTEM routes even when they're in use. +private fun defaultSubtitle(route: OutputRoute, isSelected: Boolean): String = when (route.kind) { OutputRoute.Kind.BuiltIn -> "Phone speaker" OutputRoute.Kind.Wired -> "Wired" - OutputRoute.Kind.Bluetooth -> if (route.isConnected) "Connected" else "Available" + OutputRoute.Kind.Bluetooth -> if (isSelected) "Connected" else "Available" OutputRoute.Kind.Cast -> "Cast" OutputRoute.Kind.Other -> "Available" } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputRoute.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputRoute.kt index 7b7944f2..3baecd30 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputRoute.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputRoute.kt @@ -21,7 +21,6 @@ data class OutputRoute( val description: String?, val kind: Kind, val protocol: Protocol, - val isConnected: Boolean, ) { enum class Kind { BuiltIn, Wired, Bluetooth, Cast, Other } @@ -43,9 +42,11 @@ data class OutputRoute( /** * Lift a MediaRouter [route] into the domain model. Kind is * inferred from [MediaRouter.RouteInfo.getDeviceType]; unknown - * device types fall through to [Kind.Other]. The - * `connectionState` proxy is good enough for the chip's - * "Connected"/"Available" subtitle. + * device types fall through to [Kind.Other]. "Active" is NOT an + * intrinsic of the route — the caller derives it by comparing to + * the selected route (RouteSnapshot.current), because + * RouteInfo.connectionState only reflects remote-route handshakes + * and stays DISCONNECTED for local SYSTEM routes even when in use. */ fun fromRouteInfo(route: MediaRouter.RouteInfo): OutputRoute { val kind = when (route.deviceType) { @@ -58,15 +59,12 @@ data class OutputRoute( MediaRouter.RouteInfo.DEVICE_TYPE_SPEAKER -> Kind.Other else -> Kind.Other } - val connected = - route.connectionState == MediaRouter.RouteInfo.CONNECTION_STATE_CONNECTED return OutputRoute( id = route.id, name = route.name, description = route.description, kind = kind, protocol = Protocol.SYSTEM, - isConnected = connected, ) } @@ -76,10 +74,10 @@ data class OutputRoute( * network speakers into the same `OutputPickerController` * routes stream the system routes come through. * - * `isConnected = false` because UPnP devices have no - * MediaRouter connection-state concept — they're always - * "available" on the LAN, and the picker's selected-route - * rendering handles the "currently playing" indicator. + * UPnP devices have no MediaRouter connection-state concept — + * they're always "available" on the LAN, and the picker's + * selected-route rendering handles the "currently playing" + * indicator. * * Subtitle is `manufacturer modelName` joined by a single * space, falling back to "Network speaker" when both fields @@ -96,7 +94,6 @@ data class OutputRoute( description = description, kind = Kind.Other, protocol = Protocol.UPNP, - isConnected = false, ) } } diff --git a/internal/api/diagnostics.go b/internal/api/diagnostics.go index 23dbfe92..83ec7cb2 100644 --- a/internal/api/diagnostics.go +++ b/internal/api/diagnostics.go @@ -21,7 +21,8 @@ import ( // variants do NOT require a new category here. var validDiagnosticKinds = map[string]struct{}{ "connectivity": {}, - "upnp_sync": {}, + "upnp_sync": {}, // genuinely UPnP/Sonos-specific (drops, resync) + "playback": {}, // route + player-state, any output (phone/BT/UPnP) "power": {}, "lifecycle": {}, "heartbeat": {}, diff --git a/internal/db/migrations/0037_diagnostics_playback_kind.down.sql b/internal/db/migrations/0037_diagnostics_playback_kind.down.sql new file mode 100644 index 00000000..10875ec9 --- /dev/null +++ b/internal/db/migrations/0037_diagnostics_playback_kind.down.sql @@ -0,0 +1,9 @@ +-- Drop rows using the new value first so they don't violate the restored +-- (narrower) constraint, then revert the whitelist. +DELETE FROM diagnostic_events WHERE kind = 'playback'; +ALTER TABLE diagnostic_events DROP CONSTRAINT diagnostic_events_kind_check; +ALTER TABLE diagnostic_events ADD CONSTRAINT diagnostic_events_kind_check + CHECK (kind IN ( + 'connectivity', 'upnp_sync', 'power', 'lifecycle', + 'heartbeat', 'http' + )); diff --git a/internal/db/migrations/0037_diagnostics_playback_kind.up.sql b/internal/db/migrations/0037_diagnostics_playback_kind.up.sql new file mode 100644 index 00000000..722d1dd6 --- /dev/null +++ b/internal/db/migrations/0037_diagnostics_playback_kind.up.sql @@ -0,0 +1,19 @@ +-- Add 'playback' to the diagnostic_events kind whitelist (M9 follow-up). +-- +-- `route` and `player_state` events fire for EVERY output route (phone +-- speaker, Bluetooth, UPnP/Sonos), so bucketing them under 'upnp_sync' +-- was misleading — an operator on Bluetooth earbuds saw "upnp_sync" +-- rows. 'upnp_sync' now means genuinely UPnP/Sonos-specific signal +-- (drops, and future resync/desync); general playback + route telemetry +-- moves to the new 'playback' kind. +-- +-- Per the enum-CHECK-whitelist rule, the new value lands by dropping and +-- re-adding the constraint in the same change. Existing rows tagged +-- 'upnp_sync' stay as-is (debug telemetry on a 30-day retention). + +ALTER TABLE diagnostic_events DROP CONSTRAINT diagnostic_events_kind_check; +ALTER TABLE diagnostic_events ADD CONSTRAINT diagnostic_events_kind_check + CHECK (kind IN ( + 'connectivity', 'upnp_sync', 'power', 'lifecycle', + 'heartbeat', 'http', 'playback' + )); diff --git a/web/src/routes/admin/diagnostics/+page.svelte b/web/src/routes/admin/diagnostics/+page.svelte index 12379cb9..7224421b 100644 --- a/web/src/routes/admin/diagnostics/+page.svelte +++ b/web/src/routes/admin/diagnostics/+page.svelte @@ -22,11 +22,20 @@ const client = useQueryClient(); - const KINDS = ['connectivity', 'upnp_sync', 'power', 'lifecycle', 'heartbeat', 'http'] as const; + const KINDS = [ + 'connectivity', + 'playback', + 'upnp_sync', + 'power', + 'lifecycle', + 'heartbeat', + 'http' + ] as const; function kindLabel(k: string): string { switch (k) { case 'connectivity': return 'Connectivity'; + case 'playback': return 'Playback'; case 'upnp_sync': return 'UPnP sync'; case 'power': return 'Power'; case 'lifecycle': return 'Lifecycle'; @@ -82,10 +91,17 @@ const devicesQuery = $derived($devicesStore); const devices = $derived(devicesQuery.data ?? []); + // Display sort. The API returns newest-first; default the view to that + // (most recent at the top), with an Oldest-first option for reading a + // timeline top-to-bottom. Export follows whatever's displayed. + let sortOrder = $state<'newest' | 'oldest'>('newest'); + const diagStore = $derived(createAdminDiagnosticsQuery(filter)); const diagQuery = $derived($diagStore); - // API returns newest-first; reverse to chronological for a readable timeline. - const rows = $derived([...((diagQuery.data ?? []) as AdminDiagnostic[])].reverse()); + const rows = $derived.by(() => { + const data = [...((diagQuery.data ?? []) as AdminDiagnostic[])]; // newest-first + return sortOrder === 'oldest' ? data.reverse() : data; + }); function fmtTime(iso: string): string { const d = new Date(iso); @@ -232,6 +248,16 @@ {/each} +
    From cdfc79e6abaa8c33dd61ec87ab880df8c3ab629b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 30 Jun 2026 18:48:35 -0400 Subject: [PATCH 3/4] feat(android/diagnostics): per-skip track-change event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Heartbeats are 45s apart and missed a rapid skip burst (local_index 16→22 in one gap). Add a 'playback' track_change event emitted on each queue-index / current-track change, snapshotting local vs Sonos index+position + server_health + upnp_loading + route — so a transient skip-induced desync is captured at the instant it happens. (uiState is a conflated StateFlow, so a very rapid burst may coalesce intermediate indices; we still get the boundaries + the snapshot.) Refs Scribe M9 (#119), task #1210. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01K55iTxn95BtshocgdE1shW --- .../diagnostics/DiagnosticsReporter.kt | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt b/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt index d804b348..88897a9d 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt @@ -102,6 +102,7 @@ class DiagnosticsReporter @Inject constructor( launch { collectServerHealth() } launch { collectUpnpDrops() } launch { collectPlayerState() } + launch { collectTrackChanges() } launch { collectRoutes() } launch { heartbeatLoop() } } @@ -162,6 +163,30 @@ class DiagnosticsReporter @Inject constructor( } } + // Emit a snapshot at each track/queue-index change — captures a + // skip-induced local↔Sonos desync at the INSTANT it happens, which the + // 45s heartbeat misses. Note: uiState is a conflated StateFlow, so a + // very rapid skip burst may coalesce intermediate indices (we still get + // the boundaries + the local-vs-Sonos snapshot). + private suspend fun collectTrackChanges() { + playerController.uiState + .map { it.queueIndex to it.currentTrack?.id } + .distinctUntilChanged() + .collect { (index, _) -> + val ui = playerController.uiState.value + record("playback", buildJsonObject { + put("event", "track_change") + put("local_index", index) + put("local_pos_ms", ui.positionMs) + put("sonos_track", remoteState.trackNumber) + put("sonos_pos_ms", remoteState.positionMs) + put("upnp_loading", ui.isUpnpLoading) + put("server_health", networkStatus.state.value.name) + put("route", outputPicker.routesState.value.current.name) + }) + } + } + private suspend fun collectRoutes() { // 'playback' — route changes happen for all outputs. This only ever // logs the ACTIVE route (routesState.current), so no "connected" flag. From 392454b2495321613862e2497e45258e6f4e9e0a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 30 Jun 2026 19:15:12 -0400 Subject: [PATCH 4/4] feat(android/diagnostics): track-identity enrichment + zero stale Sonos state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Numeric indices wobble across re-casts (offset +1↔0 seen during output toggling), making "same track?" ambiguous. Enrich both the track_change event and the heartbeat with local_track_id (TrackRef.id) and sonos_uri (RemotePlayerState.currentTrackUri — the URL the speaker is actually streaming), so a desync is unambiguous. Also fixes the cast→phone stale-state pollution (#1211): sonos_* is now zeroed unless a remote route is active, via a shared putSonos() helper — so a just-ended cast's RemotePlayerState can't masquerade as live Sonos data in the diagnostics. Refs Scribe M9 (#119), tasks #1210 #1211. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01K55iTxn95BtshocgdE1shW --- .../diagnostics/DiagnosticsReporter.kt | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt b/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt index 88897a9d..142f5a2b 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/diagnostics/DiagnosticsReporter.kt @@ -174,12 +174,16 @@ class DiagnosticsReporter @Inject constructor( .distinctUntilChanged() .collect { (index, _) -> val ui = playerController.uiState.value + val casting = outputPicker.routesState.value.current.protocol != + OutputRoute.Protocol.SYSTEM record("playback", buildJsonObject { put("event", "track_change") put("local_index", index) + // Track IDENTITY, not just index — indices wobble across + // re-casts, so the id + Sonos URI make a desync unambiguous. + put("local_track_id", ui.currentTrack?.id ?: "") put("local_pos_ms", ui.positionMs) - put("sonos_track", remoteState.trackNumber) - put("sonos_pos_ms", remoteState.positionMs) + putSonos(this, casting) put("upnp_loading", ui.isUpnpLoading) put("server_health", networkStatus.state.value.name) put("route", outputPicker.routesState.value.current.name) @@ -214,13 +218,11 @@ class DiagnosticsReporter @Inject constructor( put("is_playing", ui.isPlaying) put("source", ui.currentSource ?: "") put("local_index", ui.queueIndex) + put("local_track_id", ui.currentTrack?.id ?: "") put("local_pos_ms", ui.positionMs) put("route", route.name) put("route_protocol", route.protocol.name) - // UPnP/Sonos remote-vs-local — the desync signal. - put("sonos_track", remoteState.trackNumber) - put("sonos_pos_ms", remoteState.positionMs) - put("sonos_playing", remoteState.isPlaying) + putSonos(this, routeActive) addPowerFields(this) }) } @@ -285,6 +287,18 @@ class DiagnosticsReporter @Inject constructor( addPowerFields(this) } + // Sonos/UPnP remote-vs-local desync fields. Only meaningful while a + // remote route is active; zeroed otherwise so a stale RemotePlayerState + // from a just-ended cast can't masquerade as live Sonos data (the + // cast→phone handoff artifact). currentTrackUri is the desync ground + // truth — it carries the track the speaker is actually streaming. + private fun putSonos(builder: kotlinx.serialization.json.JsonObjectBuilder, casting: Boolean) { + builder.put("sonos_track", if (casting) remoteState.trackNumber else 0) + builder.put("sonos_pos_ms", if (casting) remoteState.positionMs else 0) + builder.put("sonos_playing", casting && remoteState.isPlaying) + if (casting) builder.put("sonos_uri", remoteState.currentTrackUri) + } + private fun addPowerFields(builder: kotlinx.serialization.json.JsonObjectBuilder) { val pm = context.getSystemService(Context.POWER_SERVICE) as PowerManager builder.put("doze", pm.isDeviceIdleMode)