refactor: improve quality menu UX with descending sort, mirror disambiguation, and stable index-based selection

Author: alvarorichard

internal/playback/common.go

@@ -10,12 +10,10 @@ import (
 	"sync"
 	"time"
 
-	"charm.land/huh/v2/spinner"
 	"github.com/alvarorichard/Goanime/internal/api"
 	"github.com/alvarorichard/Goanime/internal/api/providers/metadata"
 	"github.com/alvarorichard/Goanime/internal/models"
 	"github.com/alvarorichard/Goanime/internal/player"
-	"github.com/alvarorichard/Goanime/internal/tui"
 	"github.com/alvarorichard/Goanime/internal/util"
 )
 
@@ -65,37 +63,39 @@ func PlayEpisode(
 		}
 	}
 
-	// Fetch episode metadata and stream URL in parallel under a single spinner
-	// GetEpisodeData (Jikan/AniList metadata) and GetVideoURLForEpisodeEnhanced (scraper)
-	// are independent operations — running them concurrently saves a full round-trip
+	// Fetch episode metadata and stream URL in parallel.
+	//
+	// 2026-04-28: removed the huh/v2 Bubble Tea spinner that previously
+	// wrapped this block. GetVideoURLForEpisodeEnhanced may invoke a
+	// tcell-based fuzzyfinder quality picker (AnimeFire's multi-quality
+	// response). The Bubble Tea spinner and tcell racing for stdin/stdout
+	// caused two user-visible bugs: arrow keys needed multiple presses to
+	// register (input contention) and the spinner's redraw clipped the
+	// first character of the picker's prompt ("S" of "Select"). A static
+	// log line is the smaller evil — animation is a nice-to-have, the
+	// picker working is not.
+	util.Infof("Loading episode...")
+
 	var videoURL string
 	var videoErr error
-	currentEpisodeCopy := currentEpisode // capture for goroutine
-
-	_ = tui.RunClean(func() error {
-		return spinner.New().
-			Title("Loading episode...").
-			Type(spinner.Dots).
-			Action(func() {
-				var wg sync.WaitGroup
-				wg.Add(2)
-
-				go func() {
-					defer wg.Done()
-					if err := api.GetEpisodeData(anime.MalID, episodeNum, anime); err != nil {
-						util.Debugf("Error fetching episode data: %v", err)
-					}
-				}()
-
-				go func() {
-					defer wg.Done()
-					videoURL, videoErr = player.GetVideoURLForEpisodeEnhanced(currentEpisodeCopy, anime)
-				}()
-
-				wg.Wait()
-			}).
-			Run()
-	})
+	currentEpisodeCopy := currentEpisode
+
+	var wg sync.WaitGroup
+	wg.Add(2)
+
+	go func() {
+		defer wg.Done()
+		if err := api.GetEpisodeData(anime.MalID, episodeNum, anime); err != nil {
+			util.Debugf("Error fetching episode data: %v", err)
+		}
+	}()
+
+	go func() {
+		defer wg.Done()
+		videoURL, videoErr = player.GetVideoURLForEpisodeEnhanced(currentEpisodeCopy, anime)
+	}()
+
+	wg.Wait()
 
 	if videoErr != nil {
 		// Any video URL failure means the episode is not available on this source.

internal/playback/movie.go

@@ -10,12 +10,10 @@ import (
 	"sync"
 	"time"
 
-	"charm.land/huh/v2/spinner"
 	"github.com/alvarorichard/Goanime/internal/api"
 	"github.com/alvarorichard/Goanime/internal/discord"
 	"github.com/alvarorichard/Goanime/internal/models"
 	"github.com/alvarorichard/Goanime/internal/player"
-	"github.com/alvarorichard/Goanime/internal/tui"
 	"github.com/alvarorichard/Goanime/internal/util"
 )
 
@@ -40,16 +38,10 @@ func HandleMovie(anime *models.Anime, episodes []models.Episode, discordEnabled
 		var videoURL string
 		var videoErr error
 
-		// Use spinner while fetching video URL
-		_ = tui.RunClean(func() error {
-			return spinner.New().
-				Title("Loading video stream...").
-				Type(spinner.Dots).
-				Action(func() {
-					videoURL, videoErr = player.GetVideoURLForEpisodeEnhanced(&episodes[0], anime)
-				}).
-				Run()
-		})
+		// Use static log instead of spinner while fetching video URL
+		// to avoid terminal UI contention if a quality picker opens.
+		util.Infof("Loading video stream...")
+		videoURL, videoErr = player.GetVideoURLForEpisodeEnhanced(&episodes[0], anime)
 
 		if videoErr != nil {
 			log.Printf("Failed to extract video URL: %v", util.ErrorHandler(videoErr))

internal/player/animefire_quality_menu_test.go

@@ -0,0 +1,191 @@
+// Package player — regression tests for the AnimeFire quality picker.
+//
+// 2026-04-28: the AnimeFire quality picker (extractActualVideoURL inline)
+// had several visible UX bugs surfaced by a screenshot from the user:
+//
+//  1. items were displayed in whatever order AnimeFire returned them, so
+//     users could see 360p above 1080p depending on API order;
+//  2. labels like "FHD" / "HD" / empty rendered raw — confusing or empty
+//     menu rows;
+//  3. same-quality mirrors produced visually identical entries — the user
+//     had no way to tell them apart from inside the picker;
+//  4. Esc surfaced as a generic "failed to select quality" error instead
+//     of routing back to the menu.
+//
+// The fix moved the sort + label-rendering logic into
+// buildAnimeFireQualityItems and made the call site treat fuzzyfinder.ErrAbort
+// as ErrBackRequested. These tests pin the new contract for #1–#3.
+package player
+
+import (
+	"strings"
+	"testing"
+)
+
+// Bug 1 regression: descending sort independent of upstream order.
+func TestBuildAnimeFireQualityItems_SortsDescendingByParsedResolution(t *testing.T) {
+	t.Parallel()
+
+	in := []VideoData{
+		{Label: "360p", Src: "https://cdn/360.mp4"},
+		{Label: "1080p", Src: "https://cdn/1080.mp4"},
+		{Label: "720p", Src: "https://cdn/720.mp4"},
+	}
+	sorted, labels := buildAnimeFireQualityItems(in)
+
+	wantOrder := []string{"1080p", "720p", "360p"}
+	for i, w := range wantOrder {
+		if labels[i] != w {
+			t.Fatalf("labels[%d] = %q, want %q (full=%v)", i, labels[i], w, labels)
+		}
+		if sorted[i].Label != w {
+			t.Fatalf("sorted[%d].Label = %q, want %q", i, sorted[i].Label, w)
+		}
+	}
+}
+
+// AnimeFire occasionally serves non-numeric labels ("FHD", "HD", "SD").
+// Parsed resolution is unknown, so they must render with a usable
+// fallback — never an empty string and never the raw Src URL.
+func TestBuildAnimeFireQualityItems_RendersFriendlyLabelForNonNumeric(t *testing.T) {
+	t.Parallel()
+
+	in := []VideoData{
+		{Label: "FHD", Src: "https://cdn/fhd.mp4"},
+		{Label: "", Src: "https://cdn/blank.mp4"},
+		{Label: "HD", Src: "https://cdn/hd.mp4"},
+	}
+	_, labels := buildAnimeFireQualityItems(in)
+
+	for i, l := range labels {
+		if l == "" {
+			t.Fatalf("labels[%d] is empty — picker would render a blank row", i)
+		}
+		if strings.HasPrefix(l, "http") {
+			t.Fatalf("labels[%d] = %q — raw URL must never be shown", i, l)
+		}
+	}
+
+	// Empty label specifically must collapse to "Auto", not the URL.
+	foundAuto := false
+	for _, l := range labels {
+		if l == "Auto" {
+			foundAuto = true
+			break
+		}
+	}
+	if !foundAuto {
+		t.Fatalf("expected one entry to render as 'Auto' for the empty label; got %v", labels)
+	}
+}
+
+// Bug 3 regression: same numeric quality across mirrors must produce
+// distinct labels — otherwise the index returned by the fuzzyfinder picks
+// the wrong source under the previous string-match logic.
+func TestBuildAnimeFireQualityItems_DisambiguatesSameQualityMirrors(t *testing.T) {
+	t.Parallel()
+
+	in := []VideoData{
+		{Label: "720p", Src: "https://mirror-a/720.mp4"},
+		{Label: "720p", Src: "https://mirror-b/720.mp4"},
+		{Label: "720p", Src: "https://mirror-c/720.mp4"},
+	}
+	sorted, labels := buildAnimeFireQualityItems(in)
+
+	uniq := map[string]struct{}{}
+	for _, l := range labels {
+		uniq[l] = struct{}{}
+	}
+	if len(uniq) != 3 {
+		t.Fatalf("labels must be unique to keep selection unambiguous, got %v", labels)
+	}
+	if labels[0] != "720p" {
+		t.Fatalf("first occurrence must keep bare label, got %q", labels[0])
+	}
+	for i := 1; i < 3; i++ {
+		if !strings.Contains(labels[i], "mirror") {
+			t.Fatalf("labels[%d] must include 'mirror' suffix, got %q", i, labels[i])
+		}
+	}
+
+	// Stable: order of equal-resolution items mirrors input order so
+	// the index-based selection in the call site picks the exact source
+	// the user highlighted.
+	wantSrc := []string{
+		"https://mirror-a/720.mp4",
+		"https://mirror-b/720.mp4",
+		"https://mirror-c/720.mp4",
+	}
+	for i, w := range wantSrc {
+		if sorted[i].Src != w {
+			t.Fatalf("sorted[%d].Src = %q, want %q (stable sort broken)", i, sorted[i].Src, w)
+		}
+	}
+}
+
+// Locks the alignment invariant the call site depends on: labels[i]
+// describes sorted[i], and indexing the user's chosen idx into sorted
+// returns the exact VideoData they picked.
+func TestBuildAnimeFireQualityItems_LengthsAndAlignment(t *testing.T) {
+	t.Parallel()
+
+	in := []VideoData{
+		{Label: "1080p", Src: "https://cdn/1080.mp4"},
+		{Label: "720p", Src: "https://m1/720.mp4"},
+		{Label: "720p", Src: "https://m2/720.mp4"},
+		{Label: "FHD", Src: "https://cdn/fhd.mp4"},
+		{Label: "", Src: "https://cdn/blank.mp4"},
+	}
+	sorted, labels := buildAnimeFireQualityItems(in)
+
+	if len(sorted) != len(labels) {
+		t.Fatalf("len(sorted)=%d must equal len(labels)=%d", len(sorted), len(labels))
+	}
+	if len(sorted) != len(in) {
+		t.Fatalf("len(sorted)=%d must preserve input cardinality (%d)", len(sorted), len(in))
+	}
+	for i := range sorted {
+		for j := i + 1; j < len(labels); j++ {
+			if labels[i] == labels[j] {
+				t.Fatalf("labels[%d] == labels[%d] == %q breaks index alignment", i, j, labels[i])
+			}
+		}
+	}
+}
+
+func TestBuildAnimeFireQualityItems_DoesNotMutateInput(t *testing.T) {
+	t.Parallel()
+
+	in := []VideoData{
+		{Label: "360p", Src: "https://cdn/360.mp4"},
+		{Label: "1080p", Src: "https://cdn/1080.mp4"},
+		{Label: "720p", Src: "https://cdn/720.mp4"},
+	}
+	snapshot := append([]VideoData(nil), in...)
+
+	_, _ = buildAnimeFireQualityItems(in)
+
+	for i := range snapshot {
+		if in[i] != snapshot[i] {
+			t.Fatalf("input mutated at %d: got %+v want %+v", i, in[i], snapshot[i])
+		}
+	}
+}
+
+func TestBuildAnimeFireQualityItems_EmptyAndSingle(t *testing.T) {
+	t.Parallel()
+
+	sorted, labels := buildAnimeFireQualityItems(nil)
+	if len(sorted) != 0 || len(labels) != 0 {
+		t.Fatalf("nil input must yield empty slices; got sorted=%v labels=%v", sorted, labels)
+	}
+
+	in := []VideoData{{Label: "1080p", Src: "https://cdn/1080.mp4"}}
+	sorted, labels = buildAnimeFireQualityItems(in)
+	if len(sorted) != 1 || sorted[0].Src != "https://cdn/1080.mp4" {
+		t.Fatalf("single source must be preserved; got %+v", sorted)
+	}
+	if len(labels) != 1 || labels[0] != "1080p" {
+		t.Fatalf("single source must render bare label; got %v", labels)
+	}
+}