diff --git a/docs/rfcs/RFC-011-metadata-enrichment.md b/docs/rfcs/RFC-011-metadata-enrichment.md index 32cb361..9d7dce6 100644 --- a/docs/rfcs/RFC-011-metadata-enrichment.md +++ b/docs/rfcs/RFC-011-metadata-enrichment.md @@ -7,8 +7,10 @@ key), #270 (the pin moved onto them), #271 (the cache and the engine that fills it), #272 (`getArtistInfo`, `getArtistInfo2` and the native artist page read it), #273 and #274 (`getSimilarSongs` and `getSimilarSongs2` draw - from it), and the pull request closing #268 (portraits from Deezer). The web - client and the compatibility campaign are still to come. The *Status* field + from it), #275 (portraits from Deezer), and the pull request closing #269 + (the web client's artist page, and `enrichment.image` on the native API, + which #275 had left out). The compatibility campaign is still to come. The + *Status* field above never flips in this project; this line is the one that says what is running. - **Date**: 2026-10-14 diff --git a/src/lib.rs b/src/lib.rs index f79df84..e1c713e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -257,6 +257,7 @@ fn chunk_body_limit(limits: &config::UploadLimits) -> usize { services::EnrichmentState, services::EnrichmentSource, services::EnrichedBio, + services::EnrichedImage, services::EnrichedSimilar, services::SearchResult, services::PlaylistItem, diff --git a/src/services/artist_enrichment.rs b/src/services/artist_enrichment.rs index 3ebb5d5..1243016 100644 --- a/src/services/artist_enrichment.rs +++ b/src/services/artist_enrichment.rs @@ -70,6 +70,7 @@ impl DomainServices { .neighbours(user_id, &rows, &canonical, similar_limit) .await?; let bio = merged_bio(&rows); + let image = merged_image(&rows); // Only a fresh row is an answer. An expired one has just had its // refresh scheduled, and an expired miss is not "nobody knows" but // "being asked again" — `pending`, as decision 11 defines it. Stale @@ -77,13 +78,14 @@ impl DomainServices { let now = crate::authentication::now_ms(); let answered = rows.iter().filter(|row| row.expires_at > now).count(); let state = state_of( - bio.is_some() || similar.is_some(), + bio.is_some() || image.is_some() || similar.is_some(), answered, engine.provider_count(), ); Ok(ArtistEnrichment { state, bio, + image, similar, }) } @@ -321,6 +323,15 @@ fn merged_bio(rows: &[CachedArtist]) -> Option { }) } +fn merged_image(rows: &[CachedArtist]) -> Option { + rows.iter().find_map(|row| { + row.image_hash.as_ref().map(|hash| EnrichedImage { + artwork_hash: hash.clone(), + source: source_of(row), + }) + }) +} + /// Content wins over everything; without it, a provider that has not /// answered makes the lookup `pending`, and only when every one of them has /// answered is it `none`. diff --git a/src/services/mod.rs b/src/services/mod.rs index 01d5bd3..2d44f8b 100644 --- a/src/services/mod.rs +++ b/src/services/mod.rs @@ -956,6 +956,8 @@ pub struct ArtistEnrichment { pub state: EnrichmentState, /// The first configured provider with a biography supplies it. pub bio: Option, + /// The first configured provider with a portrait supplies it. + pub image: Option, /// The first configured provider whose neighbours include artists the /// caller can see supplies the list — those artists only, best first. pub similar: Option, @@ -966,6 +968,7 @@ impl ArtistEnrichment { Self { state: EnrichmentState::Disabled, bio: None, + image: None, similar: None, } } @@ -1003,6 +1006,14 @@ pub struct EnrichedBio { pub source: EnrichmentSource, } +/// A portrait, as a key into the artwork store. +#[derive(Debug, Clone, Serialize, ToSchema)] +pub struct EnrichedImage { + /// Served by the artwork route, as every `artwork_hash` is. + pub artwork_hash: String, + pub source: EnrichmentSource, +} + #[derive(Debug, Clone, Serialize, ToSchema)] pub struct EnrichedSimilar { pub artists: Vec, diff --git a/tests/enrichment.rs b/tests/enrichment.rs index 530c66c..9e72cde 100644 --- a/tests/enrichment.rs +++ b/tests/enrichment.rs @@ -1216,6 +1216,24 @@ async fn a_portrait_is_fetched_once_and_shown_on_every_artist_with_the_name() { assert_eq!(source, "deezer"); assert_eq!(artist_events(&state).await, 2); assert_eq!(listener.fake.images.load(Ordering::SeqCst), 1); + + // Decision 11: the native page names where the portrait came from. + state.services.use_enrichment(enrichment); + let bjork = artist_id(&state, owner, "Björk").await; + let enriched = state + .services + .artist_page(owner, bjork) + .await + .unwrap() + .enrichment + .expect("the native page always carries it"); + let image = enriched.image.expect("the portrait is published"); + assert!(image.artwork_hash == hash, "another portrait was published"); + assert_eq!(image.source.provider, "deezer"); + assert_eq!( + enriched.state, + waveflow_server::services::EnrichmentState::Ready + ); } /// A picture URL naming a host off the allowlist is dropped without being diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index ff75f41..b01ba5f 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -235,6 +235,15 @@ const artistDetail = { let artistPlayed: Array> = []; /** The limits the page asked for, so a screenful can be told from a catalogue. */ let topSongAsks: Array = []; +/** How many times the artist page was asked for. */ +let artistAsks = 0; +/** + * The `enrichment` object of the artist's nth answer, counting from one. + * Undefined leaves it out, as a server from before RFC-011 does. + */ +let enrichmentOf: (ask: number) => unknown = () => undefined; +/** Every artist answer after the first is a 500. */ +let artistFailsAfterFirst = false; /** * A dry run's answer: three of five entries placed, two it could not. @@ -1032,7 +1041,14 @@ async function mockAuthenticatedApi(page: Page) { return; } if (url.pathname === "/api/v2/artists/artist-1") { - await route.fulfill({ json: artistDetail }); + artistAsks += 1; + if (artistFailsAfterFirst && artistAsks > 1) { + await route.fulfill({ status: 500, json: { error: "internal" } }); + return; + } + await route.fulfill({ + json: { ...artistDetail, enrichment: enrichmentOf(artistAsks) }, + }); return; } if (url.pathname === "/api/v2/albums") { @@ -1061,6 +1077,9 @@ test.beforeEach(async ({ page }) => { radioGate = Promise.resolve(); artistPlayed = [song(2, "Cocoon", 0, false), song(1, "Hidden Place", 5, true)]; topSongAsks = []; + artistAsks = 0; + enrichmentOf = () => undefined; + artistFailsAfterFirst = false; // **Reset here, and they were not.** These four reached the mock without // reaching this block, because a `replace` whose needle had already been // reformatted did nothing and said nothing. So `subsonicWrites` @@ -1535,6 +1554,109 @@ test("an artist page shows this account's most played, and no heading without a ).toHaveCount(0); }); +/** + * RFC-011 decision 11 on the screen: what a provider said, and which one said + * it. The first answer is `pending`, as an artist's first visit is; the page + * asks again on its own, since the web client reads no event stream, and the + * second answer fills it. + */ +test("an artist page shows what the server learned, and where each part came from", async ({ + page, +}) => { + const lastfm = { provider: "lastfm", fetched_at: Date.UTC(2026, 9, 14) }; + enrichmentOf = (ask) => + ask === 1 + ? { state: "pending", bio: null, image: null, similar: null } + : { + state: "ready", + bio: { + summary: "An Icelandic singer and songwriter.", + full: "An Icelandic singer and songwriter.\nHer albums are many.", + source: lastfm, + }, + image: { + artwork_hash: "a".repeat(64), + source: { provider: "deezer", fetched_at: Date.UTC(2026, 9, 15) }, + }, + similar: { + artists: [ + { + id: "artist-2", + name: "Sigur Rós", + album_count: 1, + artwork_hash: null, + }, + ], + source: lastfm, + }, + }; + // Only the portrait's hash answers with an image: the artist row names + // none, so the hero can show it only through `enrichment.image`. + const png = Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==", + "base64", + ); + await page.route(`**/api/v2/artwork/${"a".repeat(64)}`, async (route) => { + await route.fulfill({ contentType: "image/png", body: png }); + }); + + await page.goto("/artists/artist-1"); + await expect(page.getByRole("status")).toContainText( + "Looking this artist up", + ); + await expect(page.getByRole("heading", { name: "About" })).toBeVisible({ + timeout: 10_000, + }); + await expect(page.getByText("An Icelandic singer and songwriter.").first()).toBeVisible(); + await expect(page.getByText(/^Biography from Last\.fm, /)).toBeVisible(); + await expect(page.getByText(/^Portrait from Deezer, /)).toBeVisible(); + await expect(page.locator(".artist-hero img.artist-portrait")).toBeVisible(); + await expect( + page.getByRole("heading", { name: "Similar artists in your library" }), + ).toBeVisible(); + await expect(page.getByRole("link", { name: /Sigur Rós/ })).toBeVisible(); + await expect(page.getByText(/^Suggested by Last\.fm, /)).toBeVisible(); + await expect(page.getByText("Looking this artist up")).toHaveCount(0); + expect(artistAsks).toBe(2); + + const results = await new AxeBuilder({ page }) + .withTags(["wcag2a", "wcag2aa", "wcag21a", "wcag21aa"]) + .analyze(); + expect(results.violations).toEqual([]); +}); + +/** `none` is an answer: nothing is shown, and nothing is asked again. */ +test("an artist nobody knows shows no enrichment and is not asked again", async ({ + page, +}) => { + enrichmentOf = () => ({ state: "none", bio: null, image: null, similar: null }); + await page.goto("/artists/artist-1"); + await expect(page.getByRole("link", { name: "Open the album: Vespertine" })).toBeVisible(); + await page.waitForTimeout(4_000); + await expect(page.getByRole("heading", { name: "About" })).toHaveCount(0); + await expect(page.getByText("Looking this artist up")).toHaveCount(0); + expect(artistAsks).toBe(1); +}); + +/** + * A server that fails after a `pending` answer leaves that answer on screen, + * and a failed ask does not move the count of answers. The failures are + * counted with them, so the page still stops at the bound. + */ +test("a pending artist whose server keeps failing is asked a bounded number of times", async ({ + page, +}) => { + test.setTimeout(45_000); + artistFailsAfterFirst = true; + enrichmentOf = () => ({ state: "pending", bio: null, image: null, similar: null }); + await page.goto("/artists/artist-1"); + await expect(page.getByRole("heading", { name: "Björk", level: 2 })).toBeVisible(); + await expect.poll(() => artistAsks, { timeout: 30_000 }).toBe(6); + await page.waitForTimeout(5_000); + expect(artistAsks).toBe(6); + await expect(page.getByText("Looking this artist up")).toHaveCount(0); +}); + /** * `GET /history` answers plays and not songs, so every distinct track on this * screen is a request of its own, and they all leave together. Without a cap a diff --git a/webapp/src/api.ts b/webapp/src/api.ts index 127546e..5456f0a 100644 --- a/webapp/src/api.ts +++ b/webapp/src/api.ts @@ -87,7 +87,37 @@ export type Artist = { artwork_hash: string | null; }; -export type ArtistDetail = Artist & { albums: Album[] }; +/** Where one enriched field came from — RFC-011 decision 11. */ +export type EnrichmentSource = { + /** As `WAVEFLOW_METADATA_PROVIDERS` spells it: `lastfm`, `deezer`. */ + provider: string; + /** When the provider gave this answer, in epoch milliseconds. */ + fetched_at: number; +}; + +/** + * What the server learned about an artist from outside the library. + * + * `pending` means a provider has not answered yet, `none` that every one + * answered with nothing, `disabled` that none is configured. The page asks + * again while `pending`, a bounded number of times. + */ +export type ArtistEnrichment = { + state: "disabled" | "pending" | "ready" | "none"; + bio: { + summary: string | null; + full: string | null; + source: EnrichmentSource; + } | null; + image: { artwork_hash: string; source: EnrichmentSource } | null; + similar: { artists: Artist[]; source: EnrichmentSource } | null; +}; + +export type ArtistDetail = Artist & { + albums: Album[]; + /** Always present on the native page; optional so an older server parses. */ + enrichment?: ArtistEnrichment; +}; export type SearchResult = { artists: Artist[]; diff --git a/webapp/src/i18n.tsx b/webapp/src/i18n.tsx index c7714ec..8614a04 100644 --- a/webapp/src/i18n.tsx +++ b/webapp/src/i18n.tsx @@ -182,6 +182,13 @@ const en = { other: "{count} albums in your library", }, "artists.mostPlayed": "Most played here", + "artists.about": "About", + "artists.readMore": "Read more", + "artists.similar": "Similar artists in your library", + "artists.enrichmentPending": "Looking this artist up…", + "artists.bioSource": "Biography from {provider}, {date}.", + "artists.similarSource": "Suggested by {provider}, {date}.", + "artists.portraitSource": "Portrait from {provider}, {date}.", "favourites.detail": { one: "{count} saved track", other: "{count} saved tracks", @@ -718,6 +725,13 @@ const fr: Record = { other: "{count} albums dans votre bibliothèque", }, "artists.mostPlayed": "Les plus écoutés ici", + "artists.about": "À propos", + "artists.readMore": "Lire la suite", + "artists.similar": "Artistes proches dans votre bibliothèque", + "artists.enrichmentPending": "Recherche d’informations sur cet artiste…", + "artists.bioSource": "Biographie fournie par {provider}, le {date}.", + "artists.similarSource": "Suggérés par {provider}, le {date}.", + "artists.portraitSource": "Portrait fourni par {provider}, le {date}.", "favourites.detail": { one: "{count} piste enregistrée", other: "{count} pistes enregistrées", diff --git a/webapp/src/pages.tsx b/webapp/src/pages.tsx index ad2aa89..864cc07 100644 --- a/webapp/src/pages.tsx +++ b/webapp/src/pages.tsx @@ -9,6 +9,7 @@ import { type FormEvent, Fragment, useEffect, useMemo, useState } from "react"; import { type Album, type AlbumSort, + type ArtistEnrichment, addLibrary, appendToPlaylist, authorize, @@ -64,6 +65,7 @@ import { artistQuery, artistsQuery, artistTopSongsQuery, + enrichmentRetryDelay, favoriteTracksQuery, genreSongsQuery, genresQuery, @@ -1913,9 +1915,91 @@ export function ArtistsPage() { ); } +/** A provider as a reader knows it, rather than as the setting spells it. */ +const PROVIDER_NAMES: Record = { + lastfm: "Last.fm", + deezer: "Deezer", +}; + +/** + * What the server learned about an artist, and where each part came from. + * + * Provenance is part of the data (RFC-011 decision 11): each field names its + * provider and the day it answered, because the fields are merged across + * providers and one credit for the whole section would be wrong as soon as + * two contributed. `none` and `disabled` render nothing; `pending` says so + * only while the page is still asking. + */ +function ArtistEnrichmentSection({ + enrichment, + asking, +}: { + enrichment: ArtistEnrichment | undefined; + asking: boolean; +}) { + const { t, locale } = useI18n(); + if (!enrichment) return null; + const credit = (source: { provider: string; fetched_at: number }) => ({ + provider: PROVIDER_NAMES[source.provider] ?? source.provider, + date: new Intl.DateTimeFormat(locale, { dateStyle: "medium" }).format( + new Date(source.fetched_at), + ), + }); + const { bio, similar } = enrichment; + const full = bio?.full && bio.full !== bio.summary ? bio.full : null; + return ( + <> + {enrichment.state === "pending" && asking ? ( +

+ {t("artists.enrichmentPending")} +

+ ) : null} + {bio && (bio.summary || full) ? ( +
+

{t("artists.about")}

+ {bio.summary ?

{bio.summary}

: null} + {full ? ( +
+ {t("artists.readMore")} +

{full}

+
+ ) : null} +

+ {t("artists.bioSource", credit(bio.source))} +

+
+ ) : null} + {similar && similar.artists.length > 0 ? ( +
+

{t("artists.similar")}

+
    + {similar.artists.map((artist) => ( +
  • + + + {artist.name} + +
    • + ))} +
+

+ {t("artists.similarSource", credit(similar.source))} +

+
+ ) : null} + + ); +} + export function ArtistPage({ artistId }: { artistId: string }) { - const { t } = useI18n(); - const { data: value, error } = useQuery(artistQuery(artistId)); + const { t, locale } = useI18n(); + const client = useQueryClient(); + const query = artistQuery(artistId); + const { data: value, error } = useQuery(query); /** * Asked beside the artist rather than with it, and never blocking it. * @@ -1926,11 +2010,23 @@ export function ArtistPage({ artistId }: { artistId: string }) { */ const { data: played } = useQuery(artistTopSongsQuery(artistId)); if (!value) return ; + const image = value.enrichment?.image ?? null; + // The artist row carries the portrait once the server has linked it; the + // enrichment names it as soon as it is cached, which may be a visit sooner. + const portrait = value.artwork_hash ?? image?.artwork_hash ?? null; + // Counted exactly as `artistQuery` counts, so the note goes when the asking + // does. + const asked = client.getQueryState(query.queryKey); + const asking = + enrichmentRetryDelay( + value, + (asked?.dataUpdateCount ?? 0) + (asked?.errorUpdateCount ?? 0), + ) !== false; return (
@@ -1940,8 +2036,21 @@ export function ArtistPage({ artistId }: { artistId: string }) {

{t("artists.libraryCount", { count: value.album_count })}

+ {image && image.artwork_hash === portrait ? ( +

+ {t("artists.portraitSource", { + provider: + PROVIDER_NAMES[image.source.provider] ?? + image.source.provider, + date: new Intl.DateTimeFormat(locale, { + dateStyle: "medium", + }).format(new Date(image.source.fetched_at)), + })} +

+ ) : null}
+ {/* Shown only once there is a history to show. An artist nobody has played has nothing to say here, and a heading above an empty table would say it anyway. */} diff --git a/webapp/src/queries.test.ts b/webapp/src/queries.test.ts new file mode 100644 index 0000000..aef46d1 --- /dev/null +++ b/webapp/src/queries.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from "vitest"; + +import type { ArtistDetail, ArtistEnrichment } from "./api"; +import { + ENRICHMENT_ASKS, + ENRICHMENT_RETRY_MS, + enrichmentRetryDelay, +} from "./queries"; + +const artist = (state?: ArtistEnrichment["state"]): ArtistDetail => ({ + id: "artist-1", + name: "Björk", + album_count: 0, + artwork_hash: null, + albums: [], + enrichment: state + ? { state, bio: null, image: null, similar: null } + : undefined, +}); + +/** + * The web client learns that a provider answered only by asking again, so a + * pending artist is asked again — and a provider that never answers must not + * keep an open page asking forever. + */ +describe("enrichmentRetryDelay", () => { + it("asks again while pending, then stops at the bound", () => { + expect(enrichmentRetryDelay(artist("pending"), 1)).toBe( + ENRICHMENT_RETRY_MS, + ); + expect(enrichmentRetryDelay(artist("pending"), ENRICHMENT_ASKS - 1)).toBe( + ENRICHMENT_RETRY_MS, + ); + expect(enrichmentRetryDelay(artist("pending"), ENRICHMENT_ASKS)).toBe( + false, + ); + }); + + it("never asks again once there is an answer, or nothing to wait for", () => { + for (const state of ["ready", "none", "disabled"] as const) { + expect(enrichmentRetryDelay(artist(state), 1)).toBe(false); + } + expect(enrichmentRetryDelay(artist(), 1)).toBe(false); + expect(enrichmentRetryDelay(undefined, 0)).toBe(false); + }); +}); diff --git a/webapp/src/queries.ts b/webapp/src/queries.ts index 95ae914..b418ade 100644 --- a/webapp/src/queries.ts +++ b/webapp/src/queries.ts @@ -2,6 +2,7 @@ import { infiniteQueryOptions, queryOptions } from "@tanstack/react-query"; import { type AlbumSort, + type ArtistDetail, artistTopSongs, canvasUrl, getAlbum, @@ -94,10 +95,41 @@ export const artistsQuery = (scope: string | undefined) => queryFn: () => listArtists(scope), }); +/** How many times an artist page is asked in all while enrichment is pending. */ +export const ENRICHMENT_ASKS = 6; +/** The pause between two of those asks. */ +export const ENRICHMENT_RETRY_MS = 3_000; + +/** + * When to ask for an artist again, or `false` for never. + * + * The web client does not read the library event stream, so it learns that a + * provider answered only by asking. It asks while the state is `pending`, up + * to {@link ENRICHMENT_ASKS} answers in all, and then stops: a provider that + * is unreachable stays `pending` on the server, and a page left open must + * not ask forever. The count is the query's, so a later visit refetches once + * on mount, as every page does, and does not start the series again. + */ +export function enrichmentRetryDelay( + artist: ArtistDetail | undefined, + answers: number, +): number | false { + if (artist?.enrichment?.state !== "pending") return false; + return answers < ENRICHMENT_ASKS ? ENRICHMENT_RETRY_MS : false; +} + export const artistQuery = (artistId: string) => queryOptions({ queryKey: ["artist", artistId], queryFn: () => getArtist(artistId), + // Failed asks count too: an error leaves the last answer — still + // `pending` — in place and does not move `dataUpdateCount`, so a server + // that keeps failing would otherwise be asked every few seconds for ever. + refetchInterval: (query) => + enrichmentRetryDelay( + query.state.data, + query.state.dataUpdateCount + query.state.errorUpdateCount, + ), }); export const artistTopSongsQuery = (artistId: string) => diff --git a/webapp/src/styles.css b/webapp/src/styles.css index 8fd5f34..8c165e9 100644 --- a/webapp/src/styles.css +++ b/webapp/src/styles.css @@ -1039,6 +1039,31 @@ main { font-size: var(--text-xl); } +.artist-about, +.artist-similar { + display: grid; + gap: var(--space-3); + margin-bottom: var(--space-8); +} + +.artist-about h3, +.artist-similar h3 { + margin: 0; + font-size: var(--text-xl); +} + +/* Paragraphs arrive as line breaks in plain text, not as markup. */ +.artist-bio { + max-width: 70ch; + margin: 0; + white-space: pre-line; +} + +.artist-source { + margin: 0; + font-size: var(--text-sm); +} + .list, .queue-list, .resource-list {