backend/config/config.go

@@ -411,9 +411,10 @@ func (c *Config) SetDownloadPreferences(prefs download.AutoDownloadPrefs) error
 		formats = append(formats, string(f))
 	}
 
-	c.Downloads.MinFileSizeMB = prefs.MinSizeMB
+	c.Downloads.MinKbps = prefs.MinKbps
+	c.Downloads.MaxKbps = prefs.MaxKbps
+	c.Downloads.PreferredKbps = prefs.PreferredKbps
 	c.Downloads.MaxFileSizeMB = prefs.MaxSizeMB
-	c.Downloads.PreferredFileSizeMB = prefs.PreferredSizeMB
 	c.Downloads.AllowedFormats = formats
 
 	if err := c.Save(); err != nil {

backend/download/config.go

@@ -34,13 +34,29 @@ type UserConfig struct {
 	// in one burst that every provider sees as a flood.
 	WantedBatch int `toml:"WantedBatch"`
 
-	// MinFileSizeMB, MaxFileSizeMB and PreferredFileSizeMB bound and
+	// MinKbps, MaxKbps and PreferredKbps bound and nudge what auto-pick
-	// nudge what auto-pick (interactive or via the request list) may
+	// (interactive or via the request list) may grab without asking.
-	// grab without asking.  Zero on any of them is permissive: see
+	// Zero on any of them is permissive: see AutoDownloadPrefs.
-	// AutoDownloadPrefs.
+	//
-	MinFileSizeMB       int `toml:"MinFileSizeMB"`
+	// They replaced MinFileSizeMB / MaxFileSizeMB /
-	MaxFileSizeMB       int `toml:"MaxFileSizeMB"`
+	// PreferredFileSizeMB, which were megabytes and so said nothing
-	PreferredFileSizeMB int `toml:"PreferredFileSizeMB"`
+	// without knowing how long the release was.  The old keys are
+	// deliberately *not* read back: a number that meant "300 MB" cannot
+	// be reinterpreted as a bitrate without knowing the album it was
+	// aimed at, so migrating it would be inventing an intent the user
+	// never expressed.  An existing config falls back to no window,
+	// which is the permissive default and matches a fresh install —
+	// and MaxFileSizeMB is the one that does carry over, because a
+	// ceiling on total bytes still means exactly what it did.
+	MinKbps       int `toml:"MinKbps"`
+	MaxKbps       int `toml:"MaxKbps"`
+	PreferredKbps int `toml:"PreferredKbps"`
+
+	// MaxFileSizeMB is a hard ceiling on a candidate's total size, kept
+	// in megabytes on purpose — it is a question about disk space, not
+	// about quality, and it has to apply to a candidate whose bitrate
+	// cannot be worked out at all.
+	MaxFileSizeMB int `toml:"MaxFileSizeMB"`
 
 	// AllowedFormats restricts auto-pick to these formats.  Empty means
 	// no restriction.  Values are Format strings ("flac", "mp3", ...).
@@ -56,10 +72,11 @@ func (c *UserConfig) AutoDownloadPrefs() AutoDownloadPrefs {
 	}
 
 	return AutoDownloadPrefs{
-		MinSizeMB:       c.MinFileSizeMB,
+		MinKbps:        c.MinKbps,
-		MaxSizeMB:       c.MaxFileSizeMB,
+		MaxKbps:        c.MaxKbps,
-		PreferredSizeMB: c.PreferredFileSizeMB,
+		PreferredKbps:  c.PreferredKbps,
-		AllowedFormats:  formats,
+		MaxSizeMB:      c.MaxFileSizeMB,
+		AllowedFormats: formats,
 	}
 }
 

backend/download/manager.go

@@ -236,6 +236,12 @@ func (m *Manager) AutoPickable(dl Download, ranked []Candidate) bool {
 	return AutoPickable(dl, ranked, m.preferences())
 }
 
+// AutoPickVeto wraps the package function the same way, and is what the
+// request list quotes back to the user.
+func (m *Manager) AutoPickVeto(dl Download, ranked []Candidate) string {
+	return AutoPickVeto(dl, ranked, m.preferences())
+}
+
 // Reload rebuilds every provider from stored config.  Called at startup
 // and after any provider settings change.
 //
@@ -612,16 +618,8 @@ func (m *Manager) Attempt(
 		return false, "", err
 	}
 
-	if !m.AutoPickable(dl, ranked) {
+	if veto := m.AutoPickVeto(dl, ranked); veto != "" {
-		best := ranked[0]
+		return false, veto, nil
-
-		return false, fmt.Sprintf(
-			"best of %d found is not a confident enough match "+
-				"(match %.0f%%, quality %.0f%%)",
-			len(ranked),
-			best.Match.Overall*100, //nolint:mnd // percent
-			best.Quality.Overall*100,
-		), nil
 	}
 
 	if err := m.store.CreateDownload(ctx, dl); err != nil {

backend/download/manager_test.go

@@ -218,8 +218,17 @@ func TestManagerEndToEndAutoPick(t *testing.T) {
 	}, "staging was never released, or the library was never rescanned")
 }
 
-// An ambiguous result set must park for the user rather than guess.
+// Two equally good copies are not an ambiguity — they are a spare.
-func TestManagerWaitsWhenAmbiguous(t *testing.T) {
+//
+// This asserted the opposite for as long as auto-pick required 0.08 of
+// daylight over the runner-up, and that rule was wrong in exactly the
+// case it fired hardest: a popular album turns up several *correct*
+// copies, all matching the tracklist, differing only in format and
+// seeders. There is no question there about what to fetch, only about
+// which copy, and the ranking already answers that — closest to the
+// preferred bitrate first. A candidate does not have to be better than
+// the field, only good enough on its own terms.
+func TestManagerAutoPicksAmongEquallyGoodCopies(t *testing.T) {
 	t.Parallel()
 
 	f := newManagerFixture(t)
@@ -237,11 +246,41 @@ func TestManagerWaitsWhenAmbiguous(t *testing.T) {
 		t.Fatalf("Start: %v", err)
 	}
 
-	if f.manager.AutoPickable(dl, ranked) {
+	if veto := f.manager.AutoPickVeto(dl, ranked); veto != "" {
-		t.Fatal("two equivalent candidates must not auto-pick")
+		t.Fatalf("two equally good copies must auto-pick, got veto: %s", veto)
+	}
+
+	waitForDownloadState(t, f.store, dl.ID, StateComplete)
+
+	// Exactly one of them was fetched, not both.
+	if grabs := a.GrabCalls + b.GrabCalls; grabs != 1 {
+		t.Errorf("grabs = %d, want exactly 1", grabs)
+	}
+}
+
+// The user can still pick explicitly when auto-pick is not what
+// happened — a candidate the ranking did not choose is still grabbable.
+func TestManagerPickIsExplicit(t *testing.T) {
+	t.Parallel()
+
+	f := newManagerFixture(t)
+
+	a := fakeWithAlbum(1, "source-a", ".flac")
+	b := fakeWithAlbum(2, "source-b", ".flac")
+
+	f.manager.installProvider(Config{ID: 1, Priority: 50}, a)
+	f.manager.installProvider(Config{ID: 2, Priority: 50}, b)
+
+	// No tracklist: never auto-picks, so the result set parks for the
+	// user and Pick is the only way anything is fetched.
+	dl := fourTrackDownload()
+	dl.Expected = nil
+
+	ranked, err := f.manager.Start(context.Background(), dl)
+	if err != nil {
+		t.Fatalf("Start: %v", err)
 	}
 
-	// Nothing was grabbed while waiting for the user.
 	if a.GrabCalls != 0 || b.GrabCalls != 0 {
 		t.Errorf(
 			"grabs happened without a pick: a=%d b=%d",
@@ -258,7 +297,6 @@ func TestManagerWaitsWhenAmbiguous(t *testing.T) {
 		t.Errorf("stored request id = %s, want %s", stored.ID, dl.ID)
 	}
 
-	// The user picks the second one explicitly.
 	if err := f.manager.Pick(
 		context.Background(), dl.ID, ranked[1].ID,
 	); err != nil {

backend/download/rank.go

@@ -1,6 +1,7 @@
 package download
 
 import (
+	"fmt"
 	"math"
 	"sort"
 	"strings"
@@ -34,38 +35,102 @@ const (
 	weightArtistFit    = 0.12
 )
 
-// Quality sub-weights.  They sum to 1.0 along with weightSizeFit below.
+// Quality sub-weights.  Each set sums to 1.0.
+//
+// There are two of them because a stated preference changes what the
+// other numbers are *for*.  `formatRank` and `bitrateScore` are the
+// app guessing at how good a copy is — FLAC over MP3, 320 over 128 —
+// and that guess exists precisely because the user has not said.  Once
+// they have, the guess should not outvote them: with the old single set
+// a preference of 320 kbps moved a candidate's score by at most 0.05
+// against the 0.42 riding on format, so asking for 320 and being handed
+// a FLAC every time was the *designed* behaviour.  That is the same
+// fault the megabyte window had — a preference the user can express and
+// the ranking can ignore.
 const (
-	weightFormat   = 0.42
+	weightFormat     = 0.42
-	weightBitrate  = 0.23
+	weightBitrate    = 0.23
-	weightHealth   = 0.20
+	weightHealth     = 0.20
-	weightPriority = 0.10
+	weightPriority   = 0.10
-	weightSizeFit  = 0.05
+	weightBitrateFit = 0.05
 )
 
+// Quality sub-weights when the user has named a preferred bitrate.
+// The weight comes off format and bitrate — the two proxies the
+// preference replaces — and health and priority are untouched, since
+// neither is a stand-in for anything the user just said.
+const (
+	statedWeightFormat     = 0.20
+	statedWeightBitrate    = 0.10
+	statedWeightHealth     = 0.20
+	statedWeightPriority   = 0.10
+	statedWeightBitrateFit = 0.40
+)
+
+// qualityWeights picks the set, in the order scoreQuality applies them.
+func qualityWeights(p AutoDownloadPrefs) (
+	format, bitrate, health, priority, fit float64,
+) {
+	if p.PreferredKbps > 0 {
+		return statedWeightFormat,
+			statedWeightBitrate,
+			statedWeightHealth,
+			statedWeightPriority,
+			statedWeightBitrateFit
+	}
+
+	return weightFormat,
+		weightBitrate,
+		weightHealth,
+		weightPriority,
+		weightBitrateFit
+}
+
 // unanchoredCap bounds the match score of a free-text request.  Without
 // an MBID there is no tracklist to be right about, so a confident-
 // looking score would be a lie — and auto-pick keys off this.
 const unanchoredCap = 0.65
 
 // AutoDownloadPrefs gates and scores what AutoPickable may choose
-// without asking.  Zero values are permissive: no size window and no
+// without asking.  Zero values are permissive: no bitrate window, no
-// format restriction.
+// size ceiling and no format restriction.
+//
+// **The window is a rate, not a size.**  It used to be three numbers in
+// megabytes, which cannot mean anything on their own: 300 MB is a
+// generous FLAC single and a suspiciously small boxset, and the user
+// setting the number has no idea which release the pipeline will
+// eventually apply it to.  A bitrate is the same statement normalised
+// by how long the music is, so one number holds across a 9-minute EP
+// and a 3-hour opera — and it is the unit the thing being described is
+// actually measured in.  The runtime is known for every request
+// auto-pick can act on (`Download.Expected` carries per-track lengths,
+// and an anchored request is the only kind that reaches here), so this
+// costs no extra lookup.
 type AutoDownloadPrefs struct {
-	// MinSizeMB and MaxSizeMB bound what auto-pick will grab.  Zero
+	// MinKbps and MaxKbps bound the average bitrate auto-pick will
-	// means no bound on that side.  A candidate outside the window is
+	// grab.  Zero means no bound on that side.  A candidate outside the
-	// filtered out of auto-pick entirely, not merely scored down — a
+	// window is filtered out of auto-pick entirely, not merely scored
-	// tiny "sampler" torrent or a boxset ten times the expected size is
+	// down — a 96 kbps rip of the right album is not a worse copy the
-	// usually the wrong thing entirely, not a worse copy of the right
+	// user might accept, it is one they said not to take unattended.
-	// thing.
+	//
-	MinSizeMB int `json:"minSizeMb"`
+	// For reference: 320 is the top of MP3, ~500–1000 is FLAC depending
-	MaxSizeMB int `json:"maxSizeMb"`
+	// on the material, and anything under ~128 is a transcode.
+	MinKbps int `json:"minKbps"`
+	MaxKbps int `json:"maxKbps"`
 
-	// PreferredSizeMB nudges the score toward a target size within the
+	// PreferredKbps nudges the score toward a target rate within the
-	// min/max window (a lossless rip and a heavily-padded lossless rip
+	// window, and breaks the tie when several candidates are equally
-	// can both pass the window).  Zero disables the nudge; sizeFit then
+	// good matches.  Zero disables the nudge; bitrateFit then returns a
-	// returns a neutral value that does not affect ranking.
+	// neutral value that does not affect ranking.
-	PreferredSizeMB int `json:"preferredSizeMb"`
+	PreferredKbps int `json:"preferredKbps"`
+
+	// MaxSizeMB is a hard ceiling on the whole candidate, and it is
+	// deliberately still a size.  It answers a different question from
+	// the window above — not "is this the quality I want" but "is this
+	// going to fill the disk" — and it has to hold even for a candidate
+	// whose bitrate cannot be worked out, which is exactly the shape a
+	// mislabelled boxset arrives in.  Zero means no ceiling.
+	MaxSizeMB int `json:"maxSizeMb"`
 
 	// AllowedFormats restricts auto-pick to candidates whose audio
 	// files are all in one of these formats.  Empty means no
@@ -74,19 +139,33 @@ type AutoDownloadPrefs struct {
 }
 
 // eligible reports whether a candidate may be auto-picked under these
-// preferences: within the size window (when set) and, when a format
+// preferences: inside the bitrate window and the size ceiling (when
-// list is given, every audio file in an allowed format.
+// set) and, when a format list is given, every audio file in an
-func (p AutoDownloadPrefs) eligible(c Candidate) bool {
+// allowed format.
+//
+// `runtimeMillis` is how long the requested release is, and 0 means
+// nobody knows.  An unknown runtime **passes** the bitrate window
+// rather than failing it: the window is a statement about quality, and
+// refusing everything the moment a tracklist is missing a length would
+// turn a gap in MusicBrainz into a silent embargo.  The size ceiling
+// still applies, which is why it exists separately.
+func (p AutoDownloadPrefs) eligible(c Candidate, runtimeMillis int64) bool {
 	const bytesPerMB = 1 << 20
 
-	if p.MinSizeMB > 0 && c.TotalSize < int64(p.MinSizeMB)*bytesPerMB {
-		return false
-	}
-
 	if p.MaxSizeMB > 0 && c.TotalSize > int64(p.MaxSizeMB)*bytesPerMB {
 		return false
 	}
 
+	if kbps := candidateKbps(c, runtimeMillis); kbps > 0 {
+		if p.MinKbps > 0 && kbps < float64(p.MinKbps) {
+			return false
+		}
+
+		if p.MaxKbps > 0 && kbps > float64(p.MaxKbps) {
+			return false
+		}
+	}
+
 	if len(p.AllowedFormats) == 0 {
 		return true
 	}
@@ -107,11 +186,14 @@ func (p AutoDownloadPrefs) eligible(c Candidate) bool {
 
 // filter returns only the candidates these preferences allow to be
 // auto-picked, in the same (already ranked) order.
-func (p AutoDownloadPrefs) filter(ranked []Candidate) []Candidate {
+func (p AutoDownloadPrefs) filter(
+	ranked []Candidate,
+	runtimeMillis int64,
+) []Candidate {
 	out := make([]Candidate, 0, len(ranked))
 
 	for _, c := range ranked {
-		if p.eligible(c) {
+		if p.eligible(c, runtimeMillis) {
 			out = append(out, c)
 		}
 	}
@@ -119,32 +201,116 @@ func (p AutoDownloadPrefs) filter(ranked []Candidate) []Candidate {
 	return out
 }
 
-// sizeFit scores how close totalSize is to PreferredSizeMB, 0..1,
+// bitrateFit scores how close a candidate's average bitrate is to
-// falling off linearly as the size doubles or halves away from it.
+// PreferredKbps, falling off linearly as it doubles or halves away
-// Returns a neutral 0.5 when no preference is set, so the absence of a
+// from it.
-// preference does not bias ranking.
+//
-func (p AutoDownloadPrefs) sizeFit(totalSize int64) float64 {
+// The range is **0.5 to 1.0, not 0 to 1**, and the floor is the point.
+// This carries 0.40 of the quality score once a preference is set, so a
+// span down to zero would let a preference of 320 kbps push a perfectly
+// good FLAC under `minQuality` and out of auto-pick altogether —
+// turning "I like 320" into "never take anything else", silently.  A
+// preference may promote the copy that matches it; it may not
+// disqualify the others.  That is what `MinKbps`/`MaxKbps` are for, and
+// they say so out loud.
+//
+// Returns the neutral floor when no preference is set or the rate
+// cannot be worked out, so neither an absent preference nor an absent
+// runtime biases ranking.
+func (p AutoDownloadPrefs) bitrateFit(
+	c Candidate,
+	runtimeMillis int64,
+) float64 {
 	const (
-		bytesPerMB = 1 << 20
+		neutral = 0.5
-		neutral    = 0.5
+		span    = 0.5
 	)
 
-	if p.PreferredSizeMB <= 0 || totalSize <= 0 {
+	if p.PreferredKbps <= 0 {
 		return neutral
 	}
 
-	preferred := float64(p.PreferredSizeMB) * bytesPerMB
+	kbps := candidateKbps(c, runtimeMillis)
-	ratio := float64(totalSize) / preferred
+	if kbps <= 0 {
+		return neutral
+	}
 
+	ratio := kbps / float64(p.PreferredKbps)
 	if ratio < 1 {
 		ratio = 1 / ratio
 	}
 
 	// ratio is now >= 1: 1.0 is an exact match, 2.0 is double or half
-	// the preferred size.  Falls to 0 at 2x away and beyond.
+	// the preferred rate, where the closeness term reaches 0.
-	fit := 1 - (ratio - 1)
+	return neutral + span*clamp01(1-(ratio-1))
+}
 
-	return clamp01(fit)
+// candidateKbps is a candidate's average audio bitrate, or 0 when it
+// cannot be worked out.
+//
+// Two sources, in this order, and the order matters:
+//
+//   - **Derived from bytes over runtime**, which is the honest one. It
+//     covers lossless (where a stated bitrate rarely exists), it cannot
+//     be lied to by a filename, and it is what the user's window means.
+//     Only the *audio* files count: cover scans and a log file are not
+//     part of the bitrate, and a folder with 30 MB of artwork would
+//     otherwise read as a better rip than the same music without it.
+//   - **The mean stated bitrate**, when the runtime is unknown. Weaker
+//     — a provider that parses it from an MP3 header states it and one
+//     that guesses from the filename also "states" it — but a number
+//     from the file itself beats no number at all.
+func candidateKbps(c Candidate, runtimeMillis int64) float64 {
+	const bitsPerByte = 8
+
+	audio := c.AudioFiles()
+	if len(audio) == 0 {
+		return 0
+	}
+
+	if runtimeMillis > 0 {
+		var bytes int64
+		for _, f := range audio {
+			bytes += f.Size
+		}
+
+		if bytes > 0 {
+			// bytes×8 bits over seconds, expressed in kbps: the two
+			// factors of 1000 (millis→seconds, bits→kilobits) cancel.
+			return float64(bytes) * bitsPerByte /
+				float64(runtimeMillis)
+		}
+	}
+
+	var (
+		sum   int
+		count int
+	)
+
+	for _, f := range audio {
+		if f.Bitrate > 0 {
+			sum += f.Bitrate
+			count++
+		}
+	}
+
+	if count == 0 {
+		return 0
+	}
+
+	return float64(sum) / float64(count)
+}
+
+// runtimeMillis is how long the requested release is, summed over its
+// expected tracklist.  Zero when the tracklist is absent or carries no
+// lengths, which is what every caller here treats as "unknown".
+func (d Download) runtimeMillis() int64 {
+	var total int64
+	for _, t := range d.Expected {
+		total += t.LengthMillis
+	}
+
+	return total
 }
 
 // Score fills a candidate's Match, Quality and Score fields.
@@ -160,7 +326,9 @@ func Score(dl Download, c Candidate, priority int, prefs AutoDownloadPrefs) Cand
 	c.Files = mergeMatched(c.Files, matched)
 
 	c.Match = scoreMatch(dl, c, audio, titleFit)
-	c.Quality = scoreQuality(c, audio, priority, prefs)
+	c.Quality = scoreQuality(
+		c, audio, priority, prefs, dl.runtimeMillis(),
+	)
 
 	c.Score = weightMatch*c.Match.Overall + weightQuality*c.Quality.Overall
 
@@ -279,11 +447,12 @@ func scoreQuality(
 	audio []CandidateFile,
 	priority int,
 	prefs AutoDownloadPrefs,
+	runtimeMillis int64,
 ) QualityScore {
 	q := QualityScore{
-		Health:   clamp01(c.Health),
+		Health:     clamp01(c.Health),
-		Priority: clamp01(float64(priority) / 100.0),
+		Priority:   clamp01(float64(priority) / 100.0),
-		SizeFit:  prefs.sizeFit(c.TotalSize),
+		BitrateFit: prefs.bitrateFit(c, runtimeMillis),
 	}
 
 	if len(audio) == 0 {
@@ -310,11 +479,13 @@ func scoreQuality(
 	q.FormatRank = worst
 	q.Bitrate = bitrateScore(audio)
 
-	q.Overall = weightFormat*q.FormatRank +
+	wFormat, wBitrate, wHealth, wPriority, wFit := qualityWeights(prefs)
-		weightBitrate*q.Bitrate +
+
-		weightHealth*q.Health +
+	q.Overall = wFormat*q.FormatRank +
-		weightPriority*q.Priority +
+		wBitrate*q.Bitrate +
-		weightSizeFit*q.SizeFit
+		wHealth*q.Health +
+		wPriority*q.Priority +
+		wFit*q.BitrateFit
 
 	if q.Mixed {
 		q.Overall *= 0.9
@@ -444,6 +615,19 @@ func Rank(
 			return out[i].Match.Overall > out[j].Match.Overall
 		}
 
+		// Closest to the preferred bitrate wins the tie.
+		//
+		// This is what decides which copy is taken now that auto-pick
+		// no longer requires the winner to be clear of the field: when
+		// several candidates are equally good matches of equal overall
+		// quality, the one the user said they wanted the shape of is
+		// the answer, ahead of provider priority.  With no preference
+		// set every BitrateFit is the same neutral value and this
+		// falls through, exactly as before.
+		if out[i].Quality.BitrateFit != out[j].Quality.BitrateFit {
+			return out[i].Quality.BitrateFit > out[j].Quality.BitrateFit
+		}
+
 		if out[i].Quality.Priority != out[j].Quality.Priority {
 			return out[i].Quality.Priority > out[j].Quality.Priority
 		}
@@ -454,19 +638,58 @@ func Rank(
 	return out
 }
 
-// AutoPickable reports whether a ranked list has a clear enough winner
+// Auto-pick gates.  Named rather than inlined because AutoPickVeto
-// to grab without asking.  It demands an anchored request, a high match,
+// reports which of them refused, and a number in a sentence the user
-// decent quality, and daylight between first and second place — if two
+// reads should be the same number the decision used.
-// candidates are close, the choice is the user's.
+const (
-func AutoPickable(dl Download, ranked []Candidate, prefs AutoDownloadPrefs) bool {
+	minMatch   = 0.85
-	const (
+	minQuality = 0.5
-		minMatch   = 0.85
+)
-		minQuality = 0.5
-		minLead    = 0.08
-	)
 
-	if !dl.Anchored() || len(ranked) == 0 {
+// AutoPickable reports whether a ranked list has a candidate worth
-		return false
+// grabbing without asking: an anchored request with a tracklist behind
+// it, and a candidate that clears the match and quality bars inside the
+// user's guardrails.
+//
+// **It does not require the winner to be better than the runner-up.**
+// It used to demand 0.08 of daylight on the combined score, which meant
+// the check fired hardest in the case it was never written for: a
+// popular album turns up five *correct* copies, all matching the
+// tracklist at 95%+ and differing only in format and seeders, their
+// scores land within a point of each other, and auto-pick refused
+// forever on the grounds that the choice was the user's.  It was not.
+// There was no question about *what* to fetch, only about which copy —
+// and abundance is the one condition under which that question matters
+// least.  A candidate does not need to be the best one, only one that
+// meets the criteria; where several do, `Rank` puts the one closest to
+// the preferred bitrate first.
+func AutoPickable(dl Download, ranked []Candidate, prefs AutoDownloadPrefs) bool {
+	return AutoPickVeto(dl, ranked, prefs) == ""
+}
+
+// AutoPickVeto returns the reason auto-pick declined, or "" when it
+// would go ahead.
+//
+// It exists because "it rejected all of them" was indistinguishable
+// from "it found nothing good".  The request list's message was built
+// from `ranked[0]` — the best candidate *before* the size and format
+// guardrails, and before the lead check — so a request refused because
+// the user's maximum size excluded every copy, or because three equally
+// good copies were found, reported "best of 12 found is not a confident
+// enough match (match 96%, quality 88%)".  Numbers that clear both
+// thresholds, beside a refusal, is a message that teaches the user the
+// matcher is broken.  Each gate names itself now.
+func AutoPickVeto(
+	dl Download,
+	ranked []Candidate,
+	prefs AutoDownloadPrefs,
+) string {
+	if len(ranked) == 0 {
+		return "nothing found"
+	}
+
+	if !dl.Anchored() {
+		return "the request is free text, so there is no release to be right about"
 	}
 
 	// An anchor with no tracklist behind it is an anchor in name only:
@@ -474,29 +697,42 @@ func AutoPickable(dl Download, ranked []Candidate, prefs AutoDownloadPrefs) bool
 	// is exactly the evidence a wrong-album candidate also has.  This
 	// matters most for the request list, where nobody is watching.
 	if len(dl.Expected) == 0 {
-		return false
+		return "no tracklist for this release is known yet, so a candidate cannot be checked against it"
 	}
 
-	// The guardrails apply before the match/quality/lead checks: a
+	// The guardrails apply before the match and quality checks: a
-	// candidate outside the allowed size or format is not a worse
+	// candidate outside the allowed bitrate, size or format is not a
-	// choice, it is not a choice auto-pick may make at all, so it must
+	// worse choice, it is not a choice auto-pick may make at all, so it
-	// not count as "the winner" nor as "second place" for the lead
+	// must not count as "the winner" either.
-	// check below.
+	eligible := prefs.filter(ranked, dl.runtimeMillis())
-	eligible := prefs.filter(ranked)
 	if len(eligible) == 0 {
-		return false
+		return fmt.Sprintf(
+			"all %d found are outside the auto-download bitrate, size or format limits",
+			len(ranked),
+		)
 	}
 
 	best := eligible[0]
-	if best.Match.Overall < minMatch || best.Quality.Overall < minQuality {
+
-		return false
+	if best.Match.Overall < minMatch {
+		return fmt.Sprintf(
+			"best of %d found matches this release only %.0f%% (needs %.0f%%)",
+			len(ranked),
+			best.Match.Overall*100, //nolint:mnd // percent
+			minMatch*100,           //nolint:mnd // percent
+		)
 	}
 
-	if len(eligible) > 1 && best.Score-eligible[1].Score < minLead {
+	if best.Quality.Overall < minQuality {
-		return false
+		return fmt.Sprintf(
+			"best of %d found is the right release but scores %.0f%% on quality (needs %.0f%%)",
+			len(ranked),
+			best.Quality.Overall*100, //nolint:mnd // percent
+			minQuality*100,           //nolint:mnd // percent
+		)
 	}
 
-	return true
+	return ""
 }
 
 // mergeMatched copies MatchedTo assignments from the audio-only slice

backend/download/rank_test.go

@@ -1,6 +1,34 @@
 package download
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
+
+// trackMillis is five minutes; okComputer's four of them make a
+// twenty-minute release, which is what turns a candidate's byte count
+// into a bitrate the assertions below can name.
+const trackMillis = 5 * 60 * 1000
+
+// okComputerRuntime is that release's runtime, for the helpers that
+// need it directly.
+const okComputerRuntime = 4 * trackMillis
+
+// kbpsCandidate builds an annotated candidate whose audio adds up to
+// the given average bitrate over okComputer's runtime.
+func kbpsCandidate(id, ext string, kbps int) Candidate {
+	// bits = kbps × 1000 × (runtimeMillis / 1000), so the thousands
+	// cancel and the byte count is kbps × runtimeMillis / 8.
+	const bitsPerByte = 8
+
+	total := int64(kbps) * okComputerRuntime / bitsPerByte
+
+	c := candidateFor(id, allTitles(), ext, total/int64(len(allTitles())))
+	c.Files = AnnotateFiles(c.Files)
+	c.TotalSize = total
+
+	return c
+}
 
 // okComputer is the reference request used across ranking tests.
 func okComputer() Download {
@@ -8,11 +36,15 @@ func okComputer() Download {
 		ReleaseMBID: "mbid-ok-computer",
 		Artist:      "Radiohead",
 		Album:       "OK Computer",
+		// Four five-minute tracks: twenty minutes, so a candidate's
+		// bitrate is a number these tests can state exactly. Without
+		// lengths there is no runtime and the bitrate window has
+		// nothing to divide by.
 		Expected: []ExpectedTrack{
-			{Position: 1, Title: "Airbag"},
+			{Position: 1, Title: "Airbag", LengthMillis: trackMillis},
-			{Position: 2, Title: "Paranoid Android"},
+			{Position: 2, Title: "Paranoid Android", LengthMillis: trackMillis},
-			{Position: 3, Title: "Subterranean Homesick Alien"},
+			{Position: 3, Title: "Subterranean Homesick Alien", LengthMillis: trackMillis},
-			{Position: 4, Title: "Exit Music (For a Film)"},
+			{Position: 4, Title: "Exit Music (For a Film)", LengthMillis: trackMillis},
 		},
 	}
 }
@@ -187,7 +219,7 @@ func TestUnanchoredMatchIsCapped(t *testing.T) {
 	}
 }
 
-func TestAutoPickableRequiresAnchorAndLead(t *testing.T) {
+func TestAutoPickableRequiresAnchorAndTracklist(t *testing.T) {
 	t.Parallel()
 
 	dl := okComputer()
@@ -211,14 +243,18 @@ func TestAutoPickableRequiresAnchorAndLead(t *testing.T) {
 		}
 	})
 
-	t.Run("two close candidates are not", func(t *testing.T) {
+	// Two identical copies are a spare, not an ambiguity. This
+	// asserted the opposite while auto-pick required daylight over the
+	// runner-up — a rule that made abundance the thing that stopped a
+	// request being satisfied, which is backwards.
+	t.Run("two equally good candidates still are", func(t *testing.T) {
 		t.Parallel()
 
 		twin := best
 		twin.ID = "twin"
 
-		if AutoPickable(dl, []Candidate{best, twin}, AutoDownloadPrefs{}) {
+		if !AutoPickable(dl, []Candidate{best, twin}, AutoDownloadPrefs{}) {
-			t.Error("identical candidates must not auto-pick")
+			t.Error("identical good candidates must auto-pick")
 		}
 	})
 
@@ -300,18 +336,11 @@ func TestProviderPriorityBreaksTies(t *testing.T) {
 	}
 }
 
-const mb = 1 << 20
-
 func TestAutoDownloadPrefsEligible(t *testing.T) {
 	t.Parallel()
 
-	flacCandidate := candidateFor("c", allTitles(), ".flac", 30_000_000)
+	flacCandidate := kbpsCandidate("c", ".flac", 900)
-	flacCandidate.Files = AnnotateFiles(flacCandidate.Files)
+	mp3Candidate := kbpsCandidate("c", ".mp3", 128)
-	flacCandidate.TotalSize = 300 * mb
-
-	mp3Candidate := candidateFor("c", allTitles(), ".mp3", 3_000_000)
-	mp3Candidate.Files = AnnotateFiles(mp3Candidate.Files)
-	mp3Candidate.TotalSize = 30 * mb
 
 	tests := []struct {
 		name  string
@@ -321,18 +350,25 @@ func TestAutoDownloadPrefsEligible(t *testing.T) {
 	}{
 		{"zero value is permissive", AutoDownloadPrefs{}, flacCandidate, true},
 		{
-			"within min/max window",
+			"within the bitrate window",
-			AutoDownloadPrefs{MinSizeMB: 100, MaxSizeMB: 500},
+			AutoDownloadPrefs{MinKbps: 320, MaxKbps: 1200},
 			flacCandidate, true,
 		},
 		{
-			"below minimum",
+			"below the minimum bitrate",
-			AutoDownloadPrefs{MinSizeMB: 400},
+			AutoDownloadPrefs{MinKbps: 500},
+			mp3Candidate, false,
+		},
+		{
+			"above the maximum bitrate",
+			AutoDownloadPrefs{MaxKbps: 500},
 			flacCandidate, false,
 		},
 		{
-			"above maximum",
+			// The ceiling is bytes, not a rate, and it is the guard
-			AutoDownloadPrefs{MaxSizeMB: 200},
+			// that still works when the bitrate cannot be worked out.
+			"above the hard size ceiling",
+			AutoDownloadPrefs{MaxSizeMB: 50},
 			flacCandidate, false,
 		},
 		{
@@ -351,57 +387,131 @@ func TestAutoDownloadPrefsEligible(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			if got := tt.prefs.eligible(tt.c); got != tt.want {
+			got := tt.prefs.eligible(tt.c, okComputerRuntime)
+			if got != tt.want {
 				t.Errorf("eligible() = %v, want %v", got, tt.want)
 			}
 		})
 	}
 }
 
+// A release nobody knows the length of cannot be judged on bitrate, and
+// the window must not become a silent embargo because MusicBrainz is
+// missing a track length.  The size ceiling still applies — that is why
+// it is a separate field.
+func TestBitrateWindowPassesAnUnknownRuntime(t *testing.T) {
+	t.Parallel()
+
+	c := kbpsCandidate("c", ".mp3", 128)
+	prefs := AutoDownloadPrefs{MinKbps: 900}
+
+	if !prefs.eligible(c, 0) {
+		t.Error("an unknown runtime must pass the bitrate window")
+	}
+
+	if prefs.eligible(c, okComputerRuntime) {
+		t.Error("a known runtime must still be judged")
+	}
+
+	ceiling := AutoDownloadPrefs{MaxSizeMB: 1}
+	if ceiling.eligible(c, 0) {
+		t.Error("the size ceiling must apply even with no runtime")
+	}
+}
+
+// Artwork is not part of the bitrate.  A folder carrying 30 MB of
+// scans would otherwise read as a better rip than the same music
+// without them, which is backwards.
+func TestBitrateIgnoresNonAudioFiles(t *testing.T) {
+	t.Parallel()
+
+	c := kbpsCandidate("c", ".mp3", 320)
+	bare := candidateKbps(c, okComputerRuntime)
+
+	c.Files = append(c.Files, CandidateFile{
+		Path: "Radiohead - OK Computer/cover.jpg",
+		Size: 30 << 20,
+	})
+	c.Files = AnnotateFiles(c.Files)
+
+	if got := candidateKbps(c, okComputerRuntime); got != bare {
+		t.Errorf("bitrate with artwork = %f, want %f", got, bare)
+	}
+}
+
+// Where no runtime is known, a stated per-file bitrate is better than
+// no answer at all.
+func TestBitrateFallsBackToTheStatedRate(t *testing.T) {
+	t.Parallel()
+
+	c := candidateFor("c", allTitles(), ".mp3", 3_000_000)
+	for i := range c.Files {
+		c.Files[i].Bitrate = 192
+	}
+
+	c.Files = AnnotateFiles(c.Files)
+
+	if got := candidateKbps(c, 0); got != 192 {
+		t.Errorf("stated bitrate = %f, want 192", got)
+	}
+}
+
 func TestAutoDownloadPrefsFilter(t *testing.T) {
 	t.Parallel()
 
-	small := candidateFor("small", allTitles(), ".flac", 10_000_000)
+	lossy := kbpsCandidate("lossy", ".mp3", 128)
-	small.TotalSize = 50 * mb
+	lossless := kbpsCandidate("lossless", ".flac", 900)
 
-	big := candidateFor("big", allTitles(), ".flac", 30_000_000)
+	prefs := AutoDownloadPrefs{MinKbps: 500}
-	big.TotalSize = 500 * mb
 
-	prefs := AutoDownloadPrefs{MinSizeMB: 100, MaxSizeMB: 600}
+	filtered := prefs.filter(
+		[]Candidate{lossy, lossless}, okComputerRuntime,
+	)
 
-	filtered := prefs.filter([]Candidate{small, big})
+	if len(filtered) != 1 || filtered[0].ID != "lossless" {
-
-	if len(filtered) != 1 || filtered[0].ID != "big" {
 		t.Errorf("filter() = %v, want only the in-window candidate", filtered)
 	}
 }
 
-func TestAutoDownloadPrefsSizeFit(t *testing.T) {
+func TestAutoDownloadPrefsBitrateFit(t *testing.T) {
 	t.Parallel()
 
 	const neutral = 0.5
 
 	tests := []struct {
-		name      string
+		name  string
-		prefs     AutoDownloadPrefs
+		prefs AutoDownloadPrefs
-		totalSize int64
+		c     Candidate
-		want      float64
+		want  float64
 	}{
-		{"no preference is neutral", AutoDownloadPrefs{}, 300 * mb, neutral},
+		{
+			"no preference is neutral",
+			AutoDownloadPrefs{},
+			kbpsCandidate("c", ".flac", 900), neutral,
+		},
 		{
 			"exact match scores 1",
-			AutoDownloadPrefs{PreferredSizeMB: 300},
+			AutoDownloadPrefs{PreferredKbps: 320},
-			300 * mb, 1.0,
+			kbpsCandidate("c", ".mp3", 320), 1.0,
 		},
 		{
-			"double the preferred size scores 0",
+			// The floor is neutral, not zero: this term carries 0.40
-			AutoDownloadPrefs{PreferredSizeMB: 300},
+			// of the quality score once a preference is set, and a
-			600 * mb, 0.0,
+			// span to zero would let "I like 320" quietly disqualify
+			// every FLAC from auto-pick.
+			"double the preferred rate falls to the neutral floor",
+			AutoDownloadPrefs{PreferredKbps: 320},
+			kbpsCandidate("c", ".flac", 640), neutral,
 		},
 		{
-			"half the preferred size scores 0",
+			"half the preferred rate falls to the neutral floor",
-			AutoDownloadPrefs{PreferredSizeMB: 300},
+			AutoDownloadPrefs{PreferredKbps: 320},
-			150 * mb, 0.0,
+			kbpsCandidate("c", ".mp3", 160), neutral,
+		},
+		{
+			"an unknowable rate is neutral",
+			AutoDownloadPrefs{PreferredKbps: 320},
+			kbpsCandidate("c", ".mp3", 320), neutral,
 		},
 	}
 
@@ -409,30 +519,223 @@ func TestAutoDownloadPrefsSizeFit(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			if got := tt.prefs.sizeFit(tt.totalSize); got != tt.want {
+			// The last case deliberately withholds the runtime.
-				t.Errorf("sizeFit(%d) = %f, want %f", tt.totalSize, got, tt.want)
+			runtime := int64(okComputerRuntime)
+			if tt.name == "an unknowable rate is neutral" {
+				runtime = 0
+			}
+
+			if got := tt.prefs.bitrateFit(tt.c, runtime); got != tt.want {
+				t.Errorf("bitrateFit() = %f, want %f", got, tt.want)
 			}
 		})
 	}
 }
 
 // An otherwise-perfect candidate must not auto-pick when it falls
-// outside the configured size guard: the guardrail applies before the
+// outside the configured guardrails: they apply before the match and
-// match/quality/lead checks, not as one more input averaged into them.
+// quality checks, not as one more input averaged into them.
-func TestAutoPickableRejectsCandidateOutsideSizeGuard(t *testing.T) {
+func TestAutoPickableRejectsCandidateOutsideTheGuardrails(t *testing.T) {
 	t.Parallel()
 
 	dl := okComputer()
-	best := Score(dl, candidateFor("a", allTitles(), ".flac", 30_000_000), 50, AutoDownloadPrefs{})
+	best := Score(dl, kbpsCandidate("a", ".flac", 900), 50, AutoDownloadPrefs{})
-	best.TotalSize = 500 * mb
 
 	if !AutoPickable(dl, []Candidate{best}, AutoDownloadPrefs{}) {
 		t.Fatal("expected this candidate to be auto-pickable with no guardrails")
 	}
 
-	tight := AutoDownloadPrefs{MinSizeMB: 10, MaxSizeMB: 100}
+	if AutoPickable(dl, []Candidate{best}, AutoDownloadPrefs{MaxKbps: 320}) {
+		t.Error("candidate above the bitrate window must not auto-pick")
+	}
 
-	if AutoPickable(dl, []Candidate{best}, tight) {
+	if AutoPickable(dl, []Candidate{best}, AutoDownloadPrefs{MaxSizeMB: 1}) {
-		t.Error("candidate outside the size guard must not auto-pick")
+		t.Error("candidate above the size ceiling must not auto-pick")
+	}
+}
+
+// The refusal has to name the gate that refused.
+//
+// Before AutoPickVeto, every one of these came back as the same
+// sentence built from `ranked[0]` — the best candidate before the size
+// and format guardrails — so a request refused because the user's size
+// window excluded every copy reported a match and a quality that both
+// cleared their thresholds.  A refusal quoting numbers that pass is
+// what made the matcher look broken from outside.
+func TestAutoPickVetoNamesTheGate(t *testing.T) {
+	t.Parallel()
+
+	dl := okComputer()
+	best := Score(
+		dl,
+		candidateFor("a", allTitles(), ".flac", 30_000_000),
+		50,
+		AutoDownloadPrefs{},
+	)
+
+	// candidateFor sizes the files and leaves TotalSize at 0, which is
+	// what the guardrails read.
+	sized := func(c Candidate, total int64) Candidate {
+		c.TotalSize = total
+
+		return c
+	}
+
+	tests := []struct {
+		name    string
+		dl      Download
+		ranked  []Candidate
+		prefs   AutoDownloadPrefs
+		wantSub string
+	}{
+		{
+			name:    "nothing found",
+			dl:      dl,
+			ranked:  nil,
+			wantSub: "nothing found",
+		},
+		{
+			name:    "free text",
+			dl:      Download{Artist: "Radiohead", Album: "OK Computer"},
+			ranked:  []Candidate{best},
+			wantSub: "free text",
+		},
+		{
+			name: "no tracklist behind the anchor",
+			dl: Download{
+				ReleaseMBID: "mbid-ok-computer",
+				Artist:      "Radiohead",
+				Album:       "OK Computer",
+			},
+			ranked:  []Candidate{best},
+			wantSub: "no tracklist",
+		},
+		{
+			// The candidate is 120 MB and the window tops out at 1 MB:
+			// the old message reported its match and quality instead.
+			name:    "outside the size window",
+			dl:      dl,
+			ranked:  []Candidate{sized(best, 120<<20)},
+			prefs:   AutoDownloadPrefs{MaxSizeMB: 1},
+			wantSub: "bitrate, size or format limits",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := AutoPickVeto(tt.dl, tt.ranked, tt.prefs)
+			if !strings.Contains(got, tt.wantSub) {
+				t.Errorf("veto = %q, want it to mention %q", got, tt.wantSub)
+			}
+		})
+	}
+}
+
+// A clear winner has no veto at all — the sentence is empty, which is
+// what AutoPickable reads.
+func TestAutoPickVetoIsEmptyForAClearWinner(t *testing.T) {
+	t.Parallel()
+
+	dl := okComputer()
+	best := Score(
+		dl,
+		candidateFor("a", allTitles(), ".flac", 30_000_000),
+		50,
+		AutoDownloadPrefs{},
+	)
+	weak := Score(
+		dl,
+		candidateFor("b", allTitles()[:2], ".mp3", 1_000_000),
+		50,
+		AutoDownloadPrefs{},
+	)
+
+	if got := AutoPickVeto(dl, []Candidate{best, weak}, AutoDownloadPrefs{}); got != "" {
+		t.Errorf("veto = %q, want none", got)
+	}
+}
+
+// With several candidates that all clear the bar, the preferred
+// bitrate decides which one is taken.
+//
+// This is what replaced the daylight requirement.  Auto-pick no longer
+// refuses when the field is close; it takes the copy nearest the shape
+// the user asked for, which is the question they actually answered in
+// Settings.
+func TestPreferredBitrateBreaksTheTie(t *testing.T) {
+	t.Parallel()
+
+	dl := okComputer()
+	prefs := AutoDownloadPrefs{PreferredKbps: 320}
+
+	// Same album, same completeness, same health, same provider — the
+	// only difference between them is the rate.
+	lossless := kbpsCandidate("lossless", ".flac", 900)
+	perfect := kbpsCandidate("perfect", ".mp3", 320)
+
+	ranked := Rank(
+		dl, []Candidate{lossless, perfect}, nil, prefs,
+	)
+
+	if ranked[0].ID != "perfect" {
+		t.Errorf(
+			"winner = %q (fit %f) over %q (fit %f), want the 320 kbps copy",
+			ranked[0].ID, ranked[0].Quality.BitrateFit,
+			ranked[1].ID, ranked[1].Quality.BitrateFit,
+		)
+	}
+
+	if AutoPickVeto(dl, ranked, prefs) != "" {
+		t.Error("a close field must still auto-pick")
+	}
+}
+
+// With no preference set, nothing changes: BitrateFit is the same
+// neutral value for every candidate and the older tie-breaks decide.
+func TestNoPreferredBitrateLeavesRankingAlone(t *testing.T) {
+	t.Parallel()
+
+	dl := okComputer()
+
+	lossless := kbpsCandidate("lossless", ".flac", 900)
+	lossy := kbpsCandidate("lossy", ".mp3", 320)
+
+	ranked := Rank(
+		dl, []Candidate{lossy, lossless}, nil, AutoDownloadPrefs{},
+	)
+
+	if ranked[0].ID != "lossless" {
+		t.Errorf(
+			"winner = %q, want the lossless copy on format alone",
+			ranked[0].ID,
+		)
+	}
+}
+
+// A preferred bitrate promotes the copy that matches it and must never
+// disqualify the ones that do not.  It carries 0.40 of the quality
+// score, so a fit spanning down to zero would put a perfectly good FLAC
+// under minQuality and out of auto-pick — turning a preference into a
+// prohibition without saying so.  MinKbps and MaxKbps are how a user
+// says that on purpose.
+func TestAPreferredBitrateNeverDisqualifies(t *testing.T) {
+	t.Parallel()
+
+	dl := okComputer()
+	far := AutoDownloadPrefs{PreferredKbps: 128}
+
+	lossless := Score(dl, kbpsCandidate("flac", ".flac", 900), 50, far)
+
+	if lossless.Quality.Overall < minQuality {
+		t.Errorf(
+			"quality = %f under a far-off preference, want >= %f",
+			lossless.Quality.Overall, minQuality,
+		)
+	}
+
+	if veto := AutoPickVeto(dl, []Candidate{lossless}, far); veto != "" {
+		t.Errorf("a far-off preference vetoed the candidate: %s", veto)
 	}
 }

backend/download/service_test.go

@@ -36,13 +36,24 @@ func newServiceFixture(t *testing.T) serviceFixture {
 	// assertion read it; the second is that same goroutine still writing
 	// into `t.TempDir()` after the test returned. One cause, two shapes.
 	//
-	// Putting the candidate outside the auto-pick size window stops the
+	// Putting the candidate outside the auto-pick guardrails stops the
 	// grab from ever starting, which is better than waiting for it: there
 	// is no goroutine to be slow, so the tests state what they mean
 	// ("the request exists, in this state") without a timing assumption
 	// underneath. A test that does want the download has `managerFixture`
 	// and sets its own preferences.
-	mf.manager.SetPreferences(AutoDownloadPrefs{MaxSizeMB: 1})
+	//
+	// The guard is a *format* the fake never produces, and it used to be
+	// `MaxSizeMB: 1`, which never fired: the size gates read
+	// `Candidate.TotalSize`, which real providers fill and the fake
+	// leaves at zero, and zero is under every ceiling. So the grab went
+	// ahead anyway and the second failure shape above — the TempDir
+	// cleanup race — kept happening, reproducibly, roughly one run in
+	// fifteen. A guard has to be keyed on something the fixture
+	// actually sets.
+	mf.manager.SetPreferences(AutoDownloadPrefs{
+		AllowedFormats: []Format{FormatWMA},
+	})
 
 	return serviceFixture{managerFixture: mf, svc: svc}
 }
@@ -182,6 +193,11 @@ func TestManualDownloadSatisfiesRequestOnSuccess(t *testing.T) {
 	f := newServiceFixture(t)
 	ctx := context.Background()
 
+	// This is the one test here that is *about* the download, so it
+	// undoes the fixture's guard rather than relying on it — which is
+	// what it was doing implicitly while the guard did not work.
+	f.manager.SetPreferences(AutoDownloadPrefs{})
+
 	provider := fakeWithAlbum(1, "source", ".flac")
 	f.manager.installProvider(Config{ID: 1, Priority: 50}, provider)
 

backend/download/types.go

@@ -302,7 +302,12 @@ type QualityScore struct {
 	Bitrate    float64 `json:"bitrate"`
 	Health     float64 `json:"health"`   // seeders, free slots
 	Priority   float64 `json:"priority"` // user's per-provider preference
-	SizeFit    float64 `json:"sizeFit"`  // closeness to the preferred download size
+	// BitrateFit is closeness to the preferred *rate*, which is what
+	// the auto-download window is expressed in.  It replaced a
+	// `SizeFit` measured in megabytes: a size means nothing without
+	// knowing how long the music is, so the same number described a
+	// generous single and a suspiciously small boxset.
+	BitrateFit float64 `json:"bitrateFit"`
 
 	// Mixed marks a candidate whose files are not all the same format,
 	// which usually means a hand-assembled folder rather than a rip.

backend/explore/coverartproxy.go

@@ -25,8 +25,26 @@ const (
 	// where cached cover art thumbnails are stored.
 	thumbnailDir = CoverArtCacheDirName
 
-	// thumbnailTimeout is the HTTP timeout for fetching a thumbnail.
+	// thumbnailTimeout is the HTTP timeout for fetching a thumbnail,
-	thumbnailTimeout = 10 * time.Second
+	// and it has to cover a redirect the Cover Art Archive does not
+	// serve itself.
+	//
+	// `coverartarchive.org` answers `front-250` with a 307 to an
+	// Internet Archive storage node (`dn######.us.archive.org`), and
+	// those nodes are routinely slow: measured against the twelve
+	// albums on Explore's own shelves, a successful fetch took 14–16 s
+	// and a failing one 13–17 s. At 10 s *every* cover on the page
+	// timed out — 24 cards, 5 of which had art, all of those from the
+	// disk cache — which reads as "Explore has no album art" rather
+	// than as a slow upstream, because a timeout writes nothing and
+	// says nothing.
+	//
+	// 30 s is chosen to clear that measured range with room, not to be
+	// generous: the fetch is off the critical path (each one is its own
+	// goroutine behind an 8/s limiter, and the frontend renders a
+	// placeholder until it lands), so the cost of waiting is nothing
+	// and the cost of giving up early is a blank page.
+	thumbnailTimeout = 30 * time.Second
 
 	// thumbnailMaxSize is the maximum image size to cache (2 MB).
 	thumbnailMaxSize = 2 * 1024 * 1024
@@ -97,6 +115,20 @@ func (p *CoverArtProxy) GetThumbnail(
 		return ""
 	}
 
+	// A 404 is an answer, and it is already on disk.
+	//
+	// `writeCache(mbid, nil)` has recorded "the archive has no art for
+	// this" as an empty file since this was written, and nothing has
+	// ever read it back: `readCache` returns "" for an empty file,
+	// which is indistinguishable from a miss, so every art-less release
+	// group was re-fetched from the network on every render that asked
+	// about it. On Explore's shelves a third of the cards are art-less,
+	// so that was a third of the page spending a live CAA request to be
+	// told again what the last one said.
+	if p.knownMissing(releaseGroupMBID) {
+		return ""
+	}
+
 	// Source 3: fetch from Cover Art Archive (slow, cached to disk).
 	url := CoverArtGroupURL(releaseGroupMBID)
 	data, cacheable, err := p.fetch(url)
@@ -177,8 +209,9 @@ func (p *CoverArtProxy) GetCandidateThumbnail(
 		}
 	}
 
-	// Network fetch on release group.
+	// Network fetch on release group — unless a previous one was told
-	if releaseGroupMBID != "" {
+	// there is none. See `knownMissing`.
+	if releaseGroupMBID != "" && !p.knownMissing(releaseGroupMBID) {
 		url := CoverArtGroupURL(releaseGroupMBID)
 		data, cacheable, err := p.fetch(url)
 
@@ -194,7 +227,7 @@ func (p *CoverArtProxy) GetCandidateThumbnail(
 	}
 
 	// Network fetch on release (fallback).
-	if releaseMBID != "" {
+	if releaseMBID != "" && !p.knownMissing(releaseMBID) {
 		url := CoverArtURL(releaseMBID)
 		data, cacheable, err := p.fetch(url)
 
@@ -285,6 +318,17 @@ func (p *CoverArtProxy) cachePath(mbid string) string {
 	return filepath.Join(p.cacheDir, mbid+".jpg")
 }
 
+// knownMissing reports whether a previous fetch was told the archive
+// has no art for this MBID — the empty file `writeCache(mbid, nil)`
+// leaves behind. It is deliberately separate from `readCache`, which
+// answers "what are the bytes" and cannot express the difference
+// between no answer and an answer of none.
+func (p *CoverArtProxy) knownMissing(mbid string) bool {
+	info, err := os.Stat(p.cachePath(mbid))
+
+	return err == nil && info.Size() == 0
+}
+
 func (p *CoverArtProxy) readCache(mbid string) string {
 	path := p.cachePath(mbid)
 

backend/library/library.go

@@ -289,8 +289,13 @@ func (l *Library) scanInternal(
 		l.mu.Unlock()
 	}()
 
+	// The configured mode, not a hardcoded "auto".  `ScanConcurrency`
+	// has been a validated config field with three values and one
+	// caller passing a constant, so choosing `ssd` or `hdd` by hand
+	// did nothing at all.
+	diskProfile := system.ProfileForPath(libraryPath)
 	workerCount := resolveScanWorkerCount(
-		ScanConcurrencyAuto,
+		l.conf.ScanConcurrency,
 		libraryPath,
 	)
 
@@ -300,6 +305,10 @@ func (l *Library) scanInternal(
 		"libraryName", libraryName,
 		"libraryPath", libraryPath,
 		"workers", workerCount,
+		"mode", l.conf.ScanConcurrency,
+		"device", diskProfile.Device,
+		"rotational", diskProfile.Rotational,
+		"queueDepth", diskProfile.QueueDepth,
 	)
 
 	// Helper to build a ScanProgress with library identification.
@@ -818,7 +827,7 @@ func (l *Library) scanInternal(
 	g := new(errgroup.Group)
 	g.SetLimit(workerCount)
 
-	for work := range workChan {
+	for work := range readaheadWork(scanCtx, workChan, diskProfile) {
 		g.Go(func() error {
 			if err := l.waitIfPaused(scanCtx); err != nil {
 				return err
@@ -1285,9 +1294,101 @@ func surveyAudioFiles(
 	return count, maxModTime
 }
 
-// hddWorkerCount is the maximum number of concurrent extraction
+// How many extraction workers a spinning disk gets, and why it is two
-// workers when the library resides on a spinning disk.
+// numbers rather than one.
-const hddWorkerCount = 2
+//
+// Extraction is not CPU work — every parser here reads headers and
+// returns — so on a spinning disk the whole cost is seek latency, and
+// the only question worth asking is how many reads should be in flight
+// at once.  That has two different right answers and the drive says
+// which:
+//
+//   - A drive with command queueing (NCQ: /sys/block/<dev>/device/
+//     queue_depth reports 31 or 32 on any SATA disk with it enabled)
+//     reorders outstanding reads into the order its head passes over
+//     them.  Handing it several at once is most of why a parallel scan
+//     beats a serial one at all, and four is where the returns flatten:
+//     the drive needs a few requests to have anything to reorder, and
+//     past that it is queueing requests it was already going to
+//     service in that order.
+//   - A drive without it — queue_depth 1, which is what a USB bridge
+//     or a pre-2004 disk reports — services one command at a time in
+//     the order given.  Every extra worker there is one more seek
+//     competing for one head, and the scan gets *slower* the harder it
+//     is pushed.  Two is kept rather than one because the readahead
+//     hints (see readaheadWork) do the overlapping that concurrency
+//     was standing in for, and one worker cannot hide a stall.
+//
+// This used to be a flat 2 for anything rotational, which is a
+// pre-NCQ assumption: it left a modern spinning disk with a quarter of
+// the queue depth it can use.
+const (
+	hddWorkerCountQueued = 4
+	hddWorkerCountSerial = 2
+)
+
+// Readahead tuning.
+const (
+	// readaheadDepth is how many files ahead of the workers the
+	// prefetcher runs.  It is the channel's buffer, so it is also the
+	// number of `WILLNEED` hints outstanding at once — comfortably more
+	// than a queueing drive's 32-command window is worth filling with
+	// one library, and small enough that a cancelled scan is not
+	// holding a long tail of queued reads.
+	readaheadDepth = 16
+
+	// readaheadBytes is how much of each file to pull in.  Everything
+	// the scanner reads lives at the head: ID3v2 and FLAC's
+	// STREAMINFO/VORBIS_COMMENT/PICTURE blocks, and the first MPEG
+	// frame with its Xing header.  512 KB covers a tag carrying
+	// embedded cover art, which is the large case — and reading a
+	// little too much sequentially costs a spinning disk almost
+	// nothing next to the seek that got there.
+	readaheadBytes = 512 << 10
+)
+
+// readaheadWork forwards scan work while asking the kernel to fetch
+// each file's header before a worker reaches it.
+//
+// The buffered channel *is* the lookahead: this goroutine runs ahead
+// of the workers until the buffer fills, hinting every file as it goes,
+// so by the time a worker takes an item the read it needs has been in
+// flight for `readaheadDepth` files' worth of parsing.  That is the
+// only thing that helps a spinning disk here, because the per-file work
+// is already header-only — every parser in `backend/metadata` reads a
+// few hundred bytes and returns, so the scan is not waiting on CPU or
+// on bytes, it is waiting on the head to arrive.
+//
+// It runs on rotational disks only.  An SSD has no seek to hide and
+// already has one worker per core; issuing hints there is pure syscall
+// overhead against an OS readahead that is already ahead of us.
+func readaheadWork(
+	ctx context.Context,
+	in <-chan scanWork,
+	profile system.DiskProfile,
+) <-chan scanWork {
+	if !profile.Rotational {
+		return in
+	}
+
+	out := make(chan scanWork, readaheadDepth)
+
+	go func() {
+		defer close(out)
+
+		for work := range in {
+			hintReadahead(work.absolutePath, readaheadBytes)
+
+			select {
+			case out <- work:
+			case <-ctx.Done():
+				return
+			}
+		}
+	}()
+
+	return out
+}
 
 // resolveScanWorkerCount returns the number of concurrent
 // extraction workers based on the configured concurrency mode
@@ -1296,20 +1397,45 @@ func resolveScanWorkerCount(
 	mode ScanConcurrency,
 	libraryPath string,
 ) int {
+	return workersForProfile(
+		mode,
+		system.ProfileForPath(libraryPath),
+		goruntime.NumCPU(),
+	)
+}
+
+// workersForProfile is the policy on its own, so it can be tested
+// against drives this machine does not have.
+//
+// `hdd` and `ssd` override what the device says rather than being a
+// separate branch: the mode is the user overruling detection, and
+// detection is right about the queue depth either way — a user who
+// picks `hdd` on a queueing drive still wants that drive's queue used.
+func workersForProfile(
+	mode ScanConcurrency,
+	profile system.DiskProfile,
+	cpus int,
+) int {
+	spinning := profile.Rotational
+
 	switch mode {
 	case ScanConcurrencySSD:
-		return goruntime.NumCPU()
+		spinning = false
 	case ScanConcurrencyHDD:
-		return min(hddWorkerCount, goruntime.NumCPU())
+		spinning = true
-	default: // auto
+	case ScanConcurrencyAuto:
-		if system.IsRotationalDisk(libraryPath) {
-			return min(
-				hddWorkerCount, goruntime.NumCPU(),
-			)
-		}
-
-		return goruntime.NumCPU()
 	}
+
+	if !spinning {
+		return cpus
+	}
+
+	workers := hddWorkerCountSerial
+	if profile.Queues() {
+		workers = hddWorkerCountQueued
+	}
+
+	return min(workers, cpus)
 }
 
 // scanWork represents a file to be processed by a worker.

backend/library/readahead_linux.go

@@ -0,0 +1,38 @@
+//go:build linux
+
+package library
+
+import (
+	"os"
+
+	"golang.org/x/sys/unix"
+)
+
+// hintReadahead asks the kernel to start fetching the head of a file
+// that is about to be read.
+//
+// `POSIX_FADV_WILLNEED` returns immediately and queues the read, which
+// is the whole point: on a spinning disk the first access to a file
+// costs a seek of several milliseconds, and that latency can only be
+// hidden by having the next seek already in flight while the current
+// file is being parsed.  A drive with command queueing can then service
+// the queued reads in head order rather than in the order they were
+// asked for.
+//
+// Errors are dropped on purpose.  This is a hint: a file that has since
+// been deleted, a filesystem that does not implement fadvise, or a
+// permission the walk saw and this open does not, all mean "no
+// prefetch", never "fail the scan".  The read that follows is what
+// reports a genuine problem.
+func hintReadahead(path string, bytes int64) {
+	f, err := os.Open(path)
+	if err != nil {
+		return
+	}
+
+	defer func() { _ = f.Close() }()
+
+	_ = unix.Fadvise(
+		int(f.Fd()), 0, bytes, unix.FADV_WILLNEED,
+	)
+}

backend/library/readahead_other.go

@@ -0,0 +1,13 @@
+//go:build !linux
+
+package library
+
+// hintReadahead is a no-op off Linux.
+//
+// macOS has `F_RDADVISE` and Windows has `FILE_FLAG_SEQUENTIAL_SCAN`,
+// and neither is wired up here for the reason the scan concurrency
+// heuristic is not either: this package cannot tell a spinning disk
+// from an SSD on those platforms (see system.ProfileForPath), so it
+// would be prefetching without knowing whether prefetching is what the
+// device wants.
+func hintReadahead(_ string, _ int64) {}

backend/library/scan_workers_test.go

@@ -0,0 +1,122 @@
+package library
+
+import (
+	"context"
+	"testing"
+
+	"yellowjacket/backend/system"
+)
+
+// How many workers a scan gets is decided by two facts about the
+// device, and the second one is new: a spinning disk that can queue
+// commands wants several reads in flight, and one that cannot wants
+// almost none.  Before this it was a flat 2 for anything rotational,
+// which is a pre-NCQ assumption — a modern SATA disk reports a queue
+// depth of 32 and was being given a quarter of what it can use.
+func TestWorkersForProfile(t *testing.T) {
+	t.Parallel()
+
+	const cpus = 16
+
+	ssd := system.DiskProfile{Device: "sda", QueueDepth: 32}
+	hddQueued := system.DiskProfile{
+		Device: "sdb", Rotational: true, QueueDepth: 32,
+	}
+	hddSerial := system.DiskProfile{
+		Device: "sdc", Rotational: true, QueueDepth: 1,
+	}
+	// Neither NVMe nor a device-mapper volume publishes queue_depth.
+	// An unknown depth must not be read as "cannot queue", or every
+	// such device would be scanned as if it were a 2003 drive.
+	unknown := system.DiskProfile{Device: "dm-0", Rotational: true}
+
+	tests := []struct {
+		name    string
+		mode    ScanConcurrency
+		profile system.DiskProfile
+		want    int
+	}{
+		{"ssd auto", ScanConcurrencyAuto, ssd, cpus},
+		{"queueing hdd auto", ScanConcurrencyAuto, hddQueued, hddWorkerCountQueued},
+		{"serial hdd auto", ScanConcurrencyAuto, hddSerial, hddWorkerCountSerial},
+		{"unknown depth queues", ScanConcurrencyAuto, unknown, hddWorkerCountQueued},
+
+		// The mode overrules detection about the *disk*, never about
+		// its queue: forcing hdd on a queueing drive still uses it.
+		{"forced hdd on an ssd", ScanConcurrencyHDD, ssd, hddWorkerCountQueued},
+		{"forced ssd on an hdd", ScanConcurrencySSD, hddQueued, cpus},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			if got := workersForProfile(tt.mode, tt.profile, cpus); got != tt.want {
+				t.Errorf(
+					"workersForProfile(%q, %+v) = %d, want %d",
+					tt.mode, tt.profile, got, tt.want,
+				)
+			}
+		})
+	}
+}
+
+// A machine with fewer cores than the policy asks for gets its cores.
+func TestWorkersNeverExceedTheCPUCount(t *testing.T) {
+	t.Parallel()
+
+	hdd := system.DiskProfile{Rotational: true, QueueDepth: 32}
+
+	if got := workersForProfile(ScanConcurrencyAuto, hdd, 1); got != 1 {
+		t.Errorf("single-core hdd = %d workers, want 1", got)
+	}
+}
+
+// The prefetch stage must forward every item and nothing else: it is a
+// pass-through with a side effect, and a scan that drops a file because
+// of a *hint* would be a spectacular way to lose part of a library.
+func TestReadaheadForwardsEveryFile(t *testing.T) {
+	t.Parallel()
+
+	in := make(chan scanWork, 4)
+	for _, p := range []string{"/a", "/b", "/c", "/d"} {
+		in <- scanWork{absolutePath: p}
+	}
+
+	close(in)
+
+	var got []string
+	for w := range readaheadWork(
+		context.Background(),
+		in,
+		system.DiskProfile{Rotational: true, QueueDepth: 32},
+	) {
+		got = append(got, w.absolutePath)
+	}
+
+	want := []string{"/a", "/b", "/c", "/d"}
+	if len(got) != len(want) {
+		t.Fatalf("forwarded %v, want %v", got, want)
+	}
+
+	for i := range want {
+		if got[i] != want[i] {
+			t.Errorf("item %d = %q, want %q", i, got[i], want[i])
+		}
+	}
+}
+
+// On an SSD the stage is not inserted at all — the channel comes back
+// unchanged, so a scan there pays nothing for a feature it cannot use.
+func TestReadaheadIsSkippedOnSolidState(t *testing.T) {
+	t.Parallel()
+
+	in := make(chan scanWork)
+	out := readaheadWork(
+		context.Background(), in, system.DiskProfile{QueueDepth: 32},
+	)
+
+	if out != (<-chan scanWork)(in) {
+		t.Error("an ssd must get the original channel, unwrapped")
+	}
+}

backend/system/disktype_linux.go

@@ -16,32 +16,107 @@ var errNoBlockDevice = errors.New(
 	"no matching block device found",
 )
 
-// IsRotationalDisk reports whether the block device backing the
+// DiskProfile is what the scanner needs to know about the device a
-// given path is a rotational (spinning) disk.  Detection uses the
+// library sits on.  Both fields are about the same question — how many
-// Linux sysfs interface at /sys/block/<dev>/queue/rotational.
+// reads should be in flight at once — and they answer different halves
-// Returns false on any error (assumes SSD).
+// of it, so they travel together rather than as two probes.
-func IsRotationalDisk(path string) bool {
+type DiskProfile struct {
-	dev, err := deviceForPath(path)
+	// Device is the whole-disk kernel name ("sdb"), or "" when the
-	if err != nil {
+	// path could not be resolved to one.
-		return false
+	Device string
-	}
 
-	rotational, err := os.ReadFile(
+	// Rotational is /sys/block/<dev>/queue/rotational: true for a
-		filepath.Join(
+	// spinning disk, where a seek costs milliseconds.
-			"/sys/block", dev, "queue", "rotational",
+	Rotational bool
-		),
-	)
-	if err != nil {
-		return false
-	}
 
-	return strings.TrimSpace(string(rotational)) == "1"
+	// QueueDepth is /sys/block/<dev>/device/queue_depth — how many
+	// commands the drive will accept and reorder at once.  This is
+	// NCQ: a SATA disk with it enabled reports 31 or 32, and one
+	// without reports 1.  Zero means the file was not there to read,
+	// which is the case for anything that is not a SCSI/SATA device
+	// (NVMe, MMC, device-mapper, loop, a VM's virtio disk).
+	//
+	// It is the difference between concurrency helping and hurting.
+	// With queueing, several outstanding reads let the drive service
+	// them in the order its head passes over them, which is most of
+	// why a parallel scan is faster at all.  Without it, every extra
+	// worker is one more seek competing for one head, and the scan
+	// gets slower the harder it is pushed.
+	QueueDepth int
 }
 
-// deviceForPath resolves a filesystem path to its underlying block
+// Queues reports whether the drive can reorder outstanding commands.
-// device name (e.g. "sda") by matching the device major:minor
+//
-// from stat(2) against /sys/block/ entries.
+// An unknown depth (0) counts as queueing: everything that does not
-func deviceForPath(path string) (string, error) {
+// publish this file is a device where concurrency is fine — NVMe has
+// its own queues, virtio and device-mapper are not the physical layer
+// at all.  The only case worth being careful about is the one that
+// says so explicitly.
+func (p DiskProfile) Queues() bool {
+	return p.QueueDepth != 1
+}
+
+// IsRotationalDisk reports whether the block device backing the
+// given path is a rotational (spinning) disk.  Returns false on any
+// error (assumes SSD).
+func IsRotationalDisk(path string) bool {
+	return ProfileForPath(path).Rotational
+}
+
+// ProfileForPath describes the device backing a filesystem path.  A
+// path that cannot be resolved yields the zero profile, which reads as
+// "not rotational, queueing" — the permissive answer, since assuming a
+// spinning disk on an SSD would halve a scan for nothing.
+func ProfileForPath(path string) DiskProfile {
+	dev, err := diskForPath(path)
+	if err != nil {
+		return DiskProfile{}
+	}
+
+	return DiskProfile{
+		Device:     dev,
+		Rotational: sysfsInt(dev, "queue", "rotational") == 1,
+		QueueDepth: sysfsInt(dev, "device", "queue_depth"),
+	}
+}
+
+// sysfsInt reads one small integer out of /sys/block/<dev>/<parts...>,
+// returning 0 when it is absent or unparseable.  Every attribute here
+// is optional: sysfs layout varies by driver, and a missing file is
+// "this device does not say", never an error worth propagating.
+func sysfsInt(dev string, parts ...string) int {
+	p := filepath.Join(
+		append([]string{"/sys/block", dev}, parts...)...,
+	)
+
+	data, err := os.ReadFile(p) //nolint:gosec // sysfs, name from the kernel
+	if err != nil {
+		return 0
+	}
+
+	n, err := strconv.Atoi(strings.TrimSpace(string(data)))
+	if err != nil {
+		return 0
+	}
+
+	return n
+}
+
+// diskForPath resolves a filesystem path to the *whole disk* backing
+// it — "sdb" for a file on "sdb3".
+//
+// It goes through /sys/dev/block/<major>:<minor>, which the kernel
+// maintains as a symlink to the device's own sysfs directory, and then
+// walks up to the parent when that directory turns out to be a
+// partition.  The previous implementation scanned /sys/block comparing
+// dev numbers and, failing an exact match, took the first entry whose
+// *major* agreed — and every SATA disk shares major 8.  So a library on
+// /dev/sdb3 resolved to whatever /sys/block listed first, which is
+// alphabetical, which is sda.  On the machine this was found on that
+// meant a 6 TB spinning disk was read as the SSD next to it and scanned
+// with one worker per core.  Matching on major alone cannot be right
+// whenever a machine has two disks, which is the case this exists for.
+func diskForPath(path string) (string, error) {
 	var st syscall.Stat_t
 	if err := syscall.Stat(path, &st); err != nil {
 		return "", fmt.Errorf(
@@ -49,48 +124,50 @@ func deviceForPath(path string) (string, error) {
 		)
 	}
 
-	// Extract major and minor device numbers.
+	// Linux packs dev_t as 12 bits of major and 20 of minor, split
-	major := (st.Dev >> 8) & 0xff
+	// across the word.  Masking the low byte of each — which is what
-	minor := st.Dev & 0xff
+	// this used to do — is right only for the first 256 of either.
+	major := unixMajor(uint64(st.Dev))
+	minor := unixMinor(uint64(st.Dev))
 
-	// Scan /sys/block/ for a matching device.
+	link := filepath.Join(
-	entries, err := os.ReadDir("/sys/block")
+		"/sys/dev/block",
+		strconv.FormatUint(major, 10)+":"+
+			strconv.FormatUint(minor, 10),
+	)
+
+	target, err := filepath.EvalSymlinks(link)
 	if err != nil {
 		return "", fmt.Errorf(
-			"could not read /sys/block: %w", err,
+			"%w: %s (%w)", errNoBlockDevice, link, err,
 		)
 	}
 
-	majorStr := strconv.FormatUint(major, 10)
+	// A partition's directory sits inside its disk's, and only the
-	devStr := majorStr + ":" +
+	// disk carries `queue`. Climb at most one level: sysfs nests a
-		strconv.FormatUint(minor, 10)
+	// partition exactly one deep under its disk.
+	name := filepath.Base(target)
 
-	for _, entry := range entries {
+	if _, err := os.Stat(filepath.Join(target, "queue")); err != nil {
-		devFile := filepath.Join(
+		name = filepath.Base(filepath.Dir(target))
-			"/sys/block", entry.Name(), "dev",
-		)
-
-		data, err := os.ReadFile(devFile)
-		if err != nil {
-			continue
-		}
-
-		content := strings.TrimSpace(string(data))
-
-		if content == devStr {
-			return entry.Name(), nil
-		}
-
-		// The filesystem might be on a partition (e.g. sda1)
-		// whose parent block device is sda.  Check if the
-		// major number matches.
-		parts := strings.SplitN(content, ":", 2)
-		if len(parts) == 2 && parts[0] == majorStr {
-			return entry.Name(), nil
-		}
 	}
 
-	return "", fmt.Errorf(
+	if name == "" || name == "." || name == string(filepath.Separator) {
-		"%w for %s", errNoBlockDevice, devStr,
+		return "", fmt.Errorf(
-	)
+			"%w for %d:%d", errNoBlockDevice, major, minor,
+		)
+	}
+
+	return name, nil
+}
+
+// unixMajor and unixMinor decode a Linux dev_t.  Spelled out rather
+// than taken from golang.org/x/sys/unix so this file stays readable
+// beside the encoding it is undoing.
+func unixMajor(dev uint64) uint64 {
+	return (dev>>8)&0xfff | (dev >> 32 & ^uint64(0xfff))
+}
+
+func unixMinor(dev uint64) uint64 {
+	return dev&0xff | (dev >> 12 & ^uint64(0xff))
 }

backend/system/disktype_other.go

@@ -2,9 +2,34 @@
 
 package system
 
+// DiskProfile is what the scanner needs to know about the device a
+// library sits on.  See the Linux implementation for what each field
+// means; off Linux nothing fills them, because neither macOS nor
+// Windows publishes an equivalent of sysfs's `rotational` and
+// `queue_depth` without going through platform APIs this package
+// deliberately does not link.
+type DiskProfile struct {
+	Device     string
+	Rotational bool
+	QueueDepth int
+}
+
+// Queues reports whether the drive can reorder outstanding commands.
+// Always true here: an unknown depth is the permissive answer, and
+// assuming otherwise would halve every scan on every Mac.
+func (p DiskProfile) Queues() bool {
+	return p.QueueDepth != 1
+}
+
 // IsRotationalDisk reports whether the block device backing the
 // given path is a rotational (spinning) disk.  On non-Linux
 // platforms this always returns false (assumes SSD).
 func IsRotationalDisk(_ string) bool {
 	return false
 }
+
+// ProfileForPath describes the device backing a filesystem path.  Off
+// Linux that is the zero profile, which reads as "an SSD that queues".
+func ProfileForPath(_ string) DiskProfile {
+	return DiskProfile{}
+}

e2e/specs/requested-badge.spec.ts

@@ -7,7 +7,7 @@ import { test, expect, callBinding } from '../support/fixtures.js';
  * and produced two: every one of the eight call sites was a two-way
  * ternary, so an album already on the request list showed a plus and
  * said "is not in your library" — on the same page, forty pixels from a
- * filled button reading "Wanted".
+ * filled button reading "Requested".
  *
  * This spec exists at this tier rather than only in the component one
  * because of what it drags in with it: reaching the requested state is
@@ -181,7 +181,7 @@ test.describe('the requested badge', () => {
             const ds = document.querySelector('explore-album-details')
               ?.shadowRoot;
             const btn = [...(ds?.querySelectorAll('wa-button') ?? [])].find(
-              (b) => /Wanted/.test(b.textContent ?? ''),
+              (b) => /Requested/.test(b.textContent ?? ''),
             );
 
             return btn?.querySelector('wa-icon')?.getAttribute('name') ?? '';

frontend/bindings/yellowjacket/backend/download/models.ts

@@ -3,28 +3,52 @@
 
 /**
  * AutoDownloadPrefs gates and scores what AutoPickable may choose
- * without asking.  Zero values are permissive: no size window and no
+ * without asking.  Zero values are permissive: no bitrate window, no
- * format restriction.
+ * size ceiling and no format restriction.
+ * 
+ * **The window is a rate, not a size.**  It used to be three numbers in
+ * megabytes, which cannot mean anything on their own: 300 MB is a
+ * generous FLAC single and a suspiciously small boxset, and the user
+ * setting the number has no idea which release the pipeline will
+ * eventually apply it to.  A bitrate is the same statement normalised
+ * by how long the music is, so one number holds across a 9-minute EP
+ * and a 3-hour opera — and it is the unit the thing being described is
+ * actually measured in.  The runtime is known for every request
+ * auto-pick can act on (`Download.Expected` carries per-track lengths,
+ * and an anchored request is the only kind that reaches here), so this
+ * costs no extra lookup.
  */
 export interface AutoDownloadPrefs {
     /**
-     * MinSizeMB and MaxSizeMB bound what auto-pick will grab.  Zero
+     * MinKbps and MaxKbps bound the average bitrate auto-pick will
-     * means no bound on that side.  A candidate outside the window is
+     * grab.  Zero means no bound on that side.  A candidate outside the
-     * filtered out of auto-pick entirely, not merely scored down — a
+     * window is filtered out of auto-pick entirely, not merely scored
-     * tiny "sampler" torrent or a boxset ten times the expected size is
+     * down — a 96 kbps rip of the right album is not a worse copy the
-     * usually the wrong thing entirely, not a worse copy of the right
+     * user might accept, it is one they said not to take unattended.
-     * thing.
+     * 
+     * For reference: 320 is the top of MP3, ~500–1000 is FLAC depending
+     * on the material, and anything under ~128 is a transcode.
      */
-    "minSizeMb": number;
+    "minKbps": number;
-    "maxSizeMb": number;
+    "maxKbps": number;
 
     /**
-     * PreferredSizeMB nudges the score toward a target size within the
+     * PreferredKbps nudges the score toward a target rate within the
-     * min/max window (a lossless rip and a heavily-padded lossless rip
+     * window, and breaks the tie when several candidates are equally
-     * can both pass the window).  Zero disables the nudge; sizeFit then
+     * good matches.  Zero disables the nudge; bitrateFit then returns a
-     * returns a neutral value that does not affect ranking.
+     * neutral value that does not affect ranking.
      */
-    "preferredSizeMb": number;
+    "preferredKbps": number;
+
+    /**
+     * MaxSizeMB is a hard ceiling on the whole candidate, and it is
+     * deliberately still a size.  It answers a different question from
+     * the window above — not "is this the quality I want" but "is this
+     * going to fill the disk" — and it has to hold even for a candidate
+     * whose bitrate cannot be worked out, which is exactly the shape a
+     * mislabelled boxset arrives in.  Zero means no ceiling.
+     */
+    "maxSizeMb": number;
 
     /**
      * AllowedFormats restricts auto-pick to candidates whose audio
@@ -487,9 +511,13 @@ export interface QualityScore {
     "priority": number;
 
     /**
-     * closeness to the preferred download size
+     * BitrateFit is closeness to the preferred *rate*, which is what
+     * the auto-download window is expressed in.  It replaced a
+     * `SizeFit` measured in megabytes: a size means nothing without
+     * knowing how long the music is, so the same number described a
+     * generous single and a suspiciously small boxset.
      */
-    "sizeFit": number;
+    "bitrateFit": number;
 
     /**
      * Mixed marks a candidate whose files are not all the same format,

frontend/src/assets/icons/fa/regular/star.svg

@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 576 512"><!--! Font Awesome Free 7.3.1 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL 1.1, Code: MIT License) Copyright 2026 Fonticons, Inc. --><path fill="currentColor" d="M288.1-32c9 0 17.3 5.1 21.4 13.1L383 125.3 542.9 150.7c8.9 1.4 16.3 7.7 19.1 16.3s.5 18-5.8 24.4L441.7 305.9 467 465.8c1.4 8.9-2.3 17.9-9.6 23.2s-17 6.1-25 2L288.1 417.6 143.8 491c-8 4.1-17.7 3.3-25-2s-11-14.2-9.6-23.2L134.4 305.9 20 191.4c-6.4-6.4-8.6-15.8-5.8-24.4s10.1-14.9 19.1-16.3l159.9-25.4 73.6-144.2c4.1-8 12.4-13.1 21.4-13.1zm0 76.8L230.3 158c-3.5 6.8-10 11.6-17.6 12.8l-125.5 20 89.8 89.9c5.4 5.4 7.9 13.1 6.7 20.7l-19.8 125.5 113.3-57.6c6.8-3.5 14.9-3.5 21.8 0l113.3 57.6-19.8-125.5c-1.2-7.6 1.3-15.3 6.7-20.7l89.8-89.9-125.5-20c-7.6-1.2-14.1-6-17.6-12.8L288.1 44.8z"/></svg>

frontend/src/components/artists-view/artists-view.ts

@@ -10,6 +10,7 @@ import type {
     VisibilityChangedEvent,
 } from '@lit-labs/virtualizer';
 import { grid } from '@lit-labs/virtualizer/layouts/grid.js';
+import { gridSpacingFor } from '@utils/grid-spacing';
 import {
     GetAlbumsByArtist,
     GetFilePathsByAlbums,
@@ -147,8 +148,6 @@ export class ArtistsView
 
     // ----- Grid spacing constants -----
 
-    private static readonly GRID_GAP = 8;
-    private static readonly GRID_PADDING = 8;
     private static readonly CARD_PADDING = 5;
 
     private get imageSize(): number {
@@ -177,20 +176,41 @@ export class ArtistsView
     private createGridLayout() {
         const w = this.cardSize ?? CARD_SIZE_DEFAULT;
         const h = w + this.cardTextHeight;
-        const gap = ArtistsView.GRID_GAP;
+
-        const pad = ArtistsView.GRID_PADDING;
+        // One number for the gap, the row gap and the padding: whatever
+        // a row could not spend on another card, shared out equally, so
+        // the outside is never wider than the inside.  See
+        // `utils/grid-spacing.ts`.
+        const spacing = this.spacingFor(this.containerWidth);
+
+        this.lastLayoutSpacing = spacing;
 
         return grid({
             itemSize: {
                 width: `${w}px`,
                 height: `${h}px`,
             },
-            gap: `${gap}px`,
+            gap: `${spacing}px`,
-            padding: `${pad}px`,
+            padding: `${spacing}px`,
-            justify: 'center',
+            justify: 'start',
         });
     }
 
+    /** The width the grid lays itself out in. */
+    private get containerWidth(): number {
+        return (
+            this.renderRoot?.querySelector<HTMLElement>(
+                '.grid-scroll-container',
+            )?.clientWidth ||
+            this.clientWidth ||
+            0
+        );
+    }
+
+    private spacingFor(width: number): number {
+        return gridSpacingFor(width, this.cardSize);
+    }
+
     /** Sort direction for the artist grid.
      *
      *  There is only one key to sort by: `library.Artist` carries a
@@ -478,6 +498,8 @@ export class ArtistsView
     override disconnectedCallback() {
         super.disconnectedCallback();
         this.detachWheelListener();
+        this.gridResizeObserver?.disconnect();
+        this.gridResizeObserver = null;
     }
 
     /** The wheel listener and the scroll debounce belong to the grid
@@ -730,10 +752,34 @@ export class ArtistsView
      * ================================================================ */
 
     private lastLayoutWidth = 0;
+    private lastLayoutSpacing = 0;
+
+    /** Watches the scroller so a window resize rebuilds the layout:
+     *  the spacing is derived from its width, and nothing else asks
+     *  this view to update when only that changes. */
+    private gridResizeObserver: ResizeObserver | null = null;
+
+    private observeGridWidth() {
+        const container =
+            this.renderRoot?.querySelector<HTMLElement>(
+                '.grid-scroll-container',
+            );
+
+        if (!container || this.gridResizeObserver) return;
+
+        this.gridResizeObserver = new ResizeObserver(() =>
+            this.requestUpdate(),
+        );
+        this.gridResizeObserver.observe(container);
+    }
 
     private updateGridLayout() {
+        this.observeGridWidth();
+
         if (
-            this.cardSize === this.lastLayoutWidth
+            this.cardSize === this.lastLayoutWidth &&
+            this.lastLayoutSpacing ===
+                this.spacingFor(this.containerWidth)
         ) {
             return;
         }

frontend/src/components/config-page/download-clients.ts

@@ -86,9 +86,10 @@ export class DownloadClients extends LitElement {
     /** Working copy of the auto-download guardrails. */
     @state()
     private prefs: download.AutoDownloadPrefs = {
-        minSizeMb: 0,
+        minKbps: 0,
+        maxKbps: 0,
+        preferredKbps: 0,
         maxSizeMb: 0,
-        preferredSizeMb: 0,
         allowedFormats: [],
     } as download.AutoDownloadPrefs;
 
@@ -284,25 +285,72 @@ export class DownloadClients extends LitElement {
                     : nothing}
 
                 <div class="form">
+                    <!-- Bitrate, not megabytes.  A size means nothing
+                         on its own: 300 MB is a generous single and a
+                         suspiciously small boxset, and whoever fills
+                         this in has no idea which release it will be
+                         applied to.  A rate is the same statement
+                         divided by how long the music is, so one number
+                         holds across an EP and an opera. -->
                     <div class="field-row">
                         <wa-input
-                            label="Minimum size (MB)"
+                            label="Minimum bitrate (kbps)"
                             type="number"
                             min="0"
                             placeholder="No minimum"
-                            .value=${this.prefs.minSizeMb ? String(this.prefs.minSizeMb) : ''}
+                            .value=${this.prefs.minKbps ? String(this.prefs.minKbps) : ''}
                             @input=${(e: Event) => {
                                 this.prefs = {
                                     ...this.prefs,
-                                    minSizeMb: Number((e.target as HTMLInputElement).value) || 0,
+                                    minKbps: Number((e.target as HTMLInputElement).value) || 0,
                                 };
                             }}
                         ></wa-input>
                         <wa-input
-                            label="Maximum size (MB)"
+                            label="Maximum bitrate (kbps)"
                             type="number"
                             min="0"
                             placeholder="No maximum"
+                            .value=${this.prefs.maxKbps ? String(this.prefs.maxKbps) : ''}
+                            @input=${(e: Event) => {
+                                this.prefs = {
+                                    ...this.prefs,
+                                    maxKbps: Number((e.target as HTMLInputElement).value) || 0,
+                                };
+                            }}
+                        ></wa-input>
+                        <wa-input
+                            label="Preferred bitrate (kbps)"
+                            type="number"
+                            min="0"
+                            placeholder="No preference"
+                            .value=${this.prefs.preferredKbps
+                                ? String(this.prefs.preferredKbps)
+                                : ''}
+                            @input=${(e: Event) => {
+                                this.prefs = {
+                                    ...this.prefs,
+                                    preferredKbps:
+                                        Number((e.target as HTMLInputElement).value) || 0,
+                                };
+                            }}
+                        ></wa-input>
+                    </div>
+
+                    <div class="requires">
+                        320 is the top of MP3; a FLAC rip is usually
+                        500–1000 depending on the music. Preferred
+                        decides between copies that are otherwise equally
+                        good — it never rules one out, which is what the
+                        minimum and maximum are for.
+                    </div>
+
+                    <div class="field-row">
+                        <wa-input
+                            label="Never grab more than (MB)"
+                            type="number"
+                            min="0"
+                            placeholder="No limit"
                             .value=${this.prefs.maxSizeMb ? String(this.prefs.maxSizeMb) : ''}
                             @input=${(e: Event) => {
                                 this.prefs = {
@@ -311,22 +359,14 @@ export class DownloadClients extends LitElement {
                                 };
                             }}
                         ></wa-input>
-                        <wa-input
+                    </div>
-                            label="Preferred size (MB)"
+
-                            type="number"
+                    <div class="requires">
-                            min="0"
+                        A ceiling on the download itself, in case a
-                            placeholder="No preference"
+                        mislabelled boxset gets through. Still a size
-                            .value=${this.prefs.preferredSizeMb
+                        because it is a question about disk space, and
-                                ? String(this.prefs.preferredSizeMb)
+                        because it has to apply to a candidate whose
-                                : ''}
+                        bitrate cannot be worked out at all.
-                            @input=${(e: Event) => {
-                                this.prefs = {
-                                    ...this.prefs,
-                                    preferredSizeMb:
-                                        Number((e.target as HTMLInputElement).value) || 0,
-                                };
-                            }}
-                        ></wa-input>
                     </div>
 
                     <div>

frontend/src/components/cover-grid/cover-grid.ts

@@ -19,6 +19,7 @@ import { LibraryController } from '@store/controllers/library-controller';
 import { SearchController } from '@store/controllers/search-controller';
 import { ViewLifecycleMixin } from '@utils/view-lifecycle';
 import { RovingGridController } from '@utils/roving-grid';
+import { gridColumnsFor, gridSpacingFor } from '@utils/grid-spacing';
 import { queueStore } from '@store/queue-store';
 import type { QueueSource } from '@store/queue-store';
 import '@awesome.me/webawesome/dist/components/popup/popup.js';
@@ -97,19 +98,36 @@ export class CoverGrid
     private lastAlbumsRef: library.Album[] | null =
         null;
 
-    // Fixed grid spacing constants.
-    private static readonly GRID_GAP = 8;
-    private static readonly GRID_PADDING = 8;
     private static readonly CARD_PADDING = 5;
 
     private ctxMenu = new ContextMenuController(this);
     private favCtrl = new FavoritesController(this);
     private selMgr = new AlbumSelectionManager();
     private scrollMgr = new ScrollManager(this, {
-        GRID_GAP: CoverGrid.GRID_GAP,
+        columnsFor: (width: number) => this.columnsFor(width),
-        GRID_PADDING: CoverGrid.GRID_PADDING,
+        spacingFor: (width: number) => this.spacingFor(width),
     });
 
+    /**
+     * How many cards fit across `width`, by the same arithmetic the
+     * virtualizer's `space-evenly` grid uses — no gap and no padding
+     * are reserved, because both come out of what is left over.
+     *
+     * The scroll manager restores a position by rebuilding the grid's
+     * geometry, so this and `spacingFor` must agree with the layout
+     * rather than approximate it; they were two constants that no
+     * longer describe anything once the spacing became elastic.
+     */
+    columnsFor(width: number): number {
+        return gridColumnsFor(width, this.cardWidth);
+    }
+
+    /** The spacing that width produces: between columns, between rows,
+     *  and around the outside, all the same number. */
+    spacingFor(width: number): number {
+        return gridSpacingFor(width, this.cardWidth);
+    }
+
     private lastSelectedAlbumIndex: number | null = null;
     private lastSelectedTrackIndex: number | null = null;
 
@@ -148,10 +166,30 @@ export class CoverGrid
     }
 
     // Virtualizer grid layout instance — recreated when
-    // the card size changes.
+    // the card size or the container width changes.
     private gridLayout = this.createGridLayout();
     private gridLayoutWidth = 0;
 
+    /** The spacing the current layouts were built with. */
+    private gridLayoutSpacing = 0;
+
+    /** Watches the scroll container so a window resize rebuilds the
+     *  layout: the spacing is derived from its width, and nothing else
+     *  asks this component to update when only that changes. */
+    private gridResizeObserver: ResizeObserver | null =
+        null;
+
+    private observeGridWidth(): void {
+        const container = this.scrollContainer;
+
+        if (!container || this.gridResizeObserver) return;
+
+        this.gridResizeObserver = new ResizeObserver(
+            () => this.requestUpdate(),
+        );
+        this.gridResizeObserver.observe(container);
+    }
+
     /**
      * Secondary layout for the "after" virtualizer in
      * split mode.  Uses zero top padding so there is no
@@ -169,22 +207,49 @@ export class CoverGrid
         }
 
         const h = w + this.cardTextHeight;
-        const gap = CoverGrid.GRID_GAP;
+
-        const pad = CoverGrid.GRID_PADDING;
+        // The spacing is whatever the row could not spend on another
+        // card, shared out equally — so it is the same number between
+        // two cards, between two rows, and down each outside edge.
+        // See `utils/grid-spacing.ts` for why it is computed rather
+        // than handed to the virtualizer as `space-evenly`.
+        const spacing = this.spacingFor(
+            this.containerWidth,
+        );
+
+        if (!noTopPad) {
+            this.gridLayoutSpacing = spacing;
+        }
 
         return grid({
             itemSize: {
                 width: `${w}px`,
                 height: `${h}px`,
             },
-            gap: `${gap}px`,
+            gap: `${spacing}px`,
             padding: noTopPad
-                ? `0 ${pad}px ${pad}px`
+                ? `0 ${spacing}px ${spacing}px`
-                : `${pad}px`,
+                : `${spacing}px`,
-            justify: 'center',
+            justify: 'start',
         });
     }
 
+    /**
+     * The width the grid lays itself out in.
+     *
+     * Read from the scroll container when there is one; before the
+     * first render there is not, and the fallback only has to be
+     * plausible — the layout is rebuilt from the real width as soon as
+     * one exists.
+     */
+    private get containerWidth(): number {
+        return (
+            this.scrollContainer?.clientWidth ||
+            this.clientWidth ||
+            0
+        );
+    }
+
     private dragImageEl: HTMLElement | null = null;
 
     // -- Memoisation caches for filtered albums --
@@ -466,6 +531,9 @@ export class CoverGrid
         );
         this.wheelListenerAttached = false;
 
+        this.gridResizeObserver?.disconnect();
+        this.gridResizeObserver = null;
+
         this.scrollMgr.teardown();
         this.scrollMgr.revealContainer(
             this.scrollContainer,
@@ -603,10 +671,18 @@ export class CoverGrid
             this.wheelListenerAttached = true;
         }
 
-        // Recreate the virtualizer grid layout when
+        this.observeGridWidth();
-        // the card size changes.
+
+        // Recreate the virtualizer grid layout when the card size
+        // changes — or when the spacing the container width produces
+        // does, since that is now a derived number rather than a
+        // constant.  Keyed on the spacing rather than on the width, or
+        // every pixel of a drag rebuilds a layout that would come out
+        // the same.
         const cardSizeChanged =
-            this.gridLayoutWidth !== this.cardWidth;
+            this.gridLayoutWidth !== this.cardWidth ||
+            this.gridLayoutSpacing !==
+                this.spacingFor(this.containerWidth);
 
         if (cardSizeChanged) {
             this.gridLayout = this.createGridLayout();

frontend/src/components/cover-grid/scroll-manager.ts

@@ -6,12 +6,21 @@ import type { LibraryController } from '@store/controllers/library-controller';
 import type { GridEntry } from './cover-grid-types.js';
 
 /**
- * Grid spacing constants shared between the scroll
+ * Grid geometry, asked of the host rather than written down.
- * manager and the host component.
+ *
+ * These were two constants, `GRID_GAP` and `GRID_PADDING`, which stopped
+ * describing anything the moment the grid's spacing became elastic: the
+ * gap, the padding and the column count are all derived from the
+ * container width now, and a scroll position rebuilt from a stale 8px
+ * lands in the wrong row.
  */
 export interface GridConstants {
-    readonly GRID_GAP: number;
+    /** Columns that fit across `width`. */
-    readonly GRID_PADDING: number;
+    columnsFor(width: number): number;
+
+    /** The spacing `width` produces — between columns, between rows,
+     *  and around the outside, all the same number. */
+    spacingFor(width: number): number;
 }
 
 /**
@@ -275,8 +284,8 @@ export class ScrollManager {
                 return;
             }
 
-            const gap = this.gc.GRID_GAP;
+            const gap = this.spacing(container);
-            const pad = this.gc.GRID_PADDING;
+            const pad = gap;
             const rowStep =
                 this.host.cardHeight + gap;
 
@@ -293,7 +302,7 @@ export class ScrollManager {
             () => {
                 const rowStep =
                     this.host.cardHeight +
-                    this.gc.GRID_GAP;
+                    this.spacing(container);
 
                 if (this.pendingFocus === null) {
                     this.isResizing = true;
@@ -351,7 +360,7 @@ export class ScrollManager {
         container: HTMLElement,
         rowStep: number,
     ): void {
-        const pad = this.gc.GRID_PADDING;
+        const pad = this.spacing(container);
         const cols = this.currentColumnCount;
         const filtered =
             this.host.cachedFilteredAlbums;
@@ -410,17 +419,15 @@ export class ScrollManager {
     ): number {
         if (!container) return 1;
 
-        const gap = this.gc.GRID_GAP;
+        return this.gc.columnsFor(
-        const pad = this.gc.GRID_PADDING;
+            container.clientWidth,
-        const availableWidth =
+        );
-            container.clientWidth - pad * 2;
+    }
 
-        return Math.max(
+    /** The grid's current spacing, which is also its padding. */
-            1,
+    private spacing(container?: HTMLElement): number {
-            Math.floor(
+        return this.gc.spacingFor(
-                (availableWidth + gap) /
+            container?.clientWidth ?? 800,
-                    (this.host.cardWidth + gap),
-            ),
         );
     }
 
@@ -439,7 +446,7 @@ export class ScrollManager {
         container?: HTMLElement,
     ): number {
         const cols = this.getColumnCount(container);
-        const gap = this.gc.GRID_GAP;
+        const gap = this.spacing(container);
 
         return (
             cols * this.host.cardWidth +
@@ -460,7 +467,7 @@ export class ScrollManager {
 
         const cols = this.getColumnCount(container);
         const colIndex = idx % cols;
-        const gap = this.gc.GRID_GAP;
+        const gap = this.spacing(container);
 
         return (
             colIndex *
@@ -597,8 +604,8 @@ export class ScrollManager {
 
         if (!this.host.splitMode) return raw;
 
-        const gap = this.gc.GRID_GAP;
+        const gap = this.spacing(container);
-        const pad = this.gc.GRID_PADDING;
+        const pad = gap;
         const columns =
             this.getColumnCount(container);
         const rowStep = this.host.cardHeight + gap;
@@ -678,8 +685,8 @@ export class ScrollManager {
 
         if (expandedIndex < 0) return;
 
-        const gap = this.gc.GRID_GAP;
+        const gap = this.spacing(container);
-        const pad = this.gc.GRID_PADDING;
+        const pad = gap;
         const columns =
             this.getColumnCount(container);
         const rowStep = this.host.cardHeight + gap;
@@ -772,8 +779,8 @@ export class ScrollManager {
 
         if (idx < 0) return;
 
-        const gap = this.gc.GRID_GAP;
+        const gap = this.spacing(container);
-        const pad = this.gc.GRID_PADDING;
+        const pad = gap;
         const cols =
             this.getColumnCount(container);
         const rowStep = this.host.cardHeight + gap;
@@ -854,9 +861,8 @@ export class ScrollManager {
                         this.getExpandedAlbumIndex();
 
                     if (idx >= 0) {
-                        const gap = this.gc.GRID_GAP;
+                        const gap = this.spacing(container);
-                        const pad =
+                        const pad = gap;
-                            this.gc.GRID_PADDING;
                         const cols =
                             this.getColumnCount(
                                 container,

frontend/src/components/downloads-view/downloads-view.ts

@@ -400,7 +400,7 @@ export class DownloadsView extends ViewLifecycleMixin(LitElement) {
     private renderEmptyRequests() {
         return html`
             <div class="empty">
-                Nothing requested yet. Use “Want this” on an album or artist
+                Nothing requested yet. Use “Request this” on an album or artist
                 to add it here.
             </div>
         `;

frontend/src/components/explore-album-details/explore-album-details.ts

@@ -2680,7 +2680,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
                     slot="start"
                     name=${this.isRequested ? 'solid/bookmark' : 'regular/bookmark'}
                 ></wa-icon>
-                ${this.isRequested ? 'Wanted' : 'Want this'}
+                ${this.isRequested ? 'Requested' : 'Request this'}
             </wa-button>
         `;
     }

frontend/src/components/explore-artist-details/explore-artist-details.ts

@@ -2753,7 +2753,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
                               slot="icon"
                               name=${requested ? 'xmark' : 'bookmark'}
                           ></wa-icon>
-                          ${requested ? 'Cancel Request' : 'Want This'}
+                          ${requested ? 'Cancel Request' : 'Request This'}
                       </wa-dropdown-item>
                   `
                 : nothing}

frontend/src/components/explore-view/explore-view.ts

@@ -1509,9 +1509,24 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
                             if (url) {
                                 this.thumbnailCache.set(req.mbid, url);
                                 this.requestUpdate();
+
+                                return;
                             }
+
+                            // An empty answer is not necessarily "there
+                            // is no art" — a slow Internet Archive node
+                            // is answered by a timeout, which looks
+                            // exactly the same from here. Drop the
+                            // in-flight marker so the next time this
+                            // release group is on screen it is asked
+                            // again; the backend records a genuine 404
+                            // on disk and answers that one instantly,
+                            // so a real miss costs nothing to re-ask.
+                            this.thumbnailCache.delete(req.mbid);
                         })
-                        .catch(() => {});
+                        .catch(() => {
+                            this.thumbnailCache.delete(req.mbid);
+                        });
                 }
             })
             .catch(() => {

frontend/src/components/genres-view/genres-view.ts

@@ -10,6 +10,7 @@ import type {
     VisibilityChangedEvent,
 } from '@lit-labs/virtualizer';
 import { grid } from '@lit-labs/virtualizer/layouts/grid.js';
+import { gridSpacingFor } from '@utils/grid-spacing';
 import {
     GetFilePathsByGenres,
 } from '@go/library/library.js';
@@ -155,8 +156,6 @@ export class GenresView
 
     // ----- Grid spacing constants -----
 
-    private static readonly GRID_GAP = 8;
-    private static readonly GRID_PADDING = 8;
     private static readonly CARD_PADDING = 5;
 
     private get imageSize(): number {
@@ -185,20 +184,41 @@ export class GenresView
     private createGridLayout() {
         const w = this.cardSize ?? CARD_SIZE_DEFAULT;
         const h = w + this.cardTextHeight;
-        const gap = GenresView.GRID_GAP;
+
-        const pad = GenresView.GRID_PADDING;
+        // One number for the gap, the row gap and the padding: whatever
+        // a row could not spend on another card, shared out equally, so
+        // the outside is never wider than the inside.  See
+        // `utils/grid-spacing.ts`.
+        const spacing = this.spacingFor(this.containerWidth);
+
+        this.lastLayoutSpacing = spacing;
 
         return grid({
             itemSize: {
                 width: `${w}px`,
                 height: `${h}px`,
             },
-            gap: `${gap}px`,
+            gap: `${spacing}px`,
-            padding: `${pad}px`,
+            padding: `${spacing}px`,
-            justify: 'center',
+            justify: 'start',
         });
     }
 
+    /** The width the grid lays itself out in. */
+    private get containerWidth(): number {
+        return (
+            this.renderRoot?.querySelector<HTMLElement>(
+                '.grid-scroll-container',
+            )?.clientWidth ||
+            this.clientWidth ||
+            0
+        );
+    }
+
+    private spacingFor(width: number): number {
+        return gridSpacingFor(width, this.cardSize);
+    }
+
     /** Sort key and direction for the genre grid (H-19: it had none). */
     @state()
     private sortField: 'name' | 'tracks' = 'name';
@@ -483,6 +503,8 @@ export class GenresView
     override disconnectedCallback() {
         super.disconnectedCallback();
         this.detachWheelListener();
+        this.gridResizeObserver?.disconnect();
+        this.gridResizeObserver = null;
     }
 
     /** See artists-view: off-screen the grid cannot be scrolled, and
@@ -737,10 +759,34 @@ export class GenresView
      * ================================================================ */
 
     private lastLayoutWidth = 0;
+    private lastLayoutSpacing = 0;
+
+    /** Watches the scroller so a window resize rebuilds the layout:
+     *  the spacing is derived from its width, and nothing else asks
+     *  this view to update when only that changes. */
+    private gridResizeObserver: ResizeObserver | null = null;
+
+    private observeGridWidth() {
+        const container =
+            this.renderRoot?.querySelector<HTMLElement>(
+                '.grid-scroll-container',
+            );
+
+        if (!container || this.gridResizeObserver) return;
+
+        this.gridResizeObserver = new ResizeObserver(() =>
+            this.requestUpdate(),
+        );
+        this.gridResizeObserver.observe(container);
+    }
 
     private updateGridLayout() {
+        this.observeGridWidth();
+
         if (
-            this.cardSize === this.lastLayoutWidth
+            this.cardSize === this.lastLayoutWidth &&
+            this.lastLayoutSpacing ===
+                this.spacingFor(this.containerWidth)
         ) {
             return;
         }

frontend/src/components/library-status-indicator/library-status-indicator.ts

@@ -48,7 +48,7 @@ export type LibraryStatus =
  *
  * Colours and glyphs:
  *  - in-library    → green circle, check mark
- *  - queued        → amber circle, hourglass
+ *  - queued        → amber circle, bookmark ("on your list")
  *  - not-in-library → grey circle, plus sign
  *
  * Usage:
@@ -241,12 +241,23 @@ export class LibraryStatusIndicator extends LitElement {
         }
     `;
 
+    /**
+     * The glyph for each state.
+     *
+     * `queued` is a **bookmark**, not the hourglass it used to be. An
+     * hourglass says "wait, this is under way", which overstates what a
+     * request is: nothing may be downloading, nothing may ever be found,
+     * and the user can leave one sitting on the list indefinitely. A
+     * bookmark says the honest thing — it is on your list — and reads as
+     * the opposite of the plus that put it there, which is what a
+     * toggle's two states have to do.
+     */
     private iconName(): string {
         switch (this.status) {
             case 'in-library':
                 return 'check';
             case 'queued':
-                return 'hourglass-half';
+                return 'bookmark';
             default:
                 return 'plus';
         }
@@ -276,7 +287,7 @@ export class LibraryStatusIndicator extends LitElement {
         if (this.actionable) {
             return this.status === 'queued'
                 ? `Cancel the request for ${kind}${name}`
-                : `Want ${kind}${name}`;
+                : `Request ${kind}${name}`;
         }
 
         switch (this.status) {
@@ -354,25 +365,6 @@ export class LibraryStatusIndicator extends LitElement {
         }
 
         const title = this.tooltip();
-        const icon = this.iconName()
-            ? html`<wa-icon name=${this.iconName()} aria-hidden="true"></wa-icon>`
-            : nothing;
-
-        if (this.actionable) {
-            return html`
-                <button
-                    class="badge"
-                    type="button"
-                    title=${title}
-                    aria-label=${title}
-                    ?disabled=${this.busy}
-                    @click=${this.onActivate}
-                    @keydown=${this.onKeydown}
-                >
-                    ${icon}
-                </button>
-            `;
-        }
 
         // The ring stands in for the icon wherever the icon would go —
         // including inside the button, because a partly-held album is

frontend/src/components/now-playing-view/now-playing-view.ts

@@ -307,7 +307,7 @@ export class NowPlayingView extends LitElement {
                         : `Add ${track.title} to ${this.favCtrl.playlistName}`}
                     @click=${this.toggleFavorite}
                 >
-                    <wa-icon name=${this.favCtrl.iconName}></wa-icon>
+                    <wa-icon name=${this.favCtrl.iconFor(favorited)}></wa-icon>
                 </button>
             </div>
 

frontend/src/components/now-playing/now-playing.ts

@@ -527,7 +527,7 @@ export class NowPlaying extends LitElement {
                       )}
                 >
                   <wa-icon
-                    name=${this.favCtrl.iconName}
+                    name=${this.favCtrl.iconFor(isFav)}
                     variant=${favVariant}
                   ></wa-icon>
                 </button>

frontend/src/components/playlist-view/playlist-view.ts

@@ -1756,7 +1756,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
                     ${entry.summary.ID === this.favCtrl.playlistId
                         ? html`<wa-icon
                               class="playlist-icon"
-                              name=${this.favCtrl.iconName}
+                              name=${this.favCtrl.iconFor(true)}
                           ></wa-icon>`
                         : entry.summary.IsSmart
                           ? html`<wa-icon

frontend/src/icons/names.txt

@@ -16,6 +16,7 @@
 # fetch-icons.mjs for why that is not negotiable.  Re-vendor with:
 #   node frontend/scripts/fetch-icons.mjs
 regular/heart
+regular/star
 solid/arrow-down-wide-short
 solid/arrow-left
 solid/arrow-rotate-right

frontend/src/store/controllers/favorites-controller.ts

@@ -74,12 +74,36 @@ export class FavoritesController
     }
 
     /**
-     * Returns the icon name for the current icon style.
+     * The icon name for the current icon style, unfilled.
+     *
+     * Prefer `iconFor(isFav)` — this getter is the name of the *empty*
+     * glyph, which is what every caller that does not know the state
+     * should draw.
      */
     get iconName(): string {
-        return this.iconStyle === 'star'
+        return this.iconFor(false);
-            ? 'star'
+    }
-            : 'heart';
+
+    /**
+     * The glyph for one track's favourite state.
+     *
+     * **A filled shape means favourited and an outline means not**, in
+     * every list in the app. Nine components rendered `iconName`, which
+     * was the *solid* glyph in both states — so "not a favourite" was a
+     * filled heart in a duller colour, and the only thing separating
+     * the two states was hue. That fails for anyone who cannot see the
+     * difference between them, and reads as "everything is a favourite"
+     * to everyone else. `track-list` and `album-dropdown` already drew
+     * it correctly, from inline SVG paths of their own; this is the
+     * same rule for the `<wa-icon>` call sites.
+     *
+     * The Font Awesome family is part of the name — `regular/heart` is
+     * the outline, a bare `heart` is the solid one (`src/icons`).
+     */
+    iconFor(favorited: boolean): string {
+        const shape = this.iconStyle === 'star' ? 'star' : 'heart';
+
+        return favorited ? shape : `regular/${shape}`;
     }
 
     // ===============================================================

frontend/src/utils/grid-spacing.ts

@@ -0,0 +1,58 @@
+/**
+ * Even spacing for the three card grids — albums, artists, genres.
+ *
+ * All three used `justify: 'center'` with a fixed 8px gap and 8px
+ * padding, which gives the row a fixed width and pushes everything left
+ * over to the two margins: on a 1440px window the albums grid drew its
+ * cards 16px apart inside 78px of nothing down each side. The outside
+ * was five times the inside.
+ *
+ * The fix is to spend the leftover on the spacing instead, so there is
+ * one number: between two cards, between two rows, and down each edge.
+ * The virtualizer has a word for that — `justify: 'space-evenly'` with
+ * `gap: 'auto'` — and it cannot be used, because it fits
+ * `floor(width / cardWidth)` columns without reserving the gap it is
+ * about to need: a width one card short of exact fits seven cards a
+ * pixel apart. Deciding the column count here is what puts a floor
+ * under the spacing, and the grid is then given plain numbers.
+ */
+
+/** The narrowest the spacing is allowed to get. */
+export const MIN_GRID_SPACING = 8;
+
+/**
+ * How many cards of `cardWidth` fit across `width`.
+ *
+ * A row of c cards spends c×cardWidth on cards and (c+1)×spacing on the
+ * spaces between and beside them, so c is bounded by
+ * (width − spacing) / (cardWidth + spacing) at the minimum spacing.
+ */
+export function gridColumnsFor(
+    width: number,
+    cardWidth: number,
+): number {
+    if (cardWidth <= 0) return 1;
+
+    const fit = Math.floor(
+        (width - MIN_GRID_SPACING) / (cardWidth + MIN_GRID_SPACING),
+    );
+
+    return Math.max(1, fit);
+}
+
+/**
+ * The spacing `width` produces — the gap, the row gap and the padding,
+ * which are all the same number.
+ */
+export function gridSpacingFor(
+    width: number,
+    cardWidth: number,
+): number {
+    const columns = gridColumnsFor(width, cardWidth);
+    const leftover = width - columns * cardWidth;
+
+    return Math.max(
+        MIN_GRID_SPACING,
+        Math.floor(leftover / (columns + 1)),
+    );
+}

frontend/test/components/artist-release-menu.test.ts

@@ -120,7 +120,7 @@ describe('the context menu on an artist page release', () => {
     expect(items).toContain('Add to Queue');
     expect(items).toContain('Play Next');
     // Owned: there is nothing left to ask for.
-    expect(items).not.toContain('Want This');
+    expect(items).not.toContain('Request This');
   });
 
   it('offers a request, and no playback, for a release nobody owns', async () => {
@@ -132,7 +132,7 @@ describe('the context menu on an artist page release', () => {
 
     expect(items).not.toContain('Play');
     expect(items).not.toContain('Add to Queue');
-    expect(items).toContain('Want This');
+    expect(items).toContain('Request This');
     expect(items).toContain('View on MusicBrainz');
   });
 
@@ -148,7 +148,7 @@ describe('the context menu on an artist page release', () => {
     // …but a `local:` id names nothing upstream, and wanting something
     // already in the library is not a thing to offer.
     expect(items).not.toContain('View on MusicBrainz');
-    expect(items).not.toContain('Want This');
+    expect(items).not.toContain('Request This');
   });
 
   it('opens from the keyboard on Shift+F10', async () => {

frontend/test/components/chrome.test.ts

@@ -166,7 +166,7 @@ describe('<library-status-indicator>', () => {
       glyphs.push(shadow(el, 'wa-icon')?.getAttribute('name'));
     }
 
-    expect(glyphs).toEqual(['check', 'hourglass-half', 'plus']);
+    expect(glyphs).toEqual(['check', 'bookmark', 'plus']);
   });
 
   it('phrases its label around the entity it describes', async () => {

frontend/test/components/library-status.test.ts

@@ -249,7 +249,7 @@ describe('<library-status-indicator> as a control', () => {
     const el = await badge({ requestMbid: 'rg-1' });
 
     expect(shadow(el, '.badge')?.getAttribute('aria-label')).toBe(
-      'Want album "Abbey Road"',
+      'Request album "Abbey Road"',
     );
 
     await update(el, { status: 'queued' });