musicbrainz: tell "no answer" apart from "nothing to say", and pay the plays forward

Two failures that looked like nothing at all.

**A failed MusicBrainz page was indistinguishable from an empty one.**
`musicbrainz_first_release_dates` `break`s out of its paging loop when a
request fails and returns the map it has, so one transient blip produced an
empty map — exactly what an artist with no release groups produces. Everything
downstream then no-ops in silence: the album dates keep Deezer's reissue
values, every popular row keeps the pressing Deezer licensed, and the traces
read "0/47 corrected" and "0/10 re-attributed", which is also what a page looks
like when it was already perfect.

That is not hypothetical. It cost an hour last session chasing a regression in
Eagles' dates that turned out to be this machine having throttled its own IP
with probes, and it produced a false bug report that outlived it: My Bloody
Valentine was written up as "the supplement is failing to inject Loveless" when
the supplement was fine and MusicBrainz had simply not replied that once —
verified today, both Loveless and Isn't Anything inject correctly and always
did.

The third state is cheap to know, because a `None` here can only ever be a
failure: the mbid was resolved FROM MusicBrainz moments earlier, so the artist
provably exists there, and an artist it holds nothing for answers with an empty
`release-groups` ARRAY rather than with nothing. So the first page is now
retried twice behind a widening backoff — the failure this guards against is
overwhelmingly transient, and the pass is deferred, so nothing on screen is
waiting on it — and only then is the verdict recorded and carried to the
callers. Fault-injected against an unroutable host, the traces now read:

    musicbrainz.first_release_dates  4430ms  UNAVAILABLE
    page.original_dates    0/47 corrected — MUSICBRAINZ UNAVAILABLE, dates are unverified
    page.attribution       0/10 re-attributed — MUSICBRAINZ UNAVAILABLE, rows keep Deezer's attribution

A later page failing stays partial rather than unavailable: the dates already
collected are real, and the discography is date-sorted so the tail matters
least.

**Supplemented albums had no play counts, for a stale reason.** The comment
said a MusicBrainz or Apple album "simply carries no play counts" because the
scrobble pass resolved its anchor from a DEEZER artist id, which a record
Deezer never carried does not have. But the scrobble table is keyed on an
artist MBID, and a MusicBrainz-supplemented album was fetched BY that mbid — it
was in hand at registration and simply not kept. So the records the supplement
exists to rescue opened with no Plays column at all: Loveless, Isn't Anything,
Exmilitary.

Apple needs the same thing by the other route. It has no MusicBrainz id of its
own, but it does know which Deezer artist it was supplementing, so keeping that
id lets it reach the table the ordinary way. Weezer's "Hurley (Deluxe Edition)"
is an Apple row and had no Plays column while the plain Deezer edition beside
it had one.

Measured live: Loveless 79M album plays / 1.8M listeners with Only Shallow at
10,801,602; Hurley (Deluxe Edition) 1.9M / 93K with Memories at 1,219,036;
Exmilitary showing Guillotine at 3,363,409. 75 Rust tests and 85 JS tests pass.
This commit is contained in:
KeelrLeek committed 2026-07-28 11:02:29 -05:00
1 parent 644a0192a5
commit ddf2840b78
1 file changed
+159 -18
+159 -18
View File
@@ -355,6 +355,18 @@ const POPULAR_KEEP: usize = 10;
/// mandatory 1 req/s gate.
const MB_RELEASE_GROUP_PAGE: usize = 100;
const MB_RELEASE_GROUP_PAGES: usize = 3;
/// How many times the FIRST release-group page is retried before the supplement
/// declares MusicBrainz unavailable.
///
/// The failures this exists for are transient by nature — a 503 out of the
/// 1 req/s limiter, or this machine having throttled itself with probes — and
/// the cost of giving up is the whole date + attribution pass for that artist.
/// Two retries behind a widening backoff is enough to ride out a blip without
/// turning a genuine outage into a slow page: the request rides the same
/// 1 req/s gate as everything else, and the identity pass is deferred, so
/// nothing on screen is waiting on it.
const MB_RETRY_ATTEMPTS: u32 = 2;
const MB_RETRY_BACKOFF: Duration = Duration::from_millis(1200);
/// A resolved artist banner rarely changes and costs two external calls, so it
/// is cached far longer than ordinary catalog pages.
@@ -534,6 +546,20 @@ struct ArtistSupplements {
listeners: HashMap<String, TrackStats>,
/// MusicBrainz first-release facts, keyed by [`release_date_key`].
first_released: HashMap<String, ReleaseFacts>,
/// MusicBrainz never answered, as distinct from having nothing to say.
///
/// The two used to be the same value — a failed page `break`s and leaves an
/// empty map, which reads exactly like "this artist has no release groups".
/// One transient blip therefore cost a page its true album dates AND its
/// original-album attribution, silently, with nothing in the UI or the
/// traces to say so. It also produces false bug reports: My Bloody
/// Valentine was recorded as "the supplement is failing to inject Loveless"
/// when the supplement was fine and MusicBrainz had simply not replied that
/// once.
///
/// Callers must not treat this as evidence of anything. See
/// [`MetadataClient::musicbrainz_first_release_dates`].
musicbrainz_unavailable: bool,
}
#[derive(Debug, Clone, Default, serde::Serialize)]
@@ -776,6 +802,16 @@ const HERO_SCORE_RECIPE: &str = "bing-identity-visual-v1";
#[derive(Debug, Clone)]
struct MbCatalogAlbum {
release_group_id: String,
/// The ARTIST's MusicBrainz id, not the release group's.
///
/// Kept because the Last.fm scrobble table is anchored on exactly this, and
/// a supplemented album used to have no way back to it: the play-count pass
/// resolved the mbid from a DEEZER artist id, which a record Deezer never
/// carried does not have. So the records the supplement exists to rescue —
/// Loveless, Isn't Anything, Exmilitary — opened with no Plays column at
/// all. The id was in hand when the group was registered; it just was not
/// kept.
artist_mbid: String,
artist: String,
title: String,
release_date: Option<String>,
@@ -916,6 +952,14 @@ struct DeezerReleaseEvidence {
#[derive(Debug, Clone)]
struct AppleCatalogAlbum {
collection_id: u64,
/// The DEEZER artist id this supplement was fetched for.
///
/// Same reason [`MbCatalogAlbum::artist_mbid`] exists: the play-count pass
/// needs a way back to the artist, and an Apple-supplemented row has no
/// Deezer ALBUM to read one from. Weezer's "Hurley (Deluxe Edition)" is an
/// Apple row, and it opened with no Plays column while the plain Deezer
/// edition beside it had one.
artist_deezer_id: u64,
artist: String,
title: String,
cover: Option<String>,
@@ -4135,7 +4179,20 @@ impl MetadataClient {
stage_trace(
"page.original_dates",
dates_started,
&format!("{corrected}/{} corrected, {inherited} inherited", albums.len()),
&format!(
"{corrected}/{} corrected, {inherited} inherited{}",
albums.len(),
// Without this the trace reads "0/47 corrected" whether
// MusicBrainz said the dates are already right or never
// answered at all — which is precisely the confusion that sent
// an hour chasing a regression in Eagles' dates that turned out
// to be this machine having throttled itself with probes.
if supplements.musicbrainz_unavailable {
" — MUSICBRAINZ UNAVAILABLE, dates are unverified"
} else {
""
}
),
);
artist.album_count = Some(albums.len() as u32);
albums.sort_by(|a, b| b.release_date.cmp(&a.release_date));
@@ -4158,7 +4215,18 @@ impl MetadataClient {
stage_trace(
"page.attribution",
attribution_started,
&format!("{reattributed}/{} re-attributed", popular_tracks.len()),
&format!(
"{reattributed}/{} re-attributed{}",
popular_tracks.len(),
// Attribution reads the same facts, so it fails the same way:
// with no secondary types the rule is a no-op and every popular
// row keeps whatever pressing Deezer licensed.
if supplements.musicbrainz_unavailable {
" — MUSICBRAINZ UNAVAILABLE, rows keep Deezer's attribution"
} else {
""
}
),
);
stage_trace(
"page.total",
@@ -4948,7 +5016,7 @@ impl MetadataClient {
// "Lover The Secret Studio Sessions". Verified live: with the filter
// Queen's supplement is exactly its 14 studio albums.
let query = format!("arid:{mbid} AND primarytype:album AND -secondarytype:* AND status:official");
self.musicbrainz_release_groups(artist, &query, false).await
self.musicbrainz_release_groups(artist, mbid, &query, false).await
}
/// Release groups MusicBrainz types `Mixtape/Street`, for the same
@@ -4969,7 +5037,7 @@ impl MetadataClient {
let query = format!(
"arid:{mbid} AND primarytype:album AND secondarytype:\"Mixtape/Street\" AND status:official"
);
self.musicbrainz_release_groups(artist, &query, true).await
self.musicbrainz_release_groups(artist, mbid, &query, true).await
}
/// Shared body of the two supplements above: run a release-group search,
@@ -4978,6 +5046,7 @@ impl MetadataClient {
async fn musicbrainz_release_groups(
&self,
artist: &str,
artist_mbid: &str,
query: &str,
is_mixtape: bool,
) -> Vec<CatalogAlbum> {
@@ -5030,6 +5099,7 @@ impl MetadataClient {
id,
MbCatalogAlbum {
release_group_id: release_group_id.to_string(),
artist_mbid: artist_mbid.to_string(),
artist: artist.to_string(),
title: title.clone(),
release_date: release_date.clone(),
@@ -5270,13 +5340,21 @@ impl MetadataClient {
/// Album details with the full tracklist (and preview URLs).
pub async fn catalog_album(&self, album_id: u64) -> Option<CatalogAlbumPage> {
// Only the Deezer branch knows which artist this record belongs to, and
// the scrobble table is anchored on that id. A MusicBrainz/Apple
// supplement album simply carries no play counts.
// The scrobble table is anchored on an artist MBID. Two ways to reach
// one: a Deezer album resolves its artist id and looks the link up, and
// a MusicBrainz-supplemented album already knows it outright — it was
// fetched by that id. The second path is why Loveless, which Deezer
// never carried, now has play counts at all.
let mut deezer_artist_id: Option<u64> = None;
let mut artist_mbid: Option<String> = None;
let mut page = if let Some(album) = self.mb_catalog.lock().await.get(&album_id).cloned() {
artist_mbid = Some(album.artist_mbid.clone());
self.musicbrainz_album(album_id, album).await?
} else if let Some(album) = self.apple_catalog.lock().await.get(&album_id).cloned() {
// Apple has no MusicBrainz id of its own, but it does know whose
// page it was supplementing, so it reaches the scrobble table by
// the ordinary Deezer-id route.
deezer_artist_id = Some(album.artist_deezer_id);
self.apple_album(album_id, album).await?
} else {
let body = self
@@ -5337,7 +5415,8 @@ impl MetadataClient {
if is_future_or_unresolved_date(page.album.release_date.as_deref()) {
self.mark_unreleased_tracks(&mut page).await;
}
self.apply_track_stats(&mut page, deezer_artist_id).await;
self.apply_track_stats(&mut page, deezer_artist_id, artist_mbid)
.await;
Some(page)
}
@@ -5348,12 +5427,26 @@ impl MetadataClient {
/// [`LASTFM_TRACK_LIMIT`] it reaches an album's deep cuts — measured 56 of
/// 57 tracks across five 150k-390k-track catalogues. A track Last.fm has
/// never seen keeps `None` and renders as a dash: unknown is not zero.
async fn apply_track_stats(&self, page: &mut CatalogAlbumPage, deezer_artist_id: Option<u64>) {
let Some(artist_id) = deezer_artist_id else {
return;
};
let Some(mbid) = self.deezer_linked_mbid(artist_id).await else {
return;
async fn apply_track_stats(
&self,
page: &mut CatalogAlbumPage,
deezer_artist_id: Option<u64>,
artist_mbid: Option<String>,
) {
// A supplemented album carries the MBID already; a Deezer one has to
// resolve it from the artist id. Either way the scrobble lookup below
// is identical, and both share one cached Last.fm body per artist.
let mbid = match artist_mbid {
Some(mbid) => mbid,
None => {
let Some(artist_id) = deezer_artist_id else {
return;
};
let Some(mbid) = self.deezer_linked_mbid(artist_id).await else {
return;
};
mbid
}
};
let Some(ranks) = self.lastfm_listener_ranks(&mbid).await else {
return;
@@ -5608,13 +5701,14 @@ impl MetadataClient {
let Some(mbid) = self.deezer_linked_mbid(artist_id).await else {
return ArtistSupplements::default();
};
let (listeners, first_released) = tokio::join!(
let (listeners, (first_released, musicbrainz_unavailable)) = tokio::join!(
self.lastfm_listener_ranks(&mbid),
self.musicbrainz_first_release_dates(&mbid),
);
ArtistSupplements {
listeners: listeners.unwrap_or_default(),
first_released,
musicbrainz_unavailable,
}
}
@@ -5627,7 +5721,28 @@ impl MetadataClient {
/// records and soundtracks are all fetched too — they are real releases
/// whose dates are just as wrong on Deezer, and excluding them here would
/// leave a compilation dated by its reissue.
async fn musicbrainz_first_release_dates(&self, mbid: &str) -> HashMap<String, ReleaseFacts> {
///
/// Returns `(facts, unavailable)`. The flag is the THIRD state, and it
/// exists because the first two are indistinguishable without it: an
/// artist MusicBrainz has nothing for answers with an empty
/// `release-groups` ARRAY, while a failed request answers with nothing at
/// all. Collapsing both into an empty map is what let one blip silently
/// strip a page of its dates and its attribution.
///
/// A `None` here is always a failure, never absence — the mbid was
/// resolved FROM MusicBrainz moments earlier (see
/// [`Self::deezer_linked_mbid`]), so the artist provably exists there.
///
/// The first page is retried before that verdict is reached, because the
/// failure this guards against is overwhelmingly transient: a 503 from the
/// 1 req/s limiter, or our own probes having throttled the IP. A later page
/// failing is reported as partial rather than unavailable — the dates
/// already collected are real, and the discography is date-sorted so the
/// tail matters least.
async fn musicbrainz_first_release_dates(
&self,
mbid: &str,
) -> (HashMap<String, ReleaseFacts>, bool) {
let started = Instant::now();
let mut dates: HashMap<String, ReleaseFacts> = HashMap::new();
for page in 0..MB_RELEASE_GROUP_PAGES {
@@ -5638,10 +5753,35 @@ impl MetadataClient {
let url = format!(
"https://musicbrainz.org/ws/2/release-group?artist={mbid}&limit=100&offset={offset}&fmt=json"
);
let Some(body) = self.musicbrainz_json(&url).await else {
let mut answer = self.musicbrainz_json(&url).await;
if answer.is_none() && page == 0 {
for attempt in 0..MB_RETRY_ATTEMPTS {
tokio::time::sleep(MB_RETRY_BACKOFF * (attempt + 1)).await;
net_trace(&url, &format!("mb-retry {}", attempt + 1));
answer = self.musicbrainz_json(&url).await;
if answer.is_some() {
break;
}
}
}
let Some(body) = answer else {
if page == 0 {
log::warn!(
"musicbrainz did not answer for {mbid}; \
album dates and original-album attribution are unavailable, not absent"
);
stage_trace("musicbrainz.first_release_dates", started, "UNAVAILABLE");
return (HashMap::new(), true);
}
break;
};
let Some(groups) = body.get("release-groups").and_then(|v| v.as_array()) else {
// A 200 whose shape we do not recognise is also not an answer
// about the artist. Same reasoning as above on the first page.
if page == 0 {
stage_trace("musicbrainz.first_release_dates", started, "MALFORMED");
return (HashMap::new(), true);
}
break;
};
let returned = groups.len();
@@ -5661,7 +5801,7 @@ impl MetadataClient {
started,
&format!("{} groups", dates.len()),
);
dates
(dates, false)
}
/// All-time Last.fm listener counts for one artist's tracks, keyed by
@@ -6426,6 +6566,7 @@ impl MetadataClient {
id,
AppleCatalogAlbum {
collection_id,
artist_deezer_id: deezer_id,
artist: artist.to_string(),
title: title.to_string(),
cover: cover.clone(),