diff --git a/backend/explore/explore.go b/backend/explore/explore.go index 483e076..32177a3 100644 --- a/backend/explore/explore.go +++ b/backend/explore/explore.go @@ -1236,76 +1236,38 @@ func mergeIndexHits(query string, result *MBSearchResult, hits []SearchIndexResu switch h.EntityType { case "artist": if !artistMBIDs[h.MBID] { - inLib := h.InLibrary || h.LocalArtistID > 0 + artist := artistFromIndex(h) + artist.Score = indexHitBlendedScore( + query, h.Title, "", h.Popularity, maxArtistPop, + artist.InLibrary, h.IsSimilar, + ) - newArtists = append(newArtists, MBArtist{ - MBID: h.MBID, - Name: h.Title, - Type: h.ArtistType, - Country: h.Country, - Disambiguation: h.Disambiguation, - SortName: h.SortName, - Score: indexHitBlendedScore( - query, h.Title, "", h.Popularity, maxArtistPop, inLib, h.IsSimilar, - ), - HasPopularity: h.Popularity > 0, - Popularity: h.Popularity, - ListenerCount: h.ListenerCount, - InLibrary: inLib, - LocalID: h.LocalArtistID, - }) + newArtists = append(newArtists, artist) artistMBIDs[h.MBID] = true } case "release_group": if !rgMBIDs[h.MBID] { - inLib := h.InLibrary || h.LocalReleaseGroupID > 0 + rg := releaseGroupFromIndex(h) + rg.Score = indexHitBlendedScore( + query, h.Title, h.ArtistName, h.Popularity, maxRGPop, + rg.InLibrary, h.IsSimilar, + ) - var secondary []string - if h.SecondaryTypes != "" { - secondary = strings.Split(h.SecondaryTypes, ",") - } - - newRGs = append(newRGs, MBReleaseGroup{ - MBID: h.MBID, - Title: h.Title, - ArtistCredit: h.ArtistName, - ArtistMBID: h.ArtistMBID, - Score: indexHitBlendedScore( - query, h.Title, h.ArtistName, h.Popularity, maxRGPop, inLib, h.IsSimilar, - ), - Popularity: h.Popularity, - ListenerCount: h.ListenerCount, - PrimaryType: h.PrimaryType, - SecondaryTypes: secondary, - FirstReleaseDate: h.ReleaseDate, - InLibrary: inLib, - LocalID: h.LocalReleaseGroupID, - }) + newRGs = append(newRGs, rg) rgMBIDs[h.MBID] = true } case "recording": if !recMBIDs[h.MBID] { - inLib := h.InLibrary || h.LocalRecordingID > 0 + rec := recordingFromIndex(h) + rec.Score = indexHitBlendedScore( + query, h.Title, h.ArtistName, h.Popularity, maxRecPop, + rec.InLibrary, h.IsSimilar, + ) - newRecs = append(newRecs, MBRecording{ - MBID: h.MBID, - Title: h.Title, - Length: h.Duration, - ArtistCredit: h.ArtistName, - ArtistMBID: h.ArtistMBID, - Score: indexHitBlendedScore( - query, h.Title, h.ArtistName, h.Popularity, maxRecPop, inLib, h.IsSimilar, - ), - Popularity: h.Popularity, - ListenerCount: h.ListenerCount, - CAAReleaseMBID: h.CAAReleaseMBID, - ReleaseName: h.ReleaseName, - InLibrary: inLib, - LocalID: h.LocalRecordingID, - }) + newRecs = append(newRecs, rec) recMBIDs[h.MBID] = true } @@ -1335,6 +1297,67 @@ func mergeIndexHits(query string, result *MBSearchResult, hits []SearchIndexResu } } +// The three functions below are the one definition of what an index row +// looks like as a card. They were inline in mergeIndexHits, which is +// the only place that needed them until the shelves did; a shelf builds +// the same cards from the same rows and must not grow a second, subtly +// different projection of them. Score is deliberately not set here — +// it is a property of a *search*, and a shelf has no query to be +// relevant to. + +func artistFromIndex(h SearchIndexResult) MBArtist { + return MBArtist{ + MBID: h.MBID, + Name: h.Title, + Type: h.ArtistType, + Country: h.Country, + Disambiguation: h.Disambiguation, + SortName: h.SortName, + HasPopularity: h.Popularity > 0, + Popularity: h.Popularity, + ListenerCount: h.ListenerCount, + InLibrary: h.InLibrary || h.LocalArtistID > 0, + LocalID: h.LocalArtistID, + } +} + +func releaseGroupFromIndex(h SearchIndexResult) MBReleaseGroup { + var secondary []string + if h.SecondaryTypes != "" { + secondary = strings.Split(h.SecondaryTypes, ",") + } + + return MBReleaseGroup{ + MBID: h.MBID, + Title: h.Title, + ArtistCredit: h.ArtistName, + ArtistMBID: h.ArtistMBID, + Popularity: h.Popularity, + ListenerCount: h.ListenerCount, + PrimaryType: h.PrimaryType, + SecondaryTypes: secondary, + FirstReleaseDate: h.ReleaseDate, + InLibrary: h.InLibrary || h.LocalReleaseGroupID > 0, + LocalID: h.LocalReleaseGroupID, + } +} + +func recordingFromIndex(h SearchIndexResult) MBRecording { + return MBRecording{ + MBID: h.MBID, + Title: h.Title, + Length: h.Duration, + ArtistCredit: h.ArtistName, + ArtistMBID: h.ArtistMBID, + Popularity: h.Popularity, + ListenerCount: h.ListenerCount, + CAAReleaseMBID: h.CAAReleaseMBID, + ReleaseName: h.ReleaseName, + InLibrary: h.InLibrary || h.LocalRecordingID > 0, + LocalID: h.LocalRecordingID, + } +} + // indexHitBlendedScore scores a local index hit on the same 0–100 // blended scale the MB rerank uses, so merged results sort and filter // consistently regardless of source. diff --git a/backend/explore/shelves.go b/backend/explore/shelves.go new file mode 100644 index 0000000..7b24fad --- /dev/null +++ b/backend/explore/shelves.go @@ -0,0 +1,480 @@ +package explore + +import ( + "context" + "strings" +) + +// Explore's shelves — the page's answer before anyone has typed. +// +// Every other view in this app answers "what have I got". Explore is +// the only one that answers "what exists", and until now it would not +// start: a search box over a 1.1 M-row local catalog, and a sentence +// telling the user to type into a catalog whose whole point is that +// they do not yet know what is in it (H-23). +// +// The convention is `backend/home`'s, deliberately: a shelf is a +// **reason**, not a filter, and it carries the sentence that says so; a +// shelf with nothing behind it is omitted rather than rendered empty. +// What differs is what happens when *every* shelf is empty. Home's +// answer is a shorter page, which is honest because a library with no +// history really has less to say. Explore's data is a downloaded +// artifact that can be absent, half-merged, or never fetched — so an +// empty page there is not a small library, it is a page that does not +// know yet, and it has to say which. That is what State is for. +// +// The plan named four candidate shelves and said to cut them down once +// they could be seen next to each other. Two of them could not be built +// at all, and the schema says so rather than the design: +// +// - "Big in a genre you already have depth in" needs a genre on a +// catalog row. `explore_index` has no genre or tag column, and +// genre lives only in the library's own `recording_genres`. There +// is nothing to join to. Dropped, not deferred. +// - "Artists next to ones you own" needs `similar_artist_map`, which +// `cmd/indexexport` does not ship (the artifact carries +// `explore_index` and its metadata, nothing else) and which is +// filled lazily by ListenBrainz calls from artist pages. It is +// empty on a fresh install and empty offline, which is precisely +// when this page most needs something to show. +// +// What is left is three, and only the first is guaranteed: the other +// two join back to the library through `in_library`, which is set by +// MBID and is therefore empty on an untagged library — including the +// fixture one, where these shelves correctly render as one. + +// ShelfKind identifies what a shelf is built from, so the frontend can +// pick an icon and a spec can assert on a shelf without matching +// display copy. +type ShelfKind string + +// Shelf kinds. +const ( + ShelfPopularAlbums ShelfKind = "popular-albums" + ShelfPopularArtists ShelfKind = "popular-artists" + ShelfMoreFromOwned ShelfKind = "more-from-owned" +) + +// Shelf is one horizontal row on the Explore page. +// +// A shelf carries albums or artists, never both: they route to +// different pages and render as different cards, and a row that is +// sometimes one and sometimes the other is two components pretending to +// be one. +type Shelf struct { + ID string `json:"id"` + Kind ShelfKind `json:"kind"` + + // Title is the row heading. + Title string `json:"title"` + + // Subtitle says why these are here. As on Home it is not + // decoration: without it a shelf is indistinguishable from a + // random grid. + Subtitle string `json:"subtitle"` + + Albums []MBReleaseGroup `json:"albums,omitempty"` + Artists []MBArtist `json:"artists,omitempty"` +} + +// ShelfPage is what Explore renders before a query. +// +// State exists because "no shelves" has three different causes here and +// the page must not present them identically — a blank panel is the bug +// this whole feature is fixing, and a blank panel that says nothing +// about why is the same bug with more code behind it. +type ShelfPage struct { + Shelves []Shelf `json:"shelves"` + + // State is one of: + // + // "ready" — the catalog is here and the shelves are below. + // "building" — it is being fetched or built right now. + // "no-index" — there is no catalog. Search still works over + // whatever is in the index, which may be nothing; + // the page says so and points at Settings. + State string `json:"state"` +} + +// Shelf page states. +const ( + ShelfStateReady = "ready" + ShelfStateBuilding = "building" + ShelfStateNoIndex = "no-index" +) + +// shelfSize is how many cards one row holds. Matches `home`'s, for the +// same reason: wide enough to be worth scrolling, small enough that the +// row reads as a selection rather than a dump of the catalog. +const shelfSize = 12 + +// ownedArtistPool bounds how many owned artists the "you own one album +// by this artist" shelf considers. A library with 4 000 tagged artists +// does not need all of them ranked to fill twelve cards, and the bound +// is what keeps this query off the startup path's critical section. +const ownedArtistPool = 200 + +// GetExploreShelves builds the page Explore shows before a query. +// +// One call rather than one per shelf, for `home`'s reason: the shelves +// share nothing expensive, but the page has nothing useful to render +// until it knows which rows exist, and rows that pop in one at a time +// reflow under the cursor. +func (e *Service) GetExploreShelves() ShelfPage { + ctx := e.ctx + if ctx == nil { + ctx = context.Background() + } + + // "Is there a catalog" is asked of the database, not of a flag. + // + // Two cached answers were tried first and both were wrong in the + // same way. `GetIndexStatus().TotalRows` is an in-memory field + // refreshed between build tiers, so on an ordinary launch — artifact + // already merged, nothing building — it reads 0 beside a full + // catalog, and gating on it hid every shelf. `IsReady()` is set once + // at startup by counting rows, so it is right in the app and wrong + // for anything that changes the table afterwards — including the + // e2e suite staging a catalog, which is how this page gets tested + // in CI, where the artifact URL points at a dead address on purpose. + // + // Both are the shape `emitStatus` warns about: a derived value with + // nothing left polling behind it. One `SELECT 1 … LIMIT 1` against + // an indexed table costs nothing and cannot be stale. + status := e.index.GetIndexStatus() + building := status.Building + + if !e.index.hasCatalogRows(ctx) { + state := ShelfStateNoIndex + if building { + state = ShelfStateBuilding + } + + return ShelfPage{Shelves: []Shelf{}, State: state} + } + + shelves := make([]Shelf, 0, 3) //nolint:mnd // one per shelf kind + + // Which artists the page has already spent a row on. + // + // `home` suppresses a shelf that repeats the one above it by album + // id. That test is useless here and reads as unnecessary: these + // shelves hold different entity types, so their ids are disjoint by + // construction and no overlap is possible. The page repeated itself + // anyway. Ordered by raw listen count, a ListenBrainz-derived + // catalog's top albums are seven records by one act and its members, + // and the artists shelf underneath is then the same seven people — + // visibly one fandom twice, with no two rows sharing an id. The + // duplication is by *artist*, which is what a person sees. + seen := make(map[string]struct{}) + + add := func(shelf Shelf, ok bool) { + if !ok || (len(shelf.Albums) == 0 && len(shelf.Artists) == 0) { + return + } + + for _, album := range shelf.Albums { + if album.ArtistMBID != "" { + seen[album.ArtistMBID] = struct{}{} + } + } + + for _, artist := range shelf.Artists { + seen[artist.MBID] = struct{}{} + } + + shelves = append(shelves, shelf) + } + + add(e.moreFromOwnedArtists(ctx)) + add(e.popularAlbums(ctx, seen)) + add(e.popularArtists(ctx, seen)) + + // A build in progress over a catalog that already has rows is still + // "building" — the shelves below are real but incomplete, and a page + // that will visibly gain rows should say so rather than let the user + // wonder why it changed. + state := ShelfStateReady + if building { + state = ShelfStateBuilding + } + + return ShelfPage{Shelves: shelves, State: state} +} + +// popularAlbums is the honest default for "what exists", and the only +// shelf that needs nothing from the user: no library, no tags, no +// network. If it is empty, there is no catalog, which the State +// already said. +func (e *Service) popularAlbums( + ctx context.Context, + seen map[string]struct{}, +) (Shelf, bool) { + rows := e.index.topByPopularity(ctx, "release_group", shelfSize, seen) + if len(rows) == 0 { + return Shelf{}, false + } + + albums := make([]MBReleaseGroup, 0, len(rows)) + for _, row := range rows { + albums = append(albums, releaseGroupFromIndex(row)) + } + + return Shelf{ + ID: "popular-albums", + Kind: ShelfPopularAlbums, + Title: "Popular right now", + Subtitle: "The most listened-to albums you don't already own", + Albums: albums, + }, true +} + +// popularArtists is a different question, not the same one re-sorted: +// an artist card opens the artist page, and a page made only of albums +// offers no route to the half of Explore that is about people. +// +// It is *asked* differently too: it skips whoever the rows above +// already showed, because the shelves are all ordered by the same +// listen count and the top of that list is not a broad selection. +func (e *Service) popularArtists( + ctx context.Context, + seen map[string]struct{}, +) (Shelf, bool) { + rows := e.index.topByPopularity(ctx, "artist", shelfSize, seen) + if len(rows) == 0 { + return Shelf{}, false + } + + artists := make([]MBArtist, 0, len(rows)) + for _, row := range rows { + artists = append(artists, artistFromIndex(row)) + } + + return Shelf{ + ID: "popular-artists", + Kind: ShelfPopularArtists, + Title: "Artists worth knowing", + Subtitle: "Widely listened to, and not yet in your library", + Albums: nil, + Artists: artists, + }, true +} + +// moreFromOwnedArtists is the catalog answering a gap the library can +// already see: one album by an artist is usually an accident of how it +// arrived, not a considered stopping point. +// +// It is first on the page when it exists, because it is the only shelf +// about *this* user, and last to exist at all: it reads `in_library`, +// which `PopulateLocalCrossReferences` sets by MusicBrainz ID, so an +// untagged library produces nothing here however large it is. +func (e *Service) moreFromOwnedArtists(ctx context.Context) (Shelf, bool) { + rows := e.index.unownedAlbumsBySinglyOwnedArtists(ctx, ownedArtistPool, shelfSize) + if len(rows) == 0 { + return Shelf{}, false + } + + albums := make([]MBReleaseGroup, 0, len(rows)) + for _, row := range rows { + albums = append(albums, releaseGroupFromIndex(row)) + } + + title := "You own one album by these artists" + if names := distinctArtists(rows); len(names) == 1 { + title = "More from " + names[0] + } + + return Shelf{ + ID: "more-from-owned", + Kind: ShelfMoreFromOwned, + Title: title, + Subtitle: "The rest of what they made", + Albums: albums, + }, true +} + +// The two queries the shelves are built from. +// +// They return ids and nothing else, and are joined back to the card +// projection by `rowsByIDs` — `backend/home`'s arrangement, for its +// reason: there is one definition of an Explore card, and a shelf query +// that also selected columns would quietly become a second one. That +// `rowsByIDs` returns rows in the order it was given them is what lets +// the ordering live in SQL. + +// topByPopularity is the catalog's own answer to "what exists", for one +// entity type. +// +// `in_library = 0` because Explore is the view that answers what the +// user does *not* have — every other view in the app already answers +// the other question, and a discovery row that opens with something +// they own has spent a slot saying nothing. `popularity > 0` drops the +// long tail the ListenBrainz dump had no listens for, which would +// otherwise be ordered arbitrarily among themselves. +// +// **One row per artist**, which is the difference between a shelf and a +// leaderboard. Ordered by raw listen count, the catalog's top twelve +// albums were seven records by one act and its members; a shelf is a +// selection, and twelve slots spent on one artist is the row saying one +// thing twelve times. `skip` then drops artists another shelf already +// showed, for the same reason one row further out. +// +// It over-fetches and filters in Go rather than passing the skip set to +// SQL: the set is a handful of MBIDs against a window function over an +// indexed scan, and an `artist_mbid NOT IN (?, ?, …)` would rebuild the +// statement per call for no measurable gain. +func (si *SearchIndex) topByPopularity( + ctx context.Context, + entityType string, + limit int, + skip map[string]struct{}, +) []SearchIndexResult { + // The artist rows *are* the artists, so they partition by their own + // mbid; release groups partition by whoever made them. + partition := "artist_mbid" + if entityType == "artist" { + partition = "mbid" + } + + rows := si.rowsByIDs(ctx, si.shelfIDs( + ctx, + `SELECT id FROM ( + SELECT id, popularity, + ROW_NUMBER() OVER ( + PARTITION BY `+partition+` + ORDER BY popularity DESC + ) AS rank + FROM explore_index + WHERE entity_type = ? AND in_library = 0 AND popularity > 0 + ) + WHERE rank = 1 + ORDER BY popularity DESC + LIMIT ?`, + entityType, limit+len(skip), + )) + + out := make([]SearchIndexResult, 0, limit) + + for _, row := range rows { + key := row.ArtistMBID + if entityType == "artist" { + key = row.MBID + } + + if _, ok := skip[key]; ok && key != "" { + continue + } + + out = append(out, row) + + if len(out) == limit { + break + } + } + + return out +} + +// unownedAlbumsBySinglyOwnedArtists finds albums by artists the library +// has exactly one album from. +// +// Both halves are `explore_index` rows: ownership is a column on the +// catalog, set by `PopulateLocalCrossReferences` from the library's +// MusicBrainz IDs, so this never touches the library tables and asks +// one query rather than one per artist. +// +// The artists are drawn most-popular-owned-album first, so a large +// library's pool is the part of it the user is likeliest to recognise +// rather than whichever artists sort first. +func (si *SearchIndex) unownedAlbumsBySinglyOwnedArtists( + ctx context.Context, + pool, limit int, +) []SearchIndexResult { + return si.rowsByIDs(ctx, si.shelfIDs( + ctx, + `SELECT id FROM explore_index + WHERE entity_type = 'release_group' + AND in_library = 0 + AND artist_mbid IN ( + SELECT artist_mbid FROM explore_index + WHERE entity_type = 'release_group' + AND in_library = 1 + AND artist_mbid != '' + GROUP BY artist_mbid + HAVING COUNT(*) = 1 + ORDER BY MAX(popularity) DESC + LIMIT ?) + ORDER BY popularity DESC + LIMIT ?`, + pool, limit, + )) +} + +// hasCatalogRows reports whether there is a catalog at all. +// +// Deliberately "any row", not a count: the question is whether the page +// has a catalog to draw on, and a count of 1.1 M rows costs a scan to +// answer a yes/no. +func (si *SearchIndex) hasCatalogRows(ctx context.Context) bool { + rows, err := si.db.QueryContextWith(ctx, "SELECT 1 FROM explore_index LIMIT 1") + if err != nil { + si.logger.Warn("explore shelves: catalog probe failed", "error", err) + + return false + } + + defer func() { _ = rows.Close() }() + + return rows.Next() +} + +// shelfIDs runs a shelf query that selects one id column. +func (si *SearchIndex) shelfIDs( + ctx context.Context, + query string, + args ...any, +) []int64 { + rows, err := si.db.QueryContextWith(ctx, query, args...) + if err != nil { + si.logger.Warn("explore shelves: query failed", "error", err) + + return nil + } + + defer func() { _ = rows.Close() }() + + var ids []int64 + + for rows.Next() { + var id int64 + if err := rows.Scan(&id); err == nil { + ids = append(ids, id) + } + } + + return ids +} + +// distinctArtists lists the artist credits in a shelf, in order, once +// each — so a shelf that turns out to be about one artist can say so +// by name instead of using the plural heading. +func distinctArtists(rows []SearchIndexResult) []string { + seen := make(map[string]struct{}, len(rows)) + names := make([]string, 0, len(rows)) + + for _, row := range rows { + name := strings.TrimSpace(row.ArtistName) + if name == "" { + continue + } + + if _, ok := seen[name]; ok { + continue + } + + seen[name] = struct{}{} + names = append(names, name) + } + + return names +} diff --git a/backend/explore/shelves_test.go b/backend/explore/shelves_test.go new file mode 100644 index 0000000..cf42a9c --- /dev/null +++ b/backend/explore/shelves_test.go @@ -0,0 +1,312 @@ +package explore + +import ( + "context" + "log/slog" + "testing" + + "yellowjacket/backend/database" +) + +// The shelves are three queries and one rule about what to do when they +// all come back empty, and the second half is the part that matters: +// Explore's data is a downloaded artifact, so "no shelves" can mean the +// catalog is absent, mid-build, or simply has nothing the user does not +// already own — and rendering those identically is the blank panel this +// feature exists to remove. + +// seedShelfRow writes one explore_index row. The parameter list is the +// real upsert's, so a schema change breaks these tests in the same +// place it breaks the app rather than leaving them passing against a +// shape nothing writes any more. +func seedShelfRow( + t *testing.T, + db *database.DB, + entityType, mbid, title, artistName, artistMBID string, + popularity int, + inLibrary bool, +) { + t.Helper() + + owned := 0 + if inLibrary { + owned = 1 + } + + if _, err := db.ExecContext( + upsertIndexSQL, + entityType, mbid, title, artistName, artistMBID, "", + popularity, popularity, + 0, "", "", + "Album", "", "", + "", "", "", "", + owned, 0, + 0, 0, 0, + 0, + ); err != nil { + t.Fatalf("seed explore_index row %q: %v", mbid, err) + } +} + +func newShelfService(t *testing.T) (*Service, *database.DB) { + t.Helper() + + db := database.NewTestDB(t) + index := NewSearchIndex(db, nil, nil, slog.Default()) + + return &Service{ + index: index, + db: db, + logger: slog.Default(), + ctx: context.Background(), + }, db +} + +// buildShelves builds the page. +// +// Nothing has to be marked ready first, which is the point: the gate is +// a question asked of the database, so rows seeded after startup count. +// Both cached alternatives failed here first — one because it is only +// refreshed between build tiers, the other because it is set once at +// startup, when a test has seeded nothing yet. +func buildShelves(svc *Service) ShelfPage { + return svc.GetExploreShelves() +} + +func shelfByKind(page ShelfPage, kind ShelfKind) (Shelf, bool) { + for _, shelf := range page.Shelves { + if shelf.Kind == kind { + return shelf, true + } + } + + return Shelf{}, false +} + +func TestShelves_NoIndexSaysSoRatherThanShowingNothing(t *testing.T) { + t.Parallel() + + svc, _ := newShelfService(t) + + page := buildShelves(svc) + + if page.State != ShelfStateNoIndex { + t.Fatalf("state = %q, want %q", page.State, ShelfStateNoIndex) + } + + if len(page.Shelves) != 0 { + t.Fatalf("shelves = %d, want 0", len(page.Shelves)) + } +} + +func TestShelves_PopularOrdersByPopularityAndExcludesOwned(t *testing.T) { + t.Parallel() + + svc, db := newShelfService(t) + + seedShelfRow(t, db, "release_group", "rg-quiet", "Quiet", "A", "mbid-a", 10, false) + seedShelfRow(t, db, "release_group", "rg-loud", "Loud", "B", "mbid-b", 900, false) + seedShelfRow(t, db, "release_group", "rg-mine", "Mine", "C", "mbid-c", 5000, true) + // Popularity 0 is the dump's "no listens", not a low score: those + // rows have no defined order among themselves. + seedShelfRow(t, db, "release_group", "rg-unheard", "Unheard", "D", "mbid-d", 0, false) + + page := buildShelves(svc) + + shelf, ok := shelfByKind(page, ShelfPopularAlbums) + if !ok { + t.Fatal("no popular-albums shelf") + } + + var titles []string + for _, album := range shelf.Albums { + titles = append(titles, album.Title) + } + + if len(titles) != 2 || titles[0] != "Loud" || titles[1] != "Quiet" { + t.Fatalf("albums = %v, want [Loud Quiet]", titles) + } +} + +func TestShelves_ArtistsAreTheirOwnShelf(t *testing.T) { + t.Parallel() + + svc, db := newShelfService(t) + + // Two different artists: the albums shelf takes one, and the + // artists shelf must not simply repeat them (see the dedup test + // below), so the shelf it gets is somebody else's. + seedShelfRow(t, db, "release_group", "rg-1", "An Album", "Album Maker", "ar-album", 900, false) + seedShelfRow(t, db, "artist", "ar-1", "Big Name", "Big Name", "ar-1", 800, false) + + page := buildShelves(svc) + + artists, ok := shelfByKind(page, ShelfPopularArtists) + if !ok { + t.Fatal("no popular-artists shelf") + } + + // A shelf carries albums or artists, never both: the two render as + // different cards and route to different pages. + if len(artists.Artists) != 1 || len(artists.Albums) != 0 { + t.Fatalf("artists shelf = %d artists / %d albums, want 1 / 0", + len(artists.Artists), len(artists.Albums)) + } + + if artists.Artists[0].Name != "Big Name" { + t.Fatalf("artist name = %q", artists.Artists[0].Name) + } +} + +func TestShelves_OmitsAShelfWithNothingBehindIt(t *testing.T) { + t.Parallel() + + svc, db := newShelfService(t) + + // Albums only: the artists shelf has nothing and must not be + // rendered empty, and nothing is owned so neither has the third. + seedShelfRow(t, db, "release_group", "rg-1", "An Album", "A", "mbid-a", 100, false) + + page := buildShelves(svc) + + if len(page.Shelves) != 1 { + var kinds []ShelfKind + for _, shelf := range page.Shelves { + kinds = append(kinds, shelf.Kind) + } + + t.Fatalf("shelves = %v, want just popular-albums", kinds) + } + + if page.State != ShelfStateReady { + t.Fatalf("state = %q, want %q", page.State, ShelfStateReady) + } +} + +func TestShelves_MoreFromOwnedNeedsExactlyOneOwnedAlbum(t *testing.T) { + t.Parallel() + + svc, db := newShelfService(t) + + // One album owned by "Solo", so the rest of Solo's catalog is the + // shelf. Two owned by "Complete", who is therefore not a gap. + seedShelfRow(t, db, "release_group", "rg-solo-own", "Owned", "Solo", "mbid-solo", 100, true) + seedShelfRow(t, db, "release_group", "rg-solo-a", "Second", "Solo", "mbid-solo", 90, false) + seedShelfRow(t, db, "release_group", "rg-solo-b", "Third", "Solo", "mbid-solo", 80, false) + seedShelfRow(t, db, "release_group", "rg-comp-1", "One", "Complete", "mbid-comp", 100, true) + seedShelfRow(t, db, "release_group", "rg-comp-2", "Two", "Complete", "mbid-comp", 90, true) + seedShelfRow(t, db, "release_group", "rg-comp-3", "Three", "Complete", "mbid-comp", 70, false) + + page := buildShelves(svc) + + shelf, ok := shelfByKind(page, ShelfMoreFromOwned) + if !ok { + t.Fatal("no more-from-owned shelf") + } + + var titles []string + for _, album := range shelf.Albums { + titles = append(titles, album.Title) + } + + if len(titles) != 2 || titles[0] != "Second" || titles[1] != "Third" { + t.Fatalf("albums = %v, want [Second Third]", titles) + } + + // A shelf that turns out to be about one artist says so by name. + if shelf.Title != "More from Solo" { + t.Fatalf("title = %q, want %q", shelf.Title, "More from Solo") + } + + // It is the first row: it is the only one about this user. + if page.Shelves[0].Kind != ShelfMoreFromOwned { + t.Fatalf("first shelf = %q, want %q", page.Shelves[0].Kind, ShelfMoreFromOwned) + } +} + +func TestShelves_UntaggedLibraryProducesNoOwnershipShelf(t *testing.T) { + t.Parallel() + + svc, db := newShelfService(t) + + // This is the fixture library's shape, and every untagged library's: + // `in_library` is set from MusicBrainz IDs, so nothing is marked + // owned however much music is on disk. The shelf must be absent + // rather than wrong. + seedShelfRow(t, db, "release_group", "rg-1", "An Album", "A", "mbid-a", 100, false) + seedShelfRow(t, db, "release_group", "rg-2", "Another", "A", "mbid-a", 90, false) + + page := buildShelves(svc) + + if _, ok := shelfByKind(page, ShelfMoreFromOwned); ok { + t.Fatal("more-from-owned shelf built from a library that owns nothing") + } +} + +func TestShelves_TheSecondRowIsNotTheFirstRowsArtists(t *testing.T) { + t.Parallel() + + svc, db := newShelfService(t) + + // This is what the real catalog does. Ordered by raw ListenBrainz + // listen count, the top albums are one act and its members, and the + // artists shelf underneath was then the same people — one fandom + // twice, on a page whose whole job is breadth. `home`'s duplicate + // guard could not see it: the two shelves hold different entity + // types, so no two rows share an id, and the page repeats itself + // anyway because a person reads artists, not ids. + seedShelfRow(t, db, "artist", "ar-huge", "Huge", "Huge", "ar-huge", 10000, false) + seedShelfRow(t, db, "release_group", "rg-huge", "Hit", "Huge", "ar-huge", 9000, false) + seedShelfRow(t, db, "artist", "ar-next", "Next", "Next", "ar-next", 500, false) + + page := buildShelves(svc) + + albums, ok := shelfByKind(page, ShelfPopularAlbums) + if !ok { + t.Fatal("no popular-albums shelf") + } + + artists, ok := shelfByKind(page, ShelfPopularArtists) + if !ok { + t.Fatal("no popular-artists shelf") + } + + if albums.Albums[0].ArtistMBID != "ar-huge" { + t.Fatalf("albums shelf leads with %q, want ar-huge", albums.Albums[0].ArtistMBID) + } + + for _, artist := range artists.Artists { + if artist.MBID == "ar-huge" { + t.Fatal("artists shelf repeats the artist the albums shelf just showed") + } + } +} + +func TestShelves_OneAlbumPerArtist(t *testing.T) { + t.Parallel() + + svc, db := newShelfService(t) + + // A shelf is a selection, not a leaderboard: twelve slots spent on + // one artist is a row saying one thing twelve times. + seedShelfRow(t, db, "release_group", "rg-a1", "First", "Prolific", "ar-a", 900, false) + seedShelfRow(t, db, "release_group", "rg-a2", "Second", "Prolific", "ar-a", 800, false) + seedShelfRow(t, db, "release_group", "rg-a3", "Third", "Prolific", "ar-a", 700, false) + seedShelfRow(t, db, "release_group", "rg-b1", "Only", "Other", "ar-b", 100, false) + + page := buildShelves(svc) + + shelf, ok := shelfByKind(page, ShelfPopularAlbums) + if !ok { + t.Fatal("no popular-albums shelf") + } + + var titles []string + for _, album := range shelf.Albums { + titles = append(titles, album.Title) + } + + if len(titles) != 2 || titles[0] != "First" || titles[1] != "Only" { + t.Fatalf("albums = %v, want [First Only]", titles) + } +} diff --git a/e2e/specs/explore-shelves.spec.ts b/e2e/specs/explore-shelves.spec.ts new file mode 100644 index 0000000..03aace6 --- /dev/null +++ b/e2e/specs/explore-shelves.spec.ts @@ -0,0 +1,233 @@ +import { test, expect } from '../support/fixtures.js'; +import type { Page } from '@playwright/test'; + +/** + * Plan 007 phase 6 (`H-23`): Explore starts the conversation. + * + * It was a search box over a 1.1 M-row local catalog and a sentence + * telling the user to type into it — the only view in the app that + * answers "what exists" rather than "what have I got", and it would not + * begin. + * + * Two worlds, and both are asserted rather than one assertion loose + * enough to pass in either. A developer machine downloads the real + * catalog artifact on launch and gets a million rows; **CI points + * `YJ_CORE_INDEX_URL` at a dead address**, so the app there has an + * empty index — which is also every user's first run. + * + * The empty world is the interesting one and it is where the specs used + * to *skip*, which is no signal at all. So this suite stages its own + * catalog through `/__test/sql` when there is none, the way the perf + * harness builds the playlists the bulk seed does not have. That works + * only because the page asks the database whether a catalog exists + * rather than consulting a flag set at startup — the first two versions + * of that gate were cached, and rows staged afterwards were invisible + * to both. + */ +test.describe('Explore before anyone has typed', () => { + test.beforeEach(async ({ app }) => { + // Idempotent, so running it per test costs one count query when a + // catalog is already there — which is every developer machine. + await stageCatalogIfEmpty(app); + + await app.getByTestId('nav-explore').click(); + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'explore', + ); + + // The view being on screen is not the shelves being on it: they + // are fetched when it activates. Reading them a moment too early + // gives an empty list, which is also what a broken page gives. + await expect.poll(() => shelfHeadings(app)).not.toHaveLength(0); + }); + + test('says something without being typed into', async ({ app }) => { + const view = app.locator('explore-view'); + + // Shelves, on `backend/home`'s terms: a reason per row, and the + // sentence that says so beside it. That there are any at all is + // asserted in beforeEach. + const reasons = await app.evaluate( + () => + [ + ...(document + .querySelector('explore-view') + ?.shadowRoot?.querySelectorAll('.section-reason') ?? []), + ].length, + ); + + expect(reasons).toBeGreaterThan(0); + + // And the page it replaced is gone. + await expect(view).not.toContainText('Search to discover'); + }); + + test('a shelf card opens the page it is about', async ({ app }) => { + // The cards route through what already existed — no new detail + // page was invented for this. + await clickCard(app, '.album-card'); + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'explore-album-details', + ); + + await app.getByTestId('nav-explore').click(); + + await clickCard(app, '.artist-card'); + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'explore-artist-details', + ); + }); + + test('clearing a search comes back to the shelves', async ({ app }) => { + const before = await shelfHeadings(app); + + // What the search finds is not this spec's business and differs + // between the two worlds — a real catalog answers `Artists / + // Albums / Tracks`, a staged one answers nothing at all. What has + // to be true in both is that the shelves get out of the way… + await type(app, 'nirvana'); + await expect.poll(() => shelfHeadings(app)).not.toEqual(before); + + // …and that they are what the page comes back to when the query is + // cleared, rather than a mode you have to leave. + await type(app, ''); + await expect.poll(() => shelfHeadings(app)).toEqual(before); + }); + + test('two shelves are not the same shelf twice', async ({ app }) => { + // Ordered by raw listen count, the catalog's top albums are one act + // and its members and the artists row underneath was the same + // people — a duplication no id comparison can see, because the two + // rows hold different entity types. + const names = await app.evaluate(() => { + const root = document.querySelector('explore-view')?.shadowRoot; + const text = (sel: string) => + [...(root?.querySelectorAll(sel) ?? [])].map((e) => + (e.textContent ?? '').trim(), + ); + + return { + albumArtists: text('.album-card .album-artist'), + artists: text('.artist-card .artist-name'), + }; + }); + + // One album per artist, and no artist in both rows. + expect(new Set(names.albumArtists).size).toBe(names.albumArtists.length); + + for (const artist of names.artists) { + expect(names.albumArtists).not.toContain(artist); + } + }); +}); + +/** + * Give the app a catalog if it has none, so the empty-index environment + * still exercises the shelves rather than skipping them. + * + * Deliberately shaped: two artists with albums (one of them with + * three), and a third with none. The three albums are what "one album + * per artist" can be false about; the third artist is what the artists + * shelf is made of, because the other two are spent by the albums row + * above it and correctly skipped — the first version of this fixture + * had only two, and the artists shelf was rightly omitted, which read + * as a broken page. + */ +async function stageCatalogIfEmpty(app: Page): Promise { + if ((await catalogRows(app)) > 0) return; + + const rows = [ + ['artist', 'e2e-ar-a', 'Staged Alpha', 'Staged Alpha', 'e2e-ar-a', 9000], + ['artist', 'e2e-ar-b', 'Staged Beta', 'Staged Beta', 'e2e-ar-b', 500], + ['artist', 'e2e-ar-c', 'Staged Gamma', 'Staged Gamma', 'e2e-ar-c', 300], + ['release_group', 'e2e-rg-a1', 'Alpha One', 'Staged Alpha', 'e2e-ar-a', 8000], + ['release_group', 'e2e-rg-a2', 'Alpha Two', 'Staged Alpha', 'e2e-ar-a', 7000], + ['release_group', 'e2e-rg-a3', 'Alpha Three', 'Staged Alpha', 'e2e-ar-a', 6000], + ['release_group', 'e2e-rg-b1', 'Beta One', 'Staged Beta', 'e2e-ar-b', 400], + ]; + + for (const row of rows) { + const result = await app.evaluate(async (args) => { + const res = await fetch('/__test/sql', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + sql: `INSERT OR IGNORE INTO explore_index + (entity_type, mbid, title, artist_name, artist_mbid, + popularity, listener_count, primary_type) + VALUES (?, ?, ?, ?, ?, ?, ?, 'Album')`, + args: [...args, 10], + }), + }); + + return { status: res.status, body: await res.text() }; + }, row); + + // The first version of this passed six values to seven + // placeholders and never read the response, so every insert failed + // and the staging step looked exactly like a staging step. A setup + // whose failure is not checked is not setup. + expect(result.status, `staging failed: ${result.body}`).toBe(200); + } + + expect(await catalogRows(app)).toBeGreaterThan(0); +} + +/** How many catalog rows this environment has. CI has none. */ +async function catalogRows(app: Page): Promise { + const health = await app.evaluate(async () => { + const res = await fetch('/__test/health'); + + return (await res.json()) as { counts?: { exploreIndex?: number } }; + }); + + return health.counts?.exploreIndex ?? 0; +} + +async function shelfHeadings(app: Page): Promise { + return app.evaluate(() => + [ + ...(document + .querySelector('explore-view') + ?.shadowRoot?.querySelectorAll('.section-header') ?? []), + ].map((h) => (h.textContent ?? '').trim()), + ); +} + +async function clickCard(app: Page, selector: string): Promise { + await expect + .poll(() => + app.evaluate( + (sel) => + document + .querySelector('explore-view') + ?.shadowRoot?.querySelectorAll(sel).length ?? 0, + selector, + ), + ) + .toBeGreaterThan(0); + + await app.evaluate((sel) => { + document + .querySelector('explore-view') + ?.shadowRoot?.querySelector(sel) + ?.click(); + }, selector); +} + +/** Type into Explore's own search box, and wait out its debounce. */ +async function type(app: Page, query: string): Promise { + await app.evaluate((q) => { + const input = document + .querySelector('explore-view') + ?.shadowRoot?.querySelector('.search-container input'); + + if (!input) return; + + input.value = q; + input.dispatchEvent(new Event('input', { bubbles: true })); + }, query); +} diff --git a/frontend/src/components/explore-view/explore-view.ts b/frontend/src/components/explore-view/explore-view.ts index 86ef4cd..30a78e8 100644 --- a/frontend/src/components/explore-view/explore-view.ts +++ b/frontend/src/components/explore-view/explore-view.ts @@ -3,7 +3,9 @@ import { customElement, state, query as litQuery } from 'lit/decorators.js'; import '@components/page-header/page-header'; import { designTokens } from '../../styles/tokens.css'; import { srOnly } from '../../styles/sr-only.css'; -import { SearchLocal, SearchLyrics, GetThumbnail, GetThumbnails, GetArtistImageURL, RecordSearchClick } from '@go/explore/Service'; +import { SearchLocal, SearchLyrics, GetThumbnail, GetThumbnails, GetArtistImageURL, GetExploreShelves, RecordSearchClick } from '@go/explore/Service'; +import { EventsOn } from '@runtime/runtime'; +import { Events } from '../../events'; import { libraryStore } from '../../store/library-store'; import { exploreCache, ARTIST_IMAGE_CACHE_LIMIT } from '../../store/explore-cache'; import { queueStore } from '../../store/queue-store'; @@ -22,6 +24,7 @@ type LyricsResult = explore.LyricsResult; type MBArtist = explore.MBArtist; type MBReleaseGroup = explore.MBReleaseGroup; type MBRecording = explore.MBRecording; +type ShelfPage = explore.ShelfPage; /* ── Constants ── */ const MIN_QUERY_LENGTH = 2; @@ -124,6 +127,21 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { /** Lyric-search hits (library tracks matched by lyric fragment). */ @state() private lyricsResults: LyricsResult[] | null = null; + /** + * What the page shows before anyone has typed (`H-23`). + * + * `null` means "not asked yet", which is a different thing from a + * page with no shelves — the second has an answer and a reason for + * it, and says so. + */ + @state() private shelves: ShelfPage | null = null; + + /** Guards against a second fetch while the first is in flight. */ + private shelvesPending = false; + + /** Unsubscribe for the index-status listener, while active. */ + private cancelIndexStatus?: () => void; + /** Monotonic counter to discard stale responses. */ private searchVersion = 0; /** Debounce timer for live search-as-you-type. */ @@ -373,6 +391,51 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { font-size: 11px; } + /* A shelf is a reason, not a filter, and the reason is the + part that makes it one — without it a row of covers is + another grid. Same rule as the home page's shelves. */ + .section-reason { + margin: -8px 0 12px; + color: var(--yj-text-tertiary, #888); + font-size: var(--yj-text-sm, 12px); + } + + .shelves-note { + margin: 0 0 16px; + color: var(--yj-text-tertiary, #888); + font-size: var(--yj-text-sm, 12px); + } + + /* The page with no catalog. It says what is missing and how + to get it, because "no shelves" here can mean the artifact + has not been fetched — which is a thing the user can act + on, unlike an empty shelf on Home. */ + .shelves-empty { + max-width: 34em; + margin: 3em auto; + text-align: center; + color: var(--yj-text-secondary, #b3b3b3); + } + + .shelves-empty wa-icon { + font-size: 2.5em; + color: var(--yj-text-tertiary, #888); + margin-bottom: 0.5em; + } + + .shelves-empty-title { + margin: 0 0 0.5em; + font-size: var(--yj-text-lg, 15px); + font-weight: 600; + } + + .shelves-empty-body { + margin: 0; + color: var(--yj-text-tertiary, #888); + font-size: var(--yj-text-sm, 12px); + line-height: 1.5; + } + /* ── Horizontal scroll rows ── */ .horizontal-row { display: flex; @@ -653,10 +716,81 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { this.cancelPendingSearch(); } + protected override onViewActivate(): void { + // Fetched on arrival rather than on connect: this is a cached + // primary view, created and warmed at startup, so a fetch there + // is three catalog queries every user pays for whether or not + // they ever open Explore. + void this.loadShelves(); + + // The catalog can arrive after the page does — the artifact is + // downloaded and merged in the background on first run, which is + // the case where the shelves are empty *and* about to not be. + // There is no ticker behind this any more: `emitStatus` drops an + // unchanged status, so this fires when something actually + // changed, and nothing else will tell us. + this.cancelIndexStatus = EventsOn(Events.IndexStatusChanged, () => { + if (this.shelves?.state !== 'ready') void this.loadShelves(); + }); + } + /** A debounced search that lands after the user has left the page is * a query nobody asked for, against a 1.1 M-row index. */ protected override onViewDeactivate(): void { this.cancelPendingSearch(); + this.cancelIndexStatus?.(); + this.cancelIndexStatus = undefined; + } + + /** + * Load the shelves, and the art for the cards they hold. + * + * The art goes through the same two capped caches the search path + * uses rather than a third one of its own: this view never unmounts, + * so an uncapped cache on it is a leak with a long fuse + * (`perf.M7`), and a second cache holding the same data URLs would + * make both caps meaningless. + */ + private async loadShelves(): Promise { + if (this.shelvesPending) return; + + this.shelvesPending = true; + + try { + const page = await GetExploreShelves(); + + // A binding that answers with nothing is not an error to + // report — it is a page with no catalog behind it, which is + // a state this view already renders honestly. A *rejected* + // call still reaches the catch below. + if (!page || !Array.isArray(page.shelves)) { + this.shelves = explore.ShelfPage.createFrom({ + shelves: [], + state: 'no-index', + }); + + return; + } + + this.shelves = page; + + const albums = page.shelves.flatMap((shelf) => shelf.albums ?? []); + const artists = page.shelves.flatMap((shelf) => shelf.artists ?? []); + + this.loadThumbnails(albums); + void this.loadArtistImages(artists, albums); + } catch (err) { + console.error('[explore] shelves failed', err); + // Inline, and quietly: the search box above still works, so + // this is a panel that could not fill itself rather than + // something the user asked for and did not get. + this.shelves = explore.ShelfPage.createFrom({ + shelves: [], + state: 'no-index', + }); + } finally { + this.shelvesPending = false; + } } /* ── Search Logic ── */ @@ -865,8 +999,11 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { this.results?.artists || [], this.results?.releaseGroups || [], ); - this.loadThumbnails(); - this.loadArtistImages(); + this.loadThumbnails(this.results?.releaseGroups ?? []); + void this.loadArtistImages( + this.results?.artists ?? [], + this.results?.releaseGroups ?? [], + ); this.checkLibrary(); } catch (err) { if (version !== this.searchVersion) return; @@ -928,8 +1065,8 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { * `_coverArt` underscore field that searchLibraryCache stamped * on the release group. */ - private seedThumbnailsFromLibrary() { - if (!this.results?.releaseGroups?.length) return; + private seedThumbnailsFromLibrary(releaseGroups: MBReleaseGroup[]) { + if (!releaseGroups.length) return; const cachedAlbums = libraryStore.cachedAlbums; const libAlbumsByMBID = new Map(); @@ -944,7 +1081,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { let updated = false; - for (const rg of this.results.releaseGroups) { + for (const rg of releaseGroups) { if (this.thumbnailCache.has(rg.mbid)) continue; const localArt = libAlbumsByMBID.get(rg.mbid) || (rg as any)._coverArt; @@ -966,8 +1103,8 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { * field that searchLibraryCache stamped on the artist. Falls * back to library album art when an artist has no portrait. */ - private seedArtistImagesFromLibrary() { - if (!this.results?.artists?.length) return; + private seedArtistImagesFromLibrary(artists: MBArtist[]) { + if (!artists.length) return; const cachedArtists = libraryStore.cachedArtists; const libByMBID = new Map(); @@ -982,7 +1119,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { let updated = false; - for (const a of this.results.artists) { + for (const a of artists) { if (!a.mbid || this.artistImageCache.has(a.mbid)) continue; const local = libByMBID.get(a.mbid) || (a as any)._imageMedium || (a as any)._imageSmall; @@ -1005,18 +1142,18 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { } } - private loadThumbnails() { - if (this.thumbnailBatchPending || !this.results?.releaseGroups?.length) { + private loadThumbnails(releaseGroups: MBReleaseGroup[]) { + if (this.thumbnailBatchPending || !releaseGroups.length) { return; } // Always seed from local library first. - this.seedThumbnailsFromLibrary(); + this.seedThumbnailsFromLibrary(releaseGroups); // Collect MBIDs that still need fetching from the API. const requests: ThumbnailRequest[] = []; - for (const rg of this.results.releaseGroups) { + for (const rg of releaseGroups) { if (!this.thumbnailCache.has(rg.mbid)) { requests.push({ mbid: rg.mbid, @@ -1083,14 +1220,17 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { * Load artist images for all visible artist cards. Each call * is async and updates the cache + re-renders on success. */ - private async loadArtistImages() { - if (!this.results?.artists?.length) return; + private async loadArtistImages( + artists: MBArtist[], + releaseGroups: MBReleaseGroup[] = [], + ) { + if (!artists.length) return; // Seed from library store first (instant, no API). - this.seedArtistImagesFromLibrary(); + this.seedArtistImagesFromLibrary(artists); // Fetch remaining from API (only artists not yet resolved). - for (const a of this.results.artists) { + for (const a of artists) { if (this.artistImageCache.has(a.mbid)) continue; this.artistImageCache.set(a.mbid, ''); @@ -1111,7 +1251,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { // Try library store first, then search-result release groups. let fallbackUpdated = false; - for (const a of this.results.artists) { + for (const a of artists) { if (a.mbid && !this.artistImageCache.get(a.mbid)) { // 1) Library album art const albumArt = getArtistAlbumArt(a.name); @@ -1121,10 +1261,11 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { continue; } - // 2) Cover art from a search-result release group by this artist - if (this.results.releaseGroups) { + // 2) Cover art from a release group by this artist in + // the same batch — a search's results, or a shelf's. + { const name = a.name.toLowerCase(); - for (const rg of this.results.releaseGroups) { + for (const rg of releaseGroups) { if (rg.mbid && rg.artistCredit?.toLowerCase().includes(name)) { const url = this.thumbnailCache.get(rg.mbid); if (url) { @@ -1142,7 +1283,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { // Sync resolved images into the explore cache so detail pages // pick them up without redundant API calls. - for (const a of this.results.artists) { + for (const a of artists) { const url = this.artistImageCache.get(a.mbid); if (url) { const cached = exploreCache.getArtist(a.mbid); @@ -1462,11 +1603,11 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { `; } - // No query entered yet + // No query entered yet: this is where the page used to be a + // sentence telling the user to type into a 1.1 M-row catalog + // they had never seen (`H-23`). if (!this.searchQuery.trim() && !this.results) { - return html`
- Search to discover artists, albums, and tracks. -
`; + return this.renderShelves(); } // Loading state already shown above @@ -1509,12 +1650,88 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { return nothing; } + /* ── Shelves ── */ + + /** + * The page before a query: shelves, or an honest account of why + * there are none. + * + * Explore's data is a downloaded artifact, so "no shelves" is not + * the same statement it is on Home. Home's empty shelf means a + * library with no history, which is true and permanent-ish. Here it + * can mean the catalog has not arrived yet — so a page that renders + * nothing and explains nothing is the blank panel this whole feature + * is removing, wearing different clothes. + */ + private renderShelves() { + const page = this.shelves; + + // Not asked yet. Deliberately blank rather than a spinner: the + // call is three local index queries and lands in a few + // milliseconds, and a spinner that flashes is worse than a beat + // of nothing. + if (!page) return nothing; + + if (page.shelves.length === 0) { + return html` +
+ +

+ ${page.state === 'building' + ? 'The music catalog is still downloading.' + : 'The music catalog has not been downloaded yet.'} +

+

+ ${page.state === 'building' + ? 'Suggestions will appear here when it finishes. Search still works over whatever has arrived.' + : 'Explore suggests music from a catalog of over a million albums. You can fetch it from Settings — search still works without it, over anything already indexed.'} +

+
+ `; + } + + return html` +
+ ${page.state === 'building' + ? html`

+ The catalog is still downloading, so there is more + to come. +

` + : nothing} + ${page.shelves.map((shelf) => + shelf.artists?.length + ? this.renderArtistsSection( + shelf.artists, + shelf.title, + shelf.subtitle, + ) + : this.renderAlbumsSection( + shelf.albums ?? [], + shelf.title, + shelf.subtitle, + ), + )} +
+ `; + } + /* ── Section Renderers ── */ - private renderArtistsSection(artists: MBArtist[]) { + private renderArtistsSection( + artists: MBArtist[], + heading = 'Artists', + subtitle = '', + ) { return html`
-

Artists

+

${heading}

+ ${subtitle + ? html`

${subtitle}

` + : nothing}
${artists.map((a) => { const hue = nameToHue(a.name); @@ -1566,10 +1783,17 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) { `; } - private renderAlbumsSection(releaseGroups: MBReleaseGroup[]) { + private renderAlbumsSection( + releaseGroups: MBReleaseGroup[], + heading = 'Albums', + subtitle = '', + ) { return html`
-

Albums

+

${heading}

+ ${subtitle + ? html`

${subtitle}

` + : nothing}
${releaseGroups.map((rg) => { const artURL = this.thumbnailCache.get(rg.mbid) || ''; diff --git a/frontend/test/components/explore-shelves.test.ts b/frontend/test/components/explore-shelves.test.ts new file mode 100644 index 0000000..68d9f22 --- /dev/null +++ b/frontend/test/components/explore-shelves.test.ts @@ -0,0 +1,209 @@ +/** + * Plan 007 phase 6 (`H-23`): Explore starts the conversation. + * + * The page was a search box over a 1.1 M-row local catalog and a + * sentence telling the user to type into it. It now opens with shelves + * — on `backend/home`'s terms, where a shelf is a *reason* rather than + * a filter and carries the sentence that says so. + * + * What this tier pins is the half that is a rendering decision rather + * than a query: that a reason is drawn next to its row, that a shelf of + * artists is not a shelf of albums, and — the part that matters most — + * that a page with no shelves says which of the three reasons it has no + * shelves for. Home can omit an empty shelf and be honest, because a + * library with no history really has less to say. Explore's data is a + * *downloaded artifact*, so an empty page there might just be a page + * that does not know yet, and rendering nothing is the blank panel this + * whole feature exists to remove. + */ +import { beforeEach, describe, expect, it } from 'vitest'; + +import '@components/explore-view/explore-view'; +import { flush, stub } from '@test/support/harness'; +import { fixture, shadow, shadowAll, texts } from '@test/support/render'; + +const SHELVES = 'explore.Service.GetExploreShelves'; + +type Shelves = { + shelves: { + id: string; + kind: string; + title: string; + subtitle: string; + albums?: unknown[]; + artists?: unknown[]; + }[]; + state: string; +}; + +const album = (title: string, artist = 'An Artist') => ({ + mbid: `rg-${title}`, + title, + artistCredit: artist, + artistMbid: 'ar-1', + primaryType: 'Album', + firstReleaseDate: '1994-05-01', + popularity: 100, + listenerCount: 10, + inLibrary: false, + secondaryTypes: [], +}); + +const artist = (name: string) => ({ + mbid: `ar-${name}`, + name, + sortName: name, + type: 'Group', + country: 'GB', + disambiguation: '', + score: 0, + popularity: 100, + listenerCount: 10, + inLibrary: false, +}); + +/** Mount Explore and activate it, which is what fetches the shelves. */ +async function explore(page: Shelves) { + stub(SHELVES, page); + + const el = await fixture('explore-view'); + + (el as unknown as { viewActivated(): void }).viewActivated(); + await flush(); + await el.updateComplete; + + return el; +} + +describe(' shelves', () => { + beforeEach(() => { + stub('explore.Service.GetThumbnails', []); + }); + + it('opens with shelves instead of telling the user to type', async () => { + const el = await explore({ + state: 'ready', + shelves: [ + { + id: 'popular-albums', + kind: 'popular-albums', + title: 'Popular right now', + subtitle: "The most listened-to albums you don't already own", + albums: [album('One'), album('Two')], + }, + ], + }); + + expect(texts(el, '.section-header')).toEqual(['Popular right now']); + expect(shadowAll(el, '.album-card')).toHaveLength(2); + expect(el.shadowRoot?.textContent).not.toContain('Search to discover'); + }); + + it('draws the reason next to the row, not just the title', async () => { + // Without it a shelf is indistinguishable from a random grid — + // which is the whole difference between a shelf and a filter. + const el = await explore({ + state: 'ready', + shelves: [ + { + id: 'more-from-owned', + kind: 'more-from-owned', + title: 'More from Solo', + subtitle: 'The rest of what they made', + albums: [album('Second')], + }, + ], + }); + + expect(texts(el, '.section-reason')).toEqual([ + 'The rest of what they made', + ]); + }); + + it('renders an artist shelf as artists, not as albums', async () => { + const el = await explore({ + state: 'ready', + shelves: [ + { + id: 'popular-artists', + kind: 'popular-artists', + title: 'Artists worth knowing', + subtitle: 'Widely listened to, and not yet in your library', + artists: [artist('One'), artist('Two')], + }, + ], + }); + + expect(shadowAll(el, '.artist-card')).toHaveLength(2); + expect(shadowAll(el, '.album-card')).toHaveLength(0); + }); + + it('says the catalog is missing rather than rendering nothing', async () => { + const el = await explore({ state: 'no-index', shelves: [] }); + + const empty = shadow(el, '.shelves-empty'); + + expect(empty).not.toBeNull(); + expect(empty?.textContent).toContain('has not been downloaded'); + // …and points at the one thing the user can do about it. + expect(empty?.textContent).toContain('Settings'); + }); + + it('distinguishes a catalog that is still arriving from one that is absent', async () => { + // The same empty page for both would tell a first-run user their + // catalog is missing while it is downloading in the background. + const el = await explore({ state: 'building', shelves: [] }); + + const empty = shadow(el, '.shelves-empty'); + + expect(empty?.textContent).toContain('still downloading'); + expect(empty?.textContent).not.toContain('has not been downloaded'); + }); + + it('admits a partial page while the catalog is still arriving', async () => { + const el = await explore({ + state: 'building', + shelves: [ + { + id: 'popular-albums', + kind: 'popular-albums', + title: 'Popular right now', + subtitle: 'The most listened-to albums', + albums: [album('One')], + }, + ], + }); + + expect( + shadow(el, '.shelves-note')?.textContent?.replace(/\s+/g, ' '), + ).toContain('there is more to come'); + }); + + it('does not fetch shelves until the view is on screen', async () => { + // Every primary view is created and warmed at startup, so a fetch + // on connect is three catalog queries paid by users who never open + // Explore. + stub(SHELVES, { state: 'ready', shelves: [] }); + + // Mounted the way `index.ts` creates a cached view: hidden, which + // is exactly what stops the mixin activating it on connect. + const el = document.createElement('explore-view'); + + el.classList.add('view-hidden'); + document.body.append(el); + await (el as unknown as { updateComplete: Promise }) + .updateComplete; + await flush(); + + const { calls } = await import('@test/support/harness'); + + expect(calls(SHELVES)).toHaveLength(0); + + (el as unknown as { viewActivated(): void }).viewActivated(); + await flush(); + + expect(calls(SHELVES)).toHaveLength(1); + + el.remove(); + }); +}); diff --git a/frontend/wailsjs/go/explore/Service.d.ts b/frontend/wailsjs/go/explore/Service.d.ts index 7a7aa3d..6932ea7 100755 --- a/frontend/wailsjs/go/explore/Service.d.ts +++ b/frontend/wailsjs/go/explore/Service.d.ts @@ -39,6 +39,8 @@ export function GetArtistPlayCount(arg1:string):Promise; export function GetCandidateThumbnail(arg1:string,arg2:string):Promise; +export function GetExploreShelves():Promise; + export function GetIndexStatus():Promise; export function GetLibrarySimilarArtists(arg1:string):Promise>; diff --git a/frontend/wailsjs/go/explore/Service.js b/frontend/wailsjs/go/explore/Service.js index f52d433..81f8101 100755 --- a/frontend/wailsjs/go/explore/Service.js +++ b/frontend/wailsjs/go/explore/Service.js @@ -70,6 +70,10 @@ export function GetCandidateThumbnail(arg1, arg2) { return window['go']['explore']['Service']['GetCandidateThumbnail'](arg1, arg2); } +export function GetExploreShelves() { + return window['go']['explore']['Service']['GetExploreShelves'](); +} + export function GetIndexStatus() { return window['go']['explore']['Service']['GetIndexStatus'](); } diff --git a/frontend/wailsjs/go/models.ts b/frontend/wailsjs/go/models.ts index e67bc19..949c39a 100755 --- a/frontend/wailsjs/go/models.ts +++ b/frontend/wailsjs/go/models.ts @@ -1322,6 +1322,78 @@ export namespace explore { } } + export class Shelf { + id: string; + kind: string; + title: string; + subtitle: string; + albums?: MBReleaseGroup[]; + artists?: MBArtist[]; + + static createFrom(source: any = {}) { + return new Shelf(source); + } + + constructor(source: any = {}) { + if ('string' === typeof source) source = JSON.parse(source); + this.id = source["id"]; + this.kind = source["kind"]; + this.title = source["title"]; + this.subtitle = source["subtitle"]; + this.albums = this.convertValues(source["albums"], MBReleaseGroup); + this.artists = this.convertValues(source["artists"], MBArtist); + } + + convertValues(a: any, classs: any, asMap: boolean = false): any { + if (!a) { + return a; + } + if (a.slice && a.map) { + return (a as any[]).map(elem => this.convertValues(elem, classs)); + } else if ("object" === typeof a) { + if (asMap) { + for (const key of Object.keys(a)) { + a[key] = new classs(a[key]); + } + return a; + } + return new classs(a); + } + return a; + } + } + export class ShelfPage { + shelves: Shelf[]; + state: string; + + static createFrom(source: any = {}) { + return new ShelfPage(source); + } + + constructor(source: any = {}) { + if ('string' === typeof source) source = JSON.parse(source); + this.shelves = this.convertValues(source["shelves"], Shelf); + this.state = source["state"]; + } + + convertValues(a: any, classs: any, asMap: boolean = false): any { + if (!a) { + return a; + } + if (a.slice && a.map) { + return (a as any[]).map(elem => this.convertValues(elem, classs)); + } else if ("object" === typeof a) { + if (asMap) { + for (const key of Object.keys(a)) { + a[key] = new classs(a[key]); + } + return a; + } + return new classs(a); + } + return a; + } + } export class ThumbnailRequest { mbid: string; albumName: string;