fix(download): make "check now" actually check now, and say what it did

The button ran a normal reconcile pass, which honours each request's
retry backoff — so a request searched an hour ago was not due, nothing
was searched, and the button looked broken. The backoff is a promise to
the providers, not to the user: a person pressing "check now" *is* the
schedule, so a user-initiated pass ignores it and the loop still does
not.

"Nothing happened" also needed a reason. Summary now carries how many
requests are still being looked for and whether any download client is
enabled at all, which is the one cause of silence the user can fix —
and the requests tab says so above the list rather than leaving an
inert list to be interpreted.

The rest is the retry schedule finally being admitted to: rows show
when the next check falls due, "Looking for" explains that a request
sitting there is waiting rather than failing, and the page header says
how often the list is worked.

backend/database/sql/queries/download.sql

@@ -176,6 +176,16 @@ WHERE state = 'wanted'
 ORDER BY attempts, created_at
 LIMIT ?;
 
+-- name: ListWantedDownloadRequests :many
+-- The same set ignoring the backoff, for a pass the user asked for by
+-- hand: "check now" that respected a six-hour retry schedule looked
+-- like a button that did nothing.
+SELECT * FROM download_requests
+WHERE state = 'wanted'
+  AND entity <> 'artist'
+ORDER BY attempts, created_at
+LIMIT ?;
+
 -- name: ListChildDownloadRequests :many
 SELECT * FROM download_requests WHERE parent_id = ? ORDER BY id;
 

backend/database/sql/sqlcgen/download.sql.go

@@ -739,6 +739,58 @@ func (q *Queries) ListLiveDownloads(ctx context.Context) ([]DownloadDownload, er
 	return items, nil
 }
 
+const listWantedDownloadRequests = `-- name: ListWantedDownloadRequests :many
+SELECT id, mbid, entity, library_id, artist, title, scope, secondary, state, parent_id, attempts, last_error, last_tried_at, next_try_at, external_ids, created_at, updated_at FROM download_requests
+WHERE state = 'wanted'
+  AND entity <> 'artist'
+ORDER BY attempts, created_at
+LIMIT ?
+`
+
+// The same set ignoring the backoff, for a pass the user asked for by
+// hand: "check now" that respected a six-hour retry schedule looked
+// like a button that did nothing.
+func (q *Queries) ListWantedDownloadRequests(ctx context.Context, limit int64) ([]DownloadRequest, error) {
+	rows, err := q.db.QueryContext(ctx, listWantedDownloadRequests, limit)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+	var items []DownloadRequest
+	for rows.Next() {
+		var i DownloadRequest
+		if err := rows.Scan(
+			&i.ID,
+			&i.Mbid,
+			&i.Entity,
+			&i.LibraryID,
+			&i.Artist,
+			&i.Title,
+			&i.Scope,
+			&i.Secondary,
+			&i.State,
+			&i.ParentID,
+			&i.Attempts,
+			&i.LastError,
+			&i.LastTriedAt,
+			&i.NextTryAt,
+			&i.ExternalIds,
+			&i.CreatedAt,
+			&i.UpdatedAt,
+		); err != nil {
+			return nil, err
+		}
+		items = append(items, i)
+	}
+	if err := rows.Close(); err != nil {
+		return nil, err
+	}
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+	return items, nil
+}
+
 const recordDownloadRequestAttempt = `-- name: RecordDownloadRequestAttempt :exec
 UPDATE download_requests
 SET attempts      = attempts + 1,

backend/download/reconcile.go

@@ -251,6 +251,16 @@ type Summary struct {
 
 	// Synced is how many requests were pushed to an external list.
 	Synced int `json:"synced"`
+
+	// Waiting is how many requests are on the list and still being
+	// looked for.  A pass that did nothing is the normal case, and the
+	// UI can only say so honestly if it knows the list was not empty.
+	Waiting int `json:"waiting"`
+
+	// NoProviders reports that nothing could be searched because no
+	// download client is enabled — the one "nothing happened" the user
+	// can actually fix.
+	NoProviders bool `json:"noProviders"`
 }
 
 // changed reports whether the pass altered anything worth refreshing
@@ -259,9 +269,22 @@ func (s Summary) changed() bool {
 	return s.Expanded > 0 || s.Satisfied > 0 || s.Started > 0
 }
 
-// RunOnce works the request list once.  It is safe to call directly, and
-// the "search now" button does.
+// RunOnce works the request list once, honouring each request's
+// backoff.  This is what the loop calls.
 func (r *Reconciler) RunOnce(ctx context.Context) (Summary, error) {
+	return r.run(ctx, false)
+}
+
+// RunNow works the request list ignoring backoff.  This is what the
+// "check now" button calls: a scheduled retry is a promise to the
+// provider, not to the user, and a person who presses a button expects
+// their list to actually be searched rather than to be told it is not
+// due yet.
+func (r *Reconciler) RunNow(ctx context.Context) (Summary, error) {
+	return r.run(ctx, true)
+}
+
+func (r *Reconciler) run(ctx context.Context, force bool) (Summary, error) {
 	r.runMu.Lock()
 	defer r.runMu.Unlock()
 
@@ -287,13 +310,15 @@ func (r *Reconciler) RunOnce(ctx context.Context) (Summary, error) {
 
 	summary.Synced = r.syncExternalLists(ctx)
 
-	attempted, started, err := r.attemptDue(ctx)
+	attempted, started, err := r.attemptDue(ctx, force)
 	if err != nil {
 		return summary, err
 	}
 
 	summary.Attempted = attempted
 	summary.Started = started
+	summary.Waiting = r.countWaiting(ctx)
+	summary.NoProviders = len(r.manager.enabledProviders()) == 0
 
 	r.logger.Info(
 		"reconciled request list",
@@ -506,10 +531,19 @@ func (r *Reconciler) retireOwned(ctx context.Context) (int, error) {
 // Attempting downloads
 // ---------------------------------------------------------------------------
 
-// attemptDue searches for a bounded batch of due requests and grabs the
-// ones with a clear winner.
-func (r *Reconciler) attemptDue(ctx context.Context) (attempted, started int, err error) {
-	due, err := r.store.ListDueRequests(ctx, r.batch)
+// attemptDue searches for a bounded batch of requests and grabs the
+// ones with a clear winner.  force takes requests whose backoff has not
+// elapsed as well.
+func (r *Reconciler) attemptDue(
+	ctx context.Context,
+	force bool,
+) (attempted, started int, err error) {
+	list := r.store.ListDueRequests
+	if force {
+		list = r.store.ListWantedRequests
+	}
+
+	due, err := list(ctx, r.batch)
 	if err != nil {
 		return 0, 0, err
 	}
@@ -615,6 +649,26 @@ func (r *Reconciler) attempt(ctx context.Context, req Request) (bool, string) {
 	return started, reason
 }
 
+// countWaiting reports how many non-artist requests are still being
+// looked for, so "nothing happened" can be reported as "nothing new
+// for the twelve things on your list" rather than as silence.
+func (r *Reconciler) countWaiting(ctx context.Context) int {
+	requests, err := r.store.ListRequests(ctx)
+	if err != nil {
+		return 0
+	}
+
+	waiting := 0
+
+	for _, req := range requests {
+		if req.State == RequestStateWanted && !req.Entity.Expands() {
+			waiting++
+		}
+	}
+
+	return waiting
+}
+
 // ---------------------------------------------------------------------------
 // External list sync
 // ---------------------------------------------------------------------------

backend/download/reconcile_test.go

@@ -495,3 +495,101 @@ func waitFor(t *testing.T, cond func() bool, msg string) {
 
 	t.Fatal(msg)
 }
+
+// "Check now" is the user overriding the retry schedule, so it must
+// search a request whose backoff has not elapsed.  The scheduled pass
+// must not: the backoff exists to keep a fruitless search off the
+// providers, and a loop that ignored it would hammer them.
+func TestRunNowIgnoresBackoffAndRunOnceDoesNot(t *testing.T) {
+	t.Parallel()
+
+	f := newReconcileFixture(t)
+	ctx := context.Background()
+
+	provider := NewFakeProvider(1, "weak", Caps{CanSearch: true, CanTransport: true})
+	provider.Candidates = []Candidate{candidateFor(
+		"weak-1", []string{"Something Else Entirely"}, ".mp3", 3_000_000,
+	)}
+
+	f.manager.installProvider(Config{ID: 1, Priority: 50}, provider)
+
+	id, err := f.store.AddRequest(ctx, Request{
+		MBID:      "rg-1",
+		Entity:    EntityReleaseGroup,
+		LibraryID: 1,
+		Artist:    "Radiohead",
+		Title:     "OK Computer",
+	})
+	if err != nil {
+		t.Fatalf("AddRequest: %v", err)
+	}
+
+	f.catalog.tracklists["rg-1"] = fourTrackDownload().Expected
+
+	// First pass: attempted, found nothing, backoff armed.
+	if _, err := f.reconciler.RunOnce(ctx); err != nil {
+		t.Fatalf("RunOnce: %v", err)
+	}
+
+	scheduled, err := f.reconciler.RunOnce(ctx)
+	if err != nil {
+		t.Fatalf("RunOnce (second): %v", err)
+	}
+
+	if scheduled.Attempted != 0 {
+		t.Errorf("scheduled pass attempted %d, want 0 while backed off",
+			scheduled.Attempted)
+	}
+
+	forced, err := f.reconciler.RunNow(ctx)
+	if err != nil {
+		t.Fatalf("RunNow: %v", err)
+	}
+
+	if forced.Attempted != 1 {
+		t.Errorf("forced pass attempted %d, want 1", forced.Attempted)
+	}
+
+	if forced.Waiting != 1 {
+		t.Errorf("summary reported %d waiting, want 1 so the UI can say "+
+			"what was searched", forced.Waiting)
+	}
+
+	req, err := f.store.GetRequest(ctx, id)
+	if err != nil {
+		t.Fatalf("GetRequest: %v", err)
+	}
+
+	if req.Attempts != 2 {
+		t.Errorf("attempts = %d, want 2 after a forced re-check", req.Attempts)
+	}
+}
+
+// A pass with no providers says so, because "nothing happened" with no
+// reason is the one outcome the user cannot act on.
+func TestSummaryReportsNoProviders(t *testing.T) {
+	t.Parallel()
+
+	f := newReconcileFixture(t)
+	ctx := context.Background()
+
+	if _, err := f.store.AddRequest(ctx, Request{
+		MBID:      "rg-1",
+		Entity:    EntityReleaseGroup,
+		LibraryID: 1,
+		Title:     "OK Computer",
+	}); err != nil {
+		t.Fatalf("AddRequest: %v", err)
+	}
+
+	f.catalog.tracklists["rg-1"] = fourTrackDownload().Expected
+
+	summary, err := f.reconciler.RunNow(ctx)
+	if err != nil {
+		t.Fatalf("RunNow: %v", err)
+	}
+
+	if !summary.NoProviders {
+		t.Error("summary did not report that no download client is enabled")
+	}
+}

backend/download/requeststore.go

@@ -152,6 +152,23 @@ func (s *Store) ListDueRequests(ctx context.Context, limit int) ([]Request, erro
 	return requestRowsToRequests(rows), nil
 }
 
+// ListWantedRequests returns downloadable requests regardless of their
+// backoff, least-attempted first.  Only a user-initiated pass uses
+// this: the loop honours the schedule, a person pressing "check now"
+// is the schedule.
+func (s *Store) ListWantedRequests(ctx context.Context, limit int) ([]Request, error) {
+	if limit <= 0 {
+		limit = defaultDueBatch
+	}
+
+	rows, err := s.db.ReadQueries.ListWantedDownloadRequests(ctx, int64(limit))
+	if err != nil {
+		return nil, fmt.Errorf("list wanted download requests: %w", err)
+	}
+
+	return requestRowsToRequests(rows), nil
+}
+
 // ListChildRequests returns the requests an artist subscription
 // produced.
 func (s *Store) ListChildRequests(

backend/download/service.go

@@ -609,7 +609,7 @@ func (s *Service) ReconcileRequests() (Summary, error) {
 		)
 	}
 
-	summary, err := s.reconciler.RunOnce(context.Background())
+	summary, err := s.reconciler.RunNow(context.Background())
 	if err != nil {
 		return summary, err
 	}