feat: confirm playback from the media session, and stop demoting the top row

Phase 3 DoD reached end to end: `artemis run spotify_play query="Rolling Stones Honky Tonk"` played the right song in 13 steps / 35.3s, with playback paused first so the pass proves the row tap started it. Two things stopped it before that, both found on the real phone: - The playback check asserted on an on-screen "Pause" label that Spotify never shows: its transport button keeps the label "Play" while music is playing. The recipe now ends with a `playing` step that reads the app's media session (state PLAYING) and confirms the track title contains the query words. A failure reports the state and the track it found. - tap_row pressed the wrong row. Once a track is loaded its row carries a `Paused` state indicator, and counting that content description as row text dropped the top hit below an identical duplicate further down, so the tap went to y=1443 instead of y=878. Rows are now scored on their title and subtitle only (state lives in content descriptions), and scores within rowScoreTolerance are treated as equal so the higher row wins, because Spotify already ranks by relevance. internal/ui/rows_test.go reproduces that results screen, including the state indicator and the duplicate row. docs/phase-3-spotify-recipe.md records the successful run, the independent media-session and audio-output reads, and the failure that preceded it.
2026-10-16 14:32:24 +11:00
parent ab69f37aa4
commit c178f92851
10 changed files with 403 additions and 37 deletions
--- a/internal/ui/media.go
+++ b/internal/ui/media.go
@@ -0,0 +1,116 @@
+package ui
+
+import (
+	"fmt"
+	"regexp"
+	"strings"
+
+	"gitea.lab.audasmedia.com.au/sam/artemis_control/internal/adb"
+)
+
+// Playback is what an app's media session says it is doing.
+//
+// The screen is not a reliable place to read this. Spotify's play/pause button
+// keeps the accessibility label "Play" while music is playing, so a recipe that
+// asserted on the label failed while the phone was busy playing the track. The
+// media session is the authoritative answer, and it carries the track title.
+type Playback struct {
+	Package string
+	State   string // PLAYING, PAUSED, STOPPED, NONE, ...
+	Title   string // the metadata description, usually "title, artist, album"
+}
+
+// Playing reports whether anything is being played.
+func (p *Playback) Playing() bool {
+	return p != nil && strings.EqualFold(p.State, "PLAYING")
+}
+
+// String is the one-line form used in log lines and error messages.
+func (p *Playback) String() string {
+	if p == nil {
+		return "no media session"
+	}
+	if p.Title == "" {
+		return fmt.Sprintf("state=%s", p.State)
+	}
+	return fmt.Sprintf("state=%s, playing %q", p.State, p.Title)
+}
+
+var (
+	playbackStateRe = regexp.MustCompile(`state=PlaybackState \{state=([A-Z]+)`)
+	metadataRe      = regexp.MustCompile(`metadata:.*description=(.*)$`)
+)
+
+// PlaybackState reads the media session of one package.
+//
+// It returns an error only when the dump cannot be read; an app that is simply
+// not playing comes back with a state of PAUSED or NONE.
+func PlaybackState(r *adb.Runner, pkg string) (*Playback, error) {
+	if pkg == "" {
+		return nil, fmt.Errorf("playback check needs a package name")
+	}
+	out, err := r.Shell("dumpsys media_session")
+	if err != nil {
+		return nil, fmt.Errorf("read media sessions: %w", err)
+	}
+	return parsePlayback(out, pkg), nil
+}
+
+// parsePlayback finds the block for pkg and reads its state and metadata. A
+// session block is delimited by its "package=" line, so state and metadata are
+// only read from the block that belongs to the package.
+func parsePlayback(dump, pkg string) *Playback {
+	var found *Playback
+	var cur *Playback
+
+	for _, raw := range strings.Split(dump, "\n") {
+		line := strings.TrimSpace(raw)
+		if name, ok := strings.CutPrefix(line, "package="); ok {
+			if cur != nil && cur.Package == pkg && found == nil {
+				found = cur
+			}
+			cur = &Playback{Package: strings.TrimSpace(name)}
+			continue
+		}
+		if cur == nil {
+			continue
+		}
+		if m := playbackStateRe.FindStringSubmatch(line); m != nil {
+			cur.State = m[1]
+			continue
+		}
+		if m := metadataRe.FindStringSubmatch(line); m != nil && cur.Title == "" {
+			cur.Title = strings.TrimSpace(m[1])
+		}
+	}
+	if cur != nil && cur.Package == pkg && found == nil {
+		found = cur
+	}
+	if found == nil {
+		return &Playback{Package: pkg, State: "NONE"}
+	}
+	if found.State == "" {
+		found.State = "NONE"
+	}
+	return found
+}
+
+// MatchesText reports how many of the query's words appear in text. It is used
+// to confirm that the session is playing the song the recipe asked for.
+func MatchesText(text, query string) (matched, total int) {
+	want := tokenize(query)
+	if len(want) == 0 {
+		return 0, 0
+	}
+	have := map[string]bool{}
+	for _, w := range tokenize(text) {
+		have[w] = true
+	}
+	seen := map[string]bool{}
+	for _, w := range want {
+		if have[w] {
+			seen[w] = true
+		}
+	}
+	return len(seen), len(want)
+}
--- a/internal/ui/media_test.go
+++ b/internal/ui/media_test.go
@@ -0,0 +1,77 @@
+package ui
+
+import "testing"
+
+// A real (trimmed) `dumpsys media_session` capture, taken while Spotify was
+// playing the track the recipe asked for. Note the transport button on screen
+// still read "Play".
+const mediaSessionDump = `MEDIA SESSION SERVICE (dumpsys media_session)
+
+  12 sessions listeners:
+    1: com.spotify.music/SpotifySession/1 (userId=0)
+      package=com.spotify.music
+      launchIntent=null
+      active=true
+      flags=3
+      rating type=2
+      controllers: 12
+      state=PlaybackState {state=PLAYING(3), position=606, buffered position=0, speed=1.0, updated=1207066569, actions=3025843, custom actions=[Action:mName='Start radio, mIcon=2131233633, mExtras=Bundle[mParcelledData.dataSize=144]], active item id=18, error=null}
+      audioAttrs=AudioAttributes: usage=USAGE_MEDIA content=CONTENT_TYPE_UNKNOWN flags=0x800(FLAG_MUTE_HAPTIC)
+      volumeType=LOCAL, controlType=ABSOLUTE, max=0, current=0
+      metadata: size=18, description=Honky Tonk Women, The Rolling Stones, Honky Tonk Women / You Can't Always Get What You Want
+      queueTitle=Play Queue, size=50
+    2: org.kde.kdeconnect_tp.media_session org.kde.kdeconnect_tp/org.kde.kdeconnect_tp.media_session/630 (userId=0)
+      package=org.kde.kdeconnect_tp
+      active=false
+      state=PlaybackState {state=PAUSED(2), position=881, buffered position=0, speed=0.0, updated=1206521908, actions=260, custom actions=[], active item id=-1, error=null}
+      metadata: size=0, description=, ,
+`
+
+func TestPlaybackStateReadsTheRightSession(t *testing.T) {
+	got := parsePlayback(mediaSessionDump, "com.spotify.music")
+	if got == nil || !got.Playing() {
+		t.Fatalf("spotify playback = %+v", got)
+	}
+	if got.Title != "Honky Tonk Women, The Rolling Stones, Honky Tonk Women / You Can't Always Get What You Want" {
+		t.Fatalf("title = %q", got.Title)
+	}
+
+	// The block is delimited by its package line: Spotify's state must not leak
+	// into another package and vice versa.
+	other := parsePlayback(mediaSessionDump, "org.kde.kdeconnect_tp")
+	if other.Playing() {
+		t.Fatalf("kdeconnect playback = %+v, want paused", other)
+	}
+	if other.Title != ", ," && other.Title != "" {
+		t.Fatalf("kdeconnect title = %q", other.Title)
+	}
+
+	missing := parsePlayback(mediaSessionDump, "com.example.absent")
+	if missing.Playing() || missing.State != "NONE" {
+		t.Fatalf("missing package = %+v, want state NONE", missing)
+	}
+}
+
+func TestMatchesText(t *testing.T) {
+	const meta = "Honky Tonk Women, The Rolling Stones, Honky Tonk Women / You Can't Always Get What You Want"
+	if matched, total := MatchesText(meta, "Rolling Stones Honky Tonk"); matched != 4 || total != 4 {
+		t.Fatalf("matched %d/%d, want 4/4", matched, total)
+	}
+	if matched, _ := MatchesText(meta, "Chopin Nocturne"); matched != 0 {
+		t.Fatalf("matched %d for an unrelated query, want 0", matched)
+	}
+	if _, total := MatchesText(meta, ""); total != 0 {
+		t.Fatalf("empty query should have no words, got %d", total)
+	}
+}
+
+func TestPlaybackString(t *testing.T) {
+	p := &Playback{Package: "com.spotify.music", State: "PLAYING", Title: "Honky Tonk Women"}
+	if s := p.String(); s != `state=PLAYING, playing "Honky Tonk Women"` {
+		t.Fatalf("String() = %q", s)
+	}
+	var nilSession *Playback
+	if s := nilSession.String(); s != "no media session" {
+		t.Fatalf("nil String() = %q", s)
+	}
+}
--- a/internal/ui/rows.go
+++ b/internal/ui/rows.go
@@ -133,15 +133,22 @@ func (h *Hierarchy) rowCandidates(rowID string, screenW, screenH int) []*Node {
 	return out
 }

-// betterRow ranks rows: the best text match wins, and equal matches go to the
-// row nearest the top, because search results are ranked by relevance.
+// betterRow ranks rows. Spotify already ranks results by relevance, so a higher
+// row wins whenever the text match is comparable: the top hit can carry extra
+// state text ("Paused" on the row of the track that is already loaded) that
+// must not push it below a duplicate further down the list. Scores that are
+// further apart are not comparable, and the better text match wins.
 func betterRow(a, b *Row) bool {
-	if math.Abs(a.Score-b.Score) > 1e-9 {
+	if math.Abs(a.Score-b.Score) > rowScoreTolerance {
 		return a.Score > b.Score
 	}
 	return a.Top < b.Top
 }

+// rowScoreTolerance is how close two row scores must be before the higher row
+// is preferred.
+const rowScoreTolerance = 0.15
+
 func rowMatchesID(n *Node, rowID string) bool {
 	if rowID == "" {
 		return true
@@ -170,10 +177,12 @@ func textDescendants(n *Node) []string {
 // rowTexts returns the text that belongs to the row itself: its title and
 // subtitle.
 //
-// Anything inside a clickable child is an action button ("More options", "Add
-// to library", "Add suggestion"), not the row's own text, so that subtree is
-// skipped. This is what keeps a one-line search suggestion from looking like a
-// two-line result.
+// Two things are left out on purpose. Anything inside a clickable child is an
+// action button ("More options", "Add to library", "Add suggestion"), not the
+// row's own text, so that subtree is skipped. And content descriptions are not
+// counted, because that is where apps put *state* ("Paused", "Playing") rather
+// than the row's title; counting them made the row of the track that was
+// already loaded score worse than an identical duplicate further down.
 func rowTexts(n *Node) []string {
 	var out []string
 	seen := map[string]bool{}
@@ -192,9 +201,6 @@ func rowTexts(n *Node) []string {
 			return
 		}
 		add(c.Text)
-		if !c.Clickable {
-			add(c.ContentDesc)
-		}
 		for i := range c.Children {
 			visit(&c.Children[i], false)
 		}
--- a/internal/ui/rows_test.go
+++ b/internal/ui/rows_test.go
@@ -18,9 +18,12 @@ const spotifyRowDump = `<?xml version='1.0' encoding='UTF-8' standalone='yes' ?>
    </node>
    <node index="1" class="android.view.ViewGroup" resource-id="com.spotify.music:id/row_root" clickable="true" enabled="true" bounds="[0,794][1080,962]">
      <node index="0" class="android.widget.ImageView" resource-id="com.spotify.music:id/artwork" clickable="false" enabled="true" bounds="[42,815][168,941]"/>
-      <node index="1" class="android.widget.TextView" resource-id="com.spotify.music:id/title" text="Honky Tonk Women" clickable="false" enabled="true" bounds="[200,815][828,881]"/>
-      <node index="2" class="android.widget.TextView" resource-id="com.spotify.music:id/subtitle" text="Song • The Rolling Stones" clickable="false" enabled="true" bounds="[200,886][828,941]"/>
-      <node index="3" class="android.view.ViewGroup" resource-id="com.spotify.music:id/action_bar" clickable="false" enabled="true" bounds="[828,815][1080,941]">
+      <node index="1" class="android.view.ViewGroup" resource-id="com.spotify.music:id/play_indicator" clickable="false" enabled="true" bounds="[200,832][232,864]">
+        <node index="0" class="android.view.View" content-desc="Paused" clickable="false" enabled="true" bounds="[200,832][232,864]"/>
+      </node>
+      <node index="2" class="android.widget.TextView" resource-id="com.spotify.music:id/title" text="Honky Tonk Women" clickable="false" enabled="true" bounds="[243,815][828,881]"/>
+      <node index="3" class="android.widget.TextView" resource-id="com.spotify.music:id/subtitle" text="Song • The Rolling Stones" clickable="false" enabled="true" bounds="[200,886][828,941]"/>
+      <node index="4" class="android.view.ViewGroup" resource-id="com.spotify.music:id/action_bar" clickable="false" enabled="true" bounds="[828,815][1080,941]">
        <node index="0" class="android.widget.ImageButton" resource-id="com.spotify.music:id/context_menu_button" content-desc="More options for song Honky Tonk Women" clickable="true" enabled="true" bounds="[828,815][954,941]"/>
        <node index="1" class="android.widget.Button" resource-id="com.spotify.music:id/add_button" content-desc="Add Honky Tonk Women to My Library" clickable="true" enabled="true" bounds="[954,815][1080,941]"/>
      </node>
@@ -124,6 +127,46 @@ func TestRowScorePrefersPrecision(t *testing.T) {
 	}
 }

+// TestBestRowPrefersTheTopHitWhenItsRowCarriesStateText is the regression that
+// cost the most: the top hit's row carries a "Paused" state indicator once it is
+// the loaded track, and counting that description as row text pushed the score
+// below an identical duplicate further down the list. tap_row then pressed the
+// duplicate at y=1443 and playback never started.
+func TestBestRowPrefersTheTopHitWhenItsRowCarriesStateText(t *testing.T) {
+	h := spotifyRows(t)
+	row, err := h.BestRow("Rolling Stones Honky Tonk", spotifyRowID, 1080, 2400)
+	if err != nil {
+		t.Fatalf("BestRow: %v", err)
+	}
+	if row.Title != "Honky Tonk Women" {
+		t.Fatalf("chose %q", row.Title)
+	}
+	if row.Top != 794 {
+		t.Fatalf("chose the row at y=%d, want the top hit at y=794", row.Top)
+	}
+	if row.Centre.Y != (794+962)/2 {
+		t.Fatalf("tap point y=%d is not the top row's centre", row.Centre.Y)
+	}
+}
+
+func TestRowTextsIgnoreStateDescriptions(t *testing.T) {
+	h := spotifyRows(t)
+	var top *Node
+	h.Walk(func(n *Node) {
+		if n.ResourceID == spotifyRowID && n.Bounds == "[0,794][1080,962]" {
+			top = n
+		}
+	})
+	if top == nil {
+		t.Fatal("top row not found in the sample dump")
+	}
+	for _, line := range rowTexts(top) {
+		if line == "Paused" {
+			t.Fatal("the row's state description must not count as row text")
+		}
+	}
+}
+
 func TestRowTextsSkipActionButtons(t *testing.T) {
 	h := spotifyRows(t)