From d80b78f69c9bcfc7fff7ec4390047bb962e4d351 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 16 Sep 2026 00:52:53 +0200 Subject: [PATCH 01/16] feat(radio, tags): rank mood radio by fit, and fetch album tags for review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues that share nothing but the shape of their answer: stop treating a range as a verdict, and score instead. Mood radio was a tempo window and nothing else. Inside it every track was as likely as any other, so Focus opened on 109 BPM as readily as on 85; Chill's window sat entirely inside Focus's, so two moods returned the same kind of list; and the home tile promised "tempo and energy" where energy was a loudness ceiling on two of the five moods. The window is now a gate and everything inside it is ranked — distance from the mood's tempo centre, loudness, and any genre word the mood names. Only tempo gates, because it is the only signal where being outside the range means the wrong mood: a 160 BPM track is not Sleep at any loudness. Loudness and genre rank, which is what lets a thin library still return forty tracks, closest fits first, rather than an error. That fixes the defect the issue opened on: an unmeasured loudness used to satisfy a ceiling exactly as well as a measured quiet track. It now scores 0.5 — below a track measured inside the mood, above one measured outside it. Tempo octave errors are rescued and discounted. An estimator that reads 170 as 85 fills Focus with drum'n'bass; the gate accepts any octave and a corrected reading is worth a third less than a plain one, because it is a guess about a measurement rather than a measurement. A tempo already inside the window is never reinterpreted. The windows are narrower: Party and Workout share four beats instead of fifteen. Focus and Chill still overlap, because the moods do — what separates them is the centre, the loudness and the genre. Tag fetching is the other half. Matching an album is easy; matching the tracks inside it is where this fails, so three weighted signals — title 0.60, duration 0.25, track number 0.15 — with missing data scoring 0.5 rather than 0, and a global greedy assignment that consumes each side once so a generic "Intro" cannot be handed to two files. Two thresholds rather than one: confident above 0.85 arrives pre-accepted, doubtful above 0.55 is shown and not applied for you, and below it there is no match at all. Titles are compared over the normaliser the metadata providers already share, which folds NFD combining marks — a transliteration table would not, and accented titles are not an edge case in a music library. Nothing in the fetch writes. What the review screen accepts goes through update_track_tags, one track at a time, and only the accepted fields: an omitted field is left alone, which is what makes "accept this one value" mean that and nothing more. Composer, track genre and disc number are not offered — Deezer cannot fill the first two, and its disc number is unreliable on box sets, which is where the local value is usually right. Closes #616. Closes #599. --- docs/features/integrations.md | 70 +++ docs/features/smart-playlists.md | 67 +++ src-tauri/crates/app/src/commands/mod.rs | 1 + .../crates/app/src/commands/mood_radio.rs | 446 +++++++++----- .../crates/app/src/commands/tag_fetch.rs | 371 ++++++++++++ src-tauri/crates/app/src/lib.rs | 2 + src-tauri/crates/core/src/lib.rs | 1 + .../crates/core/src/metadata/album_match.rs | 333 +++++++++++ src-tauri/crates/core/src/metadata/deezer.rs | 42 +- src-tauri/crates/core/src/metadata/mod.rs | 1 + src-tauri/crates/core/src/mood.rs | 392 ++++++++++++ src/components/common/TagFetchModal.tsx | 565 ++++++++++++++++++ src/components/views/AlbumDetailView.tsx | 22 + src/components/views/home/MoodRadioGrid.tsx | 23 +- src/i18n/locales/ar.json | 60 +- src/i18n/locales/de.json | 44 +- src/i18n/locales/en.json | 44 +- src/i18n/locales/es.json | 48 +- src/i18n/locales/fr.json | 48 +- src/i18n/locales/hi.json | 44 +- src/i18n/locales/id.json | 44 +- src/i18n/locales/it.json | 48 +- src/i18n/locales/ja.json | 44 +- src/i18n/locales/ko.json | 44 +- src/i18n/locales/nl.json | 44 +- src/i18n/locales/pt-BR.json | 48 +- src/i18n/locales/pt.json | 48 +- src/i18n/locales/ru.json | 52 +- src/i18n/locales/tr.json | 44 +- src/i18n/locales/zh-CN.json | 44 +- src/i18n/locales/zh-TW.json | 44 +- src/lib/tauri/moodRadio.ts | 24 +- src/lib/tauri/tagFetch.ts | 94 +++ 33 files changed, 3035 insertions(+), 211 deletions(-) create mode 100644 src-tauri/crates/app/src/commands/tag_fetch.rs create mode 100644 src-tauri/crates/core/src/metadata/album_match.rs create mode 100644 src-tauri/crates/core/src/mood.rs create mode 100644 src/components/common/TagFetchModal.tsx create mode 100644 src/lib/tauri/tagFetch.ts diff --git a/docs/features/integrations.md b/docs/features/integrations.md index 7fc17efa..a76fcd98 100644 --- a/docs/features/integrations.md +++ b/docs/features/integrations.md @@ -13,6 +13,7 @@ A single global toggle — Settings → Intégrations → "Mode hors-ligne" — - Artist pictures (`enrich_artist_deezer`, `batch_fetch_missing_artist_pictures`) - Album covers (`enrich_album_deezer`, `search_albums_deezer`, `set_album_artwork_from_deezer`, `batch_fetch_missing_album_covers`) - Label / fan-count metadata +- Fetching an album's tags for review (#599, below) **Deezer refuses with HTTP 200.** A rate-limited or rejected call answers `{"error":{"type":"Exception","message":"Quota limit exceeded","code":4}}` with a 200 status, so the response type has a `DeezerError::Api` arm that reads the object instead of letting the missing `data` field surface as a decode failure. Before that, a throttled client and an artist Deezer had never heard of produced the same log line and the same empty result — which is the fog #406 was diagnosed through; the reason now reaches the log the enrichment paths already write. A 429 from the edge in front of the API carries no JSON to read at all and gets its own `RateLimited` arm. @@ -237,3 +238,72 @@ The row UI shows each word as a chip — pink for captured, green-ringed for the - Plain / LRC / Enhanced LRC → `ItemKey::UnsyncLyrics` (USLT for ID3v2, UNSYNCEDLYRICS for Vorbis, `©lyr` for MP4) — unchanged. - TTML on Vorbis / MP4 / FLAC → `ItemKey::Lyrics` (the XML-friendly key). - TTML on MP3 — **skipped**. lofty has no clean ID3v2 mapping for arbitrary XML lyrics, so the file is left untouched, the DB cache still gets the TTML content, and `save_lyrics` returns `tag_write_skipped: true`. The editor surfaces this as a `lyrics.toast.tagWriteSkipped` warning so the user knows the file itself wasn't touched. + +## Fetching an album's tags (#599) + +"These tags are wrong, fetch them and let me approve the result" — +[`commands/tag_fetch.rs`](../../src-tauri/crates/app/src/commands/tag_fetch.rs) +with the matching in +[`waveflow_core::metadata::album_match`](../../src-tauri/crates/core/src/metadata/album_match.rs), +reviewed in +[`TagFetchModal`](../../src/components/common/TagFetchModal.tsx). + +Two steps, deliberately. `search_album_tag_sources` offers the +catalogue releases that might be this record and the user picks one, +because a title and an artist match several releases of the same album +— an original, a remaster, a deluxe edition with four more tracks — and +they carry different track lists. `fetch_album_tag_proposals` then +pairs the chosen release's tracks with the local files. + +### Matching the tracks + +Matching the album is the easy half. Inside it, three weighted signals: +**title 0.60, duration 0.25, track number 0.15**. Unequal on purpose — +the title carries most of the identity, the duration confirms it, and +the track number is corroboration from a field that is wrong often +enough to trust least. + +- **Missing data scores 0.5, not 0.** A track with no number is not + evidence *against* a match; scoring it zero would push every untagged + file below the threshold and make the feature useless on exactly the + libraries that need it. +- **Assignment is global and greedy**, each side consumed once. Asking + "what is the best remote track for this local one" lets a generic + title — "Intro", "Interlude" — win against several local files at + once and capture one that belonged to another. +- **Two thresholds**: confident above 0.85, doubtful above 0.55, + nothing below. The middle band is the point — it is what the review + screen exists to resolve, and it is why confident matches arrive + pre-accepted and doubtful ones do not. + +Titles are compared over +[`name_match::normalize_name`](../../src-tauri/crates/core/src/metadata/name_match.rs), +the normaliser the metadata providers already share: it folds NFD +combining marks, so a library tagged `Bjo\u{308}rk` matches a +catalogue's `Björk`. A transliteration table written for this feature +would not, and accented titles are not an edge case in a music library. + +### What is not offered + +**No composer and no track-level genre** — Deezer does not carry them, +and a review screen listing a field the source cannot fill invites +accepting a blank over something the user typed. **No disc number** +either: the API returns one, but it is unreliable on box sets, which is +precisely where the local value is usually right. + +### Nothing here writes + +No command in the module touches a file or a row. What the review +screen accepts is applied through `update_track_tags`, one track at a +time — the path that pauses playback before opening the file, writes +through the concrete tag so non-standard frames survive, re-hashes into +`track.file_hash` and relinks the album and artist rows. Writing across +a whole album is exactly where a second, simpler write path would turn +one bad moment into a folder in an unknown state, which is why #599 +waited on #598. + +Only the accepted fields are sent: `update_track_tags` leaves an +omitted field alone, which is what makes "accept this one value" mean +that and nothing more. A track whose write fails is counted and the +rest still run — stopping halfway through an album leaves a folder +nobody can describe. diff --git a/docs/features/smart-playlists.md b/docs/features/smart-playlists.md index 226fd1c9..ac2313e3 100644 --- a/docs/features/smart-playlists.md +++ b/docs/features/smart-playlists.md @@ -226,3 +226,70 @@ Wired into [`SmartPlaylistEditorModal`](../../src/components/common/SmartPlaylis [`describeRules`](../../src/lib/smartRuleSummary.ts) renders a rule tree as a sentence, shown by [`SmartRuleSummary`](../../src/components/common/SmartRuleSummary.tsx) under a custom smart playlist's title. Built from translated fragments — one key per predicate with its value interpolated, groups joined by a separator, nested groups parenthesised — so a translator only ever sees short phrases and the recursion stays out of the locale files. It reads the rules through `get_custom_smart_playlist_rules` rather than parsing `playlist.smart_rules`, which is already in the row: playlists created before the tree carry the v1 flat shape and **only the backend deserializer migrates it**, so reading the column here would render an empty sentence for exactly the oldest playlists. + +## Mood Radio + +Five presets, each a **tempo gate plus a shape** +([`commands/mood_radio.rs`](../../src-tauri/crates/app/src/commands/mood_radio.rs), +scoring in [`waveflow_core::mood`](../../src-tauri/crates/core/src/mood.rs)). +The gate decides what may be considered; everything inside it is ranked +by how well it fits, and the forty tracks that play are the best of the +pool rather than the first forty drawn out of it (#616). + +| Mood | Tempo gate | Centre | Loudness | +| --- | --- | --- | --- | +| Focus | 72–108 | 88 | prefers ≤ −14 LUFS | +| Chill | 65–95 | 78 | prefers ≤ −10 LUFS | +| Workout | 128–180 | 150 | prefers ≥ −12 LUFS | +| Party | 110–132 | 122 | prefers ≥ −12 LUFS | +| Sleep | ≤ 68 | 52 | prefers ≤ −18 LUFS | + +The pool is still drawn with `ORDER BY RANDOM() LIMIT 400`, which is +what keeps two runs of one mood from being the same queue; the ranking +then decides which of it plays. + +### Why only tempo gates + +A gate answers "is this the wrong kind of track", a score answers "how +right is it". Tempo is the only signal where falling outside the range +really does mean the wrong mood — a 160 BPM track is not Sleep at any +loudness. Loudness and genre rank instead, which is what lets a thin +library still return forty tracks, closest fits first, rather than an +error. + +That also fixes the case the issue opened on: **an unmeasured loudness +used to satisfy a ceiling exactly as well as a measured quiet track** +(`loudness_lufs IS NULL` passed the filter). It now scores 0.5 — below +a track measured inside the mood, above one measured outside it, which +is the only honest ordering and the same rule the tag matcher uses for +missing data. + +### Octave correction + +Tempo estimators land an octave out often enough that a 170 BPM track +is recorded as 85, and a library where that happened is a library where +Focus quietly fills with drum'n'bass. The candidate query accepts any +octave reading (`bpm`, `bpm × 2`, `bpm ÷ 2`) and the scorer discounts +the corrected one by a third: a guess about a measurement is worth less +than a measurement, so rescued tracks sit below the honest ones rather +than beside them. A tempo already inside the window is **never** +reinterpreted, however well its double would score. + +### Narrowed, not de-overlapped + +Chill's window used to sit entirely inside Focus's, and Party shared +fifteen beats with Workout — two different moods could return the same +kind of list. The windows above share four beats at most between Party +and Workout, and Focus and Chill still overlap because the moods +genuinely do: "calm enough to work to" and "calm enough to sit in" are +the same tempo, and what separates them is the centre, the loudness and +the genre words. + +### What the home tile says + +`mood_radio_counts` returns the per-mood counts **and** how much of the +library carries a tempo at all, because a thin radio has a reason the +counts cannot show: they look small without saying small *of what*. The +grid shows the coverage line only while the two numbers differ. The +subtitle now says what the radio does — it promised "tempo and energy" +while energy was a loudness ceiling on two of the five moods. diff --git a/src-tauri/crates/app/src/commands/mod.rs b/src-tauri/crates/app/src/commands/mod.rs index d2a7c3e6..3ce5f115 100644 --- a/src-tauri/crates/app/src/commands/mod.rs +++ b/src-tauri/crates/app/src/commands/mod.rs @@ -53,6 +53,7 @@ pub mod smart_playlists; pub mod spotify; pub mod stats; pub mod storage; +pub mod tag_fetch; pub mod tasks; pub mod track; pub mod track_tags; diff --git a/src-tauri/crates/app/src/commands/mood_radio.rs b/src-tauri/crates/app/src/commands/mood_radio.rs index 0f7e7845..894d6416 100644 --- a/src-tauri/crates/app/src/commands/mood_radio.rs +++ b/src-tauri/crates/app/src/commands/mood_radio.rs @@ -1,12 +1,20 @@ //! Mood-based radio — Spotify-style "moment of the day" queues. //! -//! Five presets, each mapped to a BPM range plus an optional LUFS -//! ceiling (focus/sleep want quieter tracks; workout/party are tempo- -//! only). The query joins `track` with `track_analysis` and filters -//! out tracks that have no analysis row at all — without BPM data we -//! can't honour the constraint, and a "mood" radio that randomly -//! sneaks in heavy metal between two ambient tracks would defeat the -//! whole point. +//! Five presets. Each one is a **tempo gate plus a shape**: the gate +//! decides what may be considered, and [`waveflow_core::mood`] ranks +//! everything inside it by how well it actually fits — distance from +//! the mood's tempo centre, loudness, and any genre word the mood +//! names. The forty tracks that play are the best of the pool, not the +//! first forty drawn out of it (#616). +//! +//! # Why only tempo gates +//! +//! A gate answers "is this the wrong kind of track"; a score answers +//! "how right is it". Tempo is the only one of the three signals where +//! falling outside the range really does mean the wrong mood — a +//! 160 BPM track is not Sleep at any loudness. Loudness and genre are +//! ranked instead, which is what lets a thin library still return +//! forty tracks, closest fits first, rather than an error. //! //! Returns the ordered `Vec` of track IDs. The frontend hands //! this to `player_play_tracks` with `source_type = "radio"` so @@ -18,6 +26,8 @@ use serde::Deserialize; use sqlx::SqlitePool; +use waveflow_core::mood::{MoodCandidate, MoodProfile}; + use crate::{ error::{AppError, AppResult}, state::AppState, @@ -32,9 +42,11 @@ const TARGET_LEN: usize = 40; /// has 100+ play_events on a single act. const PER_ARTIST_CAP: usize = 4; -/// Pool size before the per-artist cap + shuffle. Larger = more -/// variety in the candidate set, but also slower SQL — 400 hits the -/// sweet spot for libraries up to ~50k tracks. +/// Pool size before ranking. Drawn at random, which is what keeps two +/// runs of the same mood from being the same queue; the ranking then +/// decides which forty of it play. Larger = more variety in the +/// candidate set, but also slower SQL — 400 hits the sweet spot for +/// libraries up to ~50k tracks. const POOL_SIZE: i64 = 400; use waveflow_core::album_playback::{ALBUM_FIT, ALBUM_MIN_ANALYSED}; @@ -59,61 +71,117 @@ pub enum Mood { Sleep, } -struct MoodFilter { - /// Inclusive BPM range. `None` means unbounded on that side. - bpm_min: Option, - bpm_max: Option, - /// Inclusive LUFS ceiling — `Some(-14.0)` keeps tracks at or below - /// −14 LUFS (perceptually quieter, fits Focus / Sleep). LUFS is - /// negative; tracks with NULL loudness pass through (the constraint - /// only filters when we have data, so we don't punish unanalysed - /// quiet tracks). - lufs_max: Option, -} - impl Mood { - fn filter(&self) -> MoodFilter { + /// The five shapes. + /// + /// The tempo windows are narrower than they were: Chill's used to + /// sit **entirely inside** Focus's, and Party shared fifteen beats + /// with Workout, so two different moods could return the same kind + /// of list. They still overlap where the moods genuinely do — + /// Focus and Chill are both slow and calm, and no tempo separates + /// them — and there the centres, the loudness and the genre words + /// do the separating. + fn profile(&self) -> MoodProfile { match self { - // Calm, mid-tempo, quiet — picked by ear after a few work - // sessions. The −14 LUFS ceiling is roughly Spotify's - // normalisation target; tracks louder than that tend to - // pull attention away from whatever you're focusing on. - Mood::Focus => MoodFilter { - bpm_min: Some(60.0), - bpm_max: Some(110.0), + // Calm, mid-tempo, quiet. The −14 LUFS preference is + // roughly Spotify's normalisation target; louder than that + // tends to pull attention away from whatever you are + // focusing on. + Mood::Focus => MoodProfile { + bpm_min: Some(72.0), + bpm_max: Some(108.0), + bpm_centre: 88.0, lufs_max: Some(-14.0), + lufs_min: None, + genre_words: &[ + "ambient", + "classical", + "instrumental", + "piano", + "soundtrack", + "score", + "post-rock", + ], }, - // Lounge / chill territory — same tempo band as Focus but - // no loudness constraint so the radio can include warmer - // mixes (jazz, soul, downtempo). - Mood::Chill => MoodFilter { - bpm_min: Some(60.0), - bpm_max: Some(100.0), - lufs_max: None, + // Lounge / chill territory: slower than Focus on average, + // and separated from it mostly by genre — "calm enough to + // work to" and "calm enough to sit in" are the same tempo. + Mood::Chill => MoodProfile { + bpm_min: Some(65.0), + bpm_max: Some(95.0), + bpm_centre: 78.0, + lufs_max: Some(-10.0), + lufs_min: None, + genre_words: &[ + "chill", + "lounge", + "downtempo", + "soul", + "jazz", + "bossa", + "r&b", + "trip hop", + "folk", + ], }, - // Workout: keeps the cadence above ~125 (running, lifting). - // Upper bound at 180 to avoid drum'n'bass / speedcore that - // most users wouldn't want for a treadmill session. - Mood::Workout => MoodFilter { - bpm_min: Some(125.0), + // Workout: keeps the cadence above ~128 (running, + // lifting), and wants the loud end of the library rather + // than merely tolerating it. Upper bound at 180 to avoid + // the drum'n'bass and speedcore most users wouldn't want + // for a treadmill session. + Mood::Workout => MoodProfile { + bpm_min: Some(128.0), bpm_max: Some(180.0), + bpm_centre: 150.0, lufs_max: None, + lufs_min: Some(-12.0), + genre_words: &[ + "electronic", + "dance", + "techno", + "rock", + "metal", + "punk", + "hip hop", + "rap", + "drum", + ], }, - // Dance-pop tempo band, broad on purpose — house (~120), - // pop (~120-130), reggaeton (~95-100, deliberately - // excluded so a "Party" radio doesn't accidentally drop - // tempo mid-set). - Mood::Party => MoodFilter { + // Dance-pop tempo band, narrowed off Workout's: they now + // share four beats instead of fifteen. + Mood::Party => MoodProfile { bpm_min: Some(110.0), - bpm_max: Some(140.0), + bpm_max: Some(132.0), + bpm_centre: 122.0, lufs_max: None, + lufs_min: Some(-12.0), + genre_words: &[ + "dance", + "pop", + "house", + "disco", + "funk", + "reggaeton", + "electronic", + "latin", + ], }, - // Sleep: very slow, very quiet. The −18 LUFS floor catches - // almost anything that isn't ambient / piano / classical. - Mood::Sleep => MoodFilter { + // Sleep: very slow, very quiet. + Mood::Sleep => MoodProfile { bpm_min: None, - bpm_max: Some(75.0), + bpm_max: Some(68.0), + bpm_centre: 52.0, lufs_max: Some(-18.0), + lufs_min: None, + genre_words: &[ + "ambient", + "classical", + "piano", + "meditation", + "drone", + "new age", + "sleep", + ], }, } } @@ -125,7 +193,7 @@ pub async fn start_mood_radio( mood: Mood, ) -> AppResult> { let pool = state.require_profile_pool().await?; - let f = mood.filter(); + let profile = mood.profile(); if waveflow_core::album_playback::album_mode_enabled(&pool).await { // Album mode is a preference, not a contract: a library whose @@ -135,74 +203,105 @@ pub async fn start_mood_radio( // genuinely nothing, the track path says so in the words that // actually apply ("no tracks match this mood") rather than // blaming the albums. - let by_album = mood_radio_by_album(&pool, &f).await?; + let by_album = mood_radio_by_album(&pool, &profile).await?; if !by_album.is_empty() { return Ok(by_album); } tracing::info!("no record fits this mood; falling back to individual tracks"); } - // Pull the candidate pool. `bpm IS NOT NULL` is mandatory — we - // can't honour a tempo-based mood without it. LUFS is optional - // (NULL passes through when no ceiling is set, otherwise the - // ceiling acts as a soft "skip very loud tracks" filter). - let rows: Vec = sqlx::query_as::<_, TrackCandidate>( + let rows = candidate_pool(&pool, &profile, POOL_SIZE).await?; + if rows.is_empty() { + return Err(AppError::Other( + "no tracks match this mood — run BPM analysis on your library first".into(), + )); + } + + Ok(waveflow_core::mood::rank_and_cap( + &profile, + rows, + TARGET_LEN, + PER_ARTIST_CAP, + )) +} + +/// The tracks a mood may consider, drawn at random. +/// +/// `bpm IS NOT NULL` is mandatory — a tempo-based mood cannot be +/// honoured without it. The gate accepts **any octave reading** +/// (`bpm`, `bpm × 2`, `bpm ÷ 2`): estimators land an octave out often +/// enough that a 170 BPM track is recorded as 85, and without this the +/// correction in [`waveflow_core::mood`] would never see the tracks it +/// exists to rescue. The corrected reading is discounted when scoring, +/// so those tracks sit below the honest ones rather than beside them. +/// +/// Loudness and genre are **not** filtered here — they rank. See the +/// module header. +async fn candidate_pool( + pool: &SqlitePool, + profile: &MoodProfile, + limit: i64, +) -> AppResult> { + #[derive(sqlx::FromRow)] + struct Row { + track_id: i64, + primary_artist: Option, + bpm: f64, + loudness_lufs: Option, + genres: Option, + } + + let rows: Vec = sqlx::query_as::<_, Row>( r#" - SELECT t.id AS track_id, - t.primary_artist AS primary_artist, - COALESCE(ta.bpm, 0.0) AS bpm, - COALESCE(ta.loudness_lufs, 0.0) AS lufs + SELECT t.id AS track_id, + t.primary_artist AS primary_artist, + ta.bpm AS bpm, + ta.loudness_lufs AS loudness_lufs, + (SELECT GROUP_CONCAT(g.name, ' ') + FROM track_genre tg JOIN genre g ON g.id = tg.genre_id + WHERE tg.track_id = t.id) AS genres FROM track t JOIN track_analysis ta ON ta.track_id = t.id WHERE t.is_available = 1 AND ta.bpm IS NOT NULL - AND (?1 IS NULL OR ta.bpm >= ?1) - AND (?2 IS NULL OR ta.bpm <= ?2) - AND (?3 IS NULL OR ta.loudness_lufs IS NULL OR ta.loudness_lufs <= ?3) + AND ta.bpm > 0 + AND ( ((?1 IS NULL OR ta.bpm >= ?1) AND (?2 IS NULL OR ta.bpm <= ?2)) + OR ((?1 IS NULL OR ta.bpm * 2.0 >= ?1) AND (?2 IS NULL OR ta.bpm * 2.0 <= ?2)) + OR ((?1 IS NULL OR ta.bpm / 2.0 >= ?1) AND (?2 IS NULL OR ta.bpm / 2.0 <= ?2))) ORDER BY RANDOM() - LIMIT ?4 + LIMIT ?3 "#, ) - .bind(f.bpm_min) - .bind(f.bpm_max) - .bind(f.lufs_max) - .bind(POOL_SIZE) - .fetch_all(&*pool) + .bind(profile.bpm_min) + .bind(profile.bpm_max) + .bind(limit) + .fetch_all(pool) .await?; - if rows.is_empty() { - return Err(AppError::Other( - "no tracks match this mood — run BPM analysis on your library first".into(), - )); - } - - // Per-artist cap so a single heavy contributor doesn't dominate. - let mut per_artist: std::collections::HashMap = std::collections::HashMap::new(); - let mut out: Vec = Vec::with_capacity(TARGET_LEN); - for c in rows { - if out.len() >= TARGET_LEN { - break; - } - let count = per_artist.entry(c.primary_artist).or_insert(0); - if *count >= PER_ARTIST_CAP { - continue; - } - *count += 1; - out.push(c.track_id); - } - - Ok(out) + Ok(rows + .into_iter() + .map(|r| MoodCandidate { + track_id: r.track_id, + primary_artist: r.primary_artist, + bpm: r.bpm, + loudness_lufs: r.loudness_lufs, + genres: r.genres, + }) + .collect()) } /// The same radio, built out of whole records (#618). /// /// Selection is per album rather than per track: a record qualifies /// when most of what we have measured of it sits inside the mood's -/// window, and it then plays in the order it was pressed. The -/// per-artist cap becomes a cap on records, for the same reason — +/// tempo window, and the records that qualify are then **ranked by the +/// fit of their average track** rather than drawn at random — the same +/// change as the track path, applied to the unit album mode plays in. +/// Each record then plays in the order it was pressed. The per-artist +/// cap becomes a cap on records, for the same reason as on tracks: /// without it a heavy listener's mood radio is one artist's /// discography. -async fn mood_radio_by_album(pool: &SqlitePool, f: &MoodFilter) -> AppResult> { +async fn mood_radio_by_album(pool: &SqlitePool, profile: &MoodProfile) -> AppResult> { #[derive(sqlx::FromRow)] struct Row { album_id: i64, @@ -215,33 +314,40 @@ async fn mood_radio_by_album(pool: &SqlitePool, f: &MoodFilter) -> AppResult, + /// The record judged as one track: the mood ranks it by the + /// average of what was measured on it. + avg_bpm: Option, + avg_lufs: Option, } let rows: Vec = sqlx::query_as::<_, Row>( r#" - SELECT t.album_id AS album_id, - COUNT(*) AS track_count, - MIN(t.primary_artist) AS primary_artist + SELECT t.album_id AS album_id, + COUNT(*) AS track_count, + MIN(t.primary_artist) AS primary_artist, + AVG(ta.bpm) AS avg_bpm, + AVG(ta.loudness_lufs) AS avg_lufs FROM track t LEFT JOIN track_analysis ta ON ta.track_id = t.id WHERE t.is_available = 1 AND t.album_id IS NOT NULL GROUP BY t.album_id - HAVING SUM(CASE WHEN ta.bpm IS NOT NULL THEN 1 ELSE 0 END) >= ?5 - AND SUM(CASE WHEN ta.bpm IS NOT NULL - AND (?1 IS NULL OR ta.bpm >= ?1) - AND (?2 IS NULL OR ta.bpm <= ?2) - AND (?3 IS NULL OR ta.loudness_lufs IS NULL - OR ta.loudness_lufs <= ?3) + HAVING SUM(CASE WHEN ta.bpm IS NOT NULL THEN 1 ELSE 0 END) >= ?4 + AND SUM(CASE WHEN ta.bpm IS NOT NULL AND ta.bpm > 0 + AND ( ((?1 IS NULL OR ta.bpm >= ?1) + AND (?2 IS NULL OR ta.bpm <= ?2)) + OR ((?1 IS NULL OR ta.bpm * 2.0 >= ?1) + AND (?2 IS NULL OR ta.bpm * 2.0 <= ?2)) + OR ((?1 IS NULL OR ta.bpm / 2.0 >= ?1) + AND (?2 IS NULL OR ta.bpm / 2.0 <= ?2))) THEN 1 ELSE 0 END) * 1.0 - / SUM(CASE WHEN ta.bpm IS NOT NULL THEN 1 ELSE 0 END) >= ?4 + / SUM(CASE WHEN ta.bpm IS NOT NULL THEN 1 ELSE 0 END) >= ?3 ORDER BY RANDOM() - LIMIT ?6 + LIMIT ?5 "#, ) - .bind(f.bpm_min) - .bind(f.bpm_max) - .bind(f.lufs_max) + .bind(profile.bpm_min) + .bind(profile.bpm_max) .bind(ALBUM_FIT) .bind(ALBUM_MIN_ANALYSED) .bind(ALBUM_POOL_SIZE) @@ -254,25 +360,41 @@ async fn mood_radio_by_album(pool: &SqlitePool, f: &MoodFilter) -> AppResult, usize> = - std::collections::HashMap::new(); - let candidates: Vec = rows + // Rank the records the way the track path ranks tracks, by reusing + // the same scorer over the record's averages. `album_id` stands in + // for the track id, so the tie-break stays total. The cap is a cap + // on records here, and `rank_and_cap` applies it to the ranking, + // so what it drops is that artist's worst-fitting record. + let ranked = waveflow_core::mood::rank_and_cap( + profile, + rows.iter() + .map(|r| MoodCandidate { + track_id: r.album_id, + primary_artist: r.primary_artist, + // A record only qualified because it has analysed + // tracks, so this is `None` in no reachable case; zero + // then scores as the worst possible tempo rather than + // taking the radio down. + bpm: r.avg_bpm.unwrap_or(0.0), + loudness_lufs: r.avg_lufs, + genres: None, + }) + .collect(), + rows.len(), + ALBUM_PER_ARTIST_CAP, + ); + + let track_counts: std::collections::HashMap = + rows.iter().map(|r| (r.album_id, r.track_count)).collect(); + let candidates: Vec = ranked .into_iter() - .filter(|row| { - // Records with no artist at all share one bucket rather - // than each counting as a different artist: that is the - // conservative reading, and it keeps a pile of untagged - // rips from filling the radio between them. - let count = per_artist.entry(row.primary_artist).or_insert(0); - if *count >= ALBUM_PER_ARTIST_CAP { - return false; - } - *count += 1; - true - }) - .map(|row| waveflow_core::album_playback::AlbumCandidate { - album_id: row.album_id, - track_count: row.track_count, + .filter_map(|album_id| { + track_counts.get(&album_id).map(|&track_count| { + waveflow_core::album_playback::AlbumCandidate { + album_id, + track_count, + } + }) }) .collect(); @@ -280,29 +402,27 @@ async fn mood_radio_by_album(pool: &SqlitePool, f: &MoodFilter) -> AppResult) -> AppResult { let pool = state.require_profile_pool().await?; + let (analysed_tracks, total_tracks) = analysis_coverage(&pool).await?; Ok(MoodCounts { - focus: count_for_mood(&pool, &Mood::Focus.filter()).await?, - chill: count_for_mood(&pool, &Mood::Chill.filter()).await?, - workout: count_for_mood(&pool, &Mood::Workout.filter()).await?, - party: count_for_mood(&pool, &Mood::Party.filter()).await?, - sleep: count_for_mood(&pool, &Mood::Sleep.filter()).await?, + focus: count_for_mood(&pool, &Mood::Focus.profile()).await?, + chill: count_for_mood(&pool, &Mood::Chill.profile()).await?, + workout: count_for_mood(&pool, &Mood::Workout.profile()).await?, + party: count_for_mood(&pool, &Mood::Party.profile()).await?, + sleep: count_for_mood(&pool, &Mood::Sleep.profile()).await?, + analysed_tracks, + total_tracks, }) } @@ -313,9 +433,33 @@ pub struct MoodCounts { pub workout: i64, pub party: i64, pub sleep: i64, + /// Tracks carrying a tempo measurement — the population every mood + /// draws from. + pub analysed_tracks: i64, + /// Playable tracks in the library, analysed or not. + pub total_tracks: i64, +} + +/// How much of the library has a tempo, and how big the library is. +async fn analysis_coverage(pool: &SqlitePool) -> AppResult<(i64, i64)> { + // `SUM` over an empty set is NULL, and an empty library is a real + // state (a fresh profile) — decoding that into `i64` would fail + // the command on exactly the install that has nothing to show. + let row: (Option, i64) = sqlx::query_as( + r#" + SELECT SUM(CASE WHEN ta.bpm IS NOT NULL THEN 1 ELSE 0 END), + COUNT(*) + FROM track t + LEFT JOIN track_analysis ta ON ta.track_id = t.id + WHERE t.is_available = 1 + "#, + ) + .fetch_one(pool) + .await?; + Ok((row.0.unwrap_or(0), row.1)) } -async fn count_for_mood(pool: &SqlitePool, f: &MoodFilter) -> AppResult { +async fn count_for_mood(pool: &SqlitePool, profile: &MoodProfile) -> AppResult { let n: i64 = sqlx::query_scalar( r#" SELECT COUNT(*) @@ -323,14 +467,14 @@ async fn count_for_mood(pool: &SqlitePool, f: &MoodFilter) -> AppResult { JOIN track_analysis ta ON ta.track_id = t.id WHERE t.is_available = 1 AND ta.bpm IS NOT NULL - AND (?1 IS NULL OR ta.bpm >= ?1) - AND (?2 IS NULL OR ta.bpm <= ?2) - AND (?3 IS NULL OR ta.loudness_lufs IS NULL OR ta.loudness_lufs <= ?3) + AND ta.bpm > 0 + AND ( ((?1 IS NULL OR ta.bpm >= ?1) AND (?2 IS NULL OR ta.bpm <= ?2)) + OR ((?1 IS NULL OR ta.bpm * 2.0 >= ?1) AND (?2 IS NULL OR ta.bpm * 2.0 <= ?2)) + OR ((?1 IS NULL OR ta.bpm / 2.0 >= ?1) AND (?2 IS NULL OR ta.bpm / 2.0 <= ?2))) "#, ) - .bind(f.bpm_min) - .bind(f.bpm_max) - .bind(f.lufs_max) + .bind(profile.bpm_min) + .bind(profile.bpm_max) .fetch_one(pool) .await?; Ok(n) diff --git a/src-tauri/crates/app/src/commands/tag_fetch.rs b/src-tauri/crates/app/src/commands/tag_fetch.rs new file mode 100644 index 00000000..ecb0c8b7 --- /dev/null +++ b/src-tauri/crates/app/src/commands/tag_fetch.rs @@ -0,0 +1,371 @@ +//! Fetching an album's tags from Deezer, for review (#599). +//! +//! Deezer enrichment already fills artwork and artist pages; this is +//! the other half — "these tags are wrong, fetch them and let me +//! approve the result". It is two steps, deliberately: +//! +//! 1. [`search_album_tag_sources`] offers the catalogue albums that +//! might be this record. Choosing is the user's, because a title +//! and an artist name match several releases of the same album — +//! an original, a remaster, a deluxe edition with four more tracks +//! — and they carry different track lists. +//! 2. [`fetch_album_tag_proposals`] pairs the chosen release's tracks +//! with the local files through +//! [`waveflow_core::metadata::album_match`] and hands back both +//! sides, field by field. +//! +//! # Nothing here writes +//! +//! No command in this module touches a file or a row. The proposals go +//! to the review screen, and what the user accepts is applied through +//! [`crate::commands::edit::update_track_tags`] — the path that pauses +//! playback before opening the file, writes through the concrete tag +//! so non-standard frames survive, re-hashes, and relinks the album and +//! artist rows. Writing across a whole album is exactly where a second, +//! simpler write path would turn one bad moment into a folder in an +//! unknown state. +//! +//! # What Deezer cannot fill +//! +//! No composer, and no genre at track level. Neither is offered: a +//! review screen that lists a field the source cannot fill invites the +//! user to accept a blank over something they typed themselves. +//! `disk_number` is returned by the API but is unreliable on box sets, +//! so it is not offered either — the field it would overwrite is +//! usually right where the catalogue's is wrong. + +use serde::Serialize; +use sqlx::SqlitePool; +use waveflow_core::metadata::{ + album_match::{self, Confidence, TrackSignals}, + deezer::DeezerClient, +}; + +use crate::{ + error::{AppError, AppResult}, + state::AppState, +}; + +/// How many catalogue releases to offer for one local album. +/// +/// Enough that a remaster and a deluxe edition both appear, few enough +/// that the choice stays a choice. +const MAX_SOURCES: usize = 8; + +/// One catalogue release that might be this record. +#[derive(Debug, Serialize)] +pub struct AlbumSource { + pub deezer_id: i64, + pub title: String, + pub artist: Option, + pub track_count: Option, + pub year: Option, + pub cover_url: Option, +} + +#[tauri::command] +pub async fn search_album_tag_sources( + state: tauri::State<'_, AppState>, + album_id: i64, +) -> AppResult> { + let pool = state.require_profile_pool().await?; + let (title, artist) = album_identity(&pool, album_id).await?; + + if crate::offline::is_offline() { + return Err(AppError::Other( + "offline mode is on — turn it off to fetch tags".into(), + )); + } + + let query = match artist.as_deref() { + Some(artist) => format!("{title} {artist}"), + None => title.clone(), + }; + let client = DeezerClient::new(); + let hits = client + .search_album(&query) + .await + .map_err(|e| AppError::Other(format!("Deezer album search failed: {e}")))?; + + Ok(hits + .into_iter() + .take(MAX_SOURCES) + .map(|hit| AlbumSource { + deezer_id: hit.id, + title: hit.title, + artist: hit.artist.map(|a| a.name), + track_count: hit.nb_tracks, + // `release_date` is `YYYY-MM-DD`; only the year is worth + // offering, and a malformed one is dropped rather than + // guessed at. + year: hit + .release_date + .as_deref() + .and_then(|d| d.get(0..4)) + .and_then(|y| y.parse::().ok()), + cover_url: hit.cover_medium.or(hit.cover_big), + }) + .collect()) +} + +/// The values a tag fetch can offer for one track. +/// +/// Every field is optional on both sides: the local file may not carry +/// it, and the catalogue may not either. The review screen compares +/// them field by field, so it needs the pair rather than a diff. +#[derive(Debug, Clone, Default, Serialize)] +pub struct TagValues { + pub title: Option, + pub artist: Option, + pub album: Option, + pub year: Option, + pub track_number: Option, +} + +/// One local file, with what the catalogue says about it. +#[derive(Debug, Serialize)] +pub struct TrackProposal { + pub track_id: i64, + /// Shown for a track the matcher could not pair, where the title + /// alone would not tell the user which file is meant. + pub file_name: String, + pub current: TagValues, + /// `None` when nothing in the release matched this file well + /// enough — a real answer, and the reason the screen must not + /// silently apply "the best available". + pub fetched: Option, + pub score: Option, + pub confidence: Option, +} + +#[derive(Debug, Serialize)] +pub struct AlbumProposals { + pub album_id: i64, + pub deezer_id: i64, + /// Tracks in the album's own order, matched or not. + pub tracks: Vec, + /// Catalogue tracks no local file claimed — a deluxe edition's + /// extras, or the songs a partial rip is missing. + pub unmatched_remote: Vec, +} + +#[tauri::command] +pub async fn fetch_album_tag_proposals( + state: tauri::State<'_, AppState>, + album_id: i64, + deezer_album_id: i64, +) -> AppResult { + let pool = state.require_profile_pool().await?; + + if crate::offline::is_offline() { + return Err(AppError::Other( + "offline mode is on — turn it off to fetch tags".into(), + )); + } + + let locals = local_tracks(&pool, album_id).await?; + if locals.is_empty() { + return Err(AppError::Other(format!("album {album_id} has no tracks"))); + } + + let client = DeezerClient::new(); + let album = client + .get_album(deezer_album_id) + .await + .map_err(|e| AppError::Other(format!("Deezer album fetch failed: {e}")))?; + let remote_tracks = client + .get_album_tracks(deezer_album_id) + .await + .map_err(|e| AppError::Other(format!("Deezer track listing failed: {e}")))?; + if remote_tracks.is_empty() { + return Err(AppError::Other( + "this release has no track listing on Deezer".into(), + )); + } + + let album_year = album + .release_date + .as_deref() + .and_then(|d| d.get(0..4)) + .and_then(|y| y.parse::().ok()); + let album_title = album.title.clone(); + + let local_signals: Vec = locals + .iter() + .map(|l| TrackSignals { + title: l.title.clone(), + duration_ms: Some(l.duration_ms), + track_number: l.track_number, + }) + .collect(); + let remote_signals: Vec = remote_tracks + .iter() + .map(|r| TrackSignals { + title: r.title.clone(), + duration_ms: r.duration_ms(), + track_number: r.track_position, + }) + .collect(); + + let assignments = album_match::assign(&local_signals, &remote_signals); + // Indexed by local position so the album's order is preserved + // whatever order the matcher settled the pairs in. + let mut by_local: std::collections::HashMap = + std::collections::HashMap::new(); + for a in &assignments { + by_local.insert(a.local, a); + } + let claimed: std::collections::HashSet = assignments.iter().map(|a| a.remote).collect(); + + let tracks = locals + .iter() + .enumerate() + .map(|(idx, local)| { + let matched = by_local.get(&idx); + TrackProposal { + track_id: local.track_id, + file_name: file_name_of(&local.file_path), + current: TagValues { + title: Some(local.title.clone()), + artist: local.artist.clone(), + album: local.album.clone(), + year: local.year, + track_number: local.track_number, + }, + fetched: matched.map(|a| { + let r = &remote_tracks[a.remote]; + TagValues { + title: Some(r.title.clone()), + // The track's own credit where the catalogue + // gives one: a compilation's tracks are not all + // by the album's artist, and taking the album's + // would rewrite every guest credit on it. + artist: r.artist.as_ref().map(|a| a.name.clone()), + album: Some(album_title.clone()), + year: album_year, + track_number: r.track_position, + } + }), + score: matched.map(|a| a.score), + confidence: matched.map(|a| a.confidence), + } + }) + .collect(); + + let unmatched_remote = remote_tracks + .iter() + .enumerate() + .filter(|(i, _)| !claimed.contains(i)) + .map(|(_, r)| r.title.clone()) + .collect(); + + Ok(AlbumProposals { + album_id, + deezer_id: deezer_album_id, + tracks, + unmatched_remote, + }) +} + +// ── Reads ─────────────────────────────────────────────────────────── + +/// The album's title and its artist's name, for the search query. +async fn album_identity(pool: &SqlitePool, album_id: i64) -> AppResult<(String, Option)> { + let row: Option<(String, Option)> = sqlx::query_as( + "SELECT al.title, ar.name + FROM album al + LEFT JOIN artist ar ON ar.id = al.artist_id + WHERE al.id = ?", + ) + .bind(album_id) + .fetch_optional(pool) + .await?; + row.ok_or_else(|| AppError::Other(format!("album {album_id} not found"))) +} + +struct LocalTrack { + track_id: i64, + file_path: String, + title: String, + duration_ms: i64, + track_number: Option, + year: Option, + artist: Option, + album: Option, +} + +/// The album's tracks, in the order the record plays. +/// +/// The artist credit is rebuilt from `track_artist` with `"; "`, the +/// library's one spelling for a multi-artist credit — reading +/// `track.primary_artist` instead would offer to replace a full credit +/// with its first name, which is a silent loss dressed as a fix. +async fn local_tracks(pool: &SqlitePool, album_id: i64) -> AppResult> { + #[derive(sqlx::FromRow)] + struct Row { + id: i64, + file_path: String, + title: String, + duration_ms: i64, + track_number: Option, + year: Option, + artists: Option, + album: Option, + } + + let rows: Vec = sqlx::query_as::<_, Row>( + r#" + SELECT t.id AS id, + t.file_path AS file_path, + t.title AS title, + t.duration_ms AS duration_ms, + t.track_number AS track_number, + t.year AS year, + (SELECT GROUP_CONCAT(ar.name, '; ') + FROM track_artist ta + JOIN artist ar ON ar.id = ta.artist_id + WHERE ta.track_id = t.id + ORDER BY ta.position) AS artists, + al.title AS album + FROM track t + LEFT JOIN album al ON al.id = t.album_id + WHERE t.album_id = ? + AND t.is_available = 1 + ORDER BY COALESCE(t.disc_number, 1), COALESCE(t.track_number, 9999), t.title + "#, + ) + .bind(album_id) + .fetch_all(pool) + .await?; + + Ok(rows + .into_iter() + .map(|r| LocalTrack { + track_id: r.id, + file_path: r.file_path, + title: r.title, + duration_ms: r.duration_ms, + track_number: r.track_number, + year: r.year, + artist: r.artists, + album: r.album, + }) + .collect()) +} + +/// The last path segment, whichever separator the scanning OS used. +fn file_name_of(path: &str) -> String { + path.rsplit(['/', '\\']).next().unwrap_or(path).to_string() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn file_name_handles_both_separators() { + assert_eq!(file_name_of(r"E:\Music\a\b.flac"), "b.flac"); + assert_eq!(file_name_of("/home/u/Music/a/b.flac"), "b.flac"); + assert_eq!(file_name_of("bare.flac"), "bare.flac"); + } +} diff --git a/src-tauri/crates/app/src/lib.rs b/src-tauri/crates/app/src/lib.rs index 8130c9e2..2c1a83cd 100644 --- a/src-tauri/crates/app/src/lib.rs +++ b/src-tauri/crates/app/src/lib.rs @@ -1088,6 +1088,8 @@ pub fn run() { commands::player::player_set_match_source_rate, commands::inventory::inventory_summary, commands::inventory::inventory_tracks, + commands::tag_fetch::search_album_tag_sources, + commands::tag_fetch::fetch_album_tag_proposals, commands::track_tags::list_track_tag_keys, commands::track_tags::list_track_tag_values, commands::tasks::list_tasks, diff --git a/src-tauri/crates/core/src/lib.rs b/src-tauri/crates/core/src/lib.rs index af30d1e2..122e81ad 100644 --- a/src-tauri/crates/core/src/lib.rs +++ b/src-tauri/crates/core/src/lib.rs @@ -18,6 +18,7 @@ pub mod domain; pub mod error; pub mod inventory; pub mod metadata; +pub mod mood; // `plugin` carries the wasmtime + Cranelift + WASI stack (~5 MiB // of native codegen). Gated so `waveflow-server` (which never // executes guest WASM in v1) can opt out and stay lean. The diff --git a/src-tauri/crates/core/src/metadata/album_match.rs b/src-tauri/crates/core/src/metadata/album_match.rs new file mode 100644 index 00000000..791a3b68 --- /dev/null +++ b/src-tauri/crates/core/src/metadata/album_match.rs @@ -0,0 +1,333 @@ +//! Matching a local album's tracks against a catalogue's (#599). +//! +//! Matching the *album* is the easy half — a title and an artist go to +//! a search endpoint and the first plausible hit is usually right. +//! Matching the tracks inside it is where this kind of feature fails, +//! and it fails in two specific ways that this module is shaped +//! against: +//! +//! 1. **Judging on one signal.** A title alone cannot separate the two +//! "Intro"s on a record; a duration alone cannot tell two three- +//! minute songs apart. Three weighted signals are used instead, and +//! the weights are deliberately unequal — the title carries most of +//! the identity, the duration confirms it, and the track number is +//! corroboration from a field that is wrong often enough to trust +//! least. +//! 2. **Choosing per track.** Asking "what is the best remote track +//! for this local one" lets a generic title win against several +//! local tracks at once and capture one that belonged to another. +//! The assignment here is global and greedy: every pair is scored, +//! the best pair wins, and both sides are consumed. +//! +//! Nothing here touches the network or the database, which is what +//! makes the rules above testable as arithmetic. + +use super::name_match::normalize_name; + +/// Weights of the three signals. They sum to 1. +const W_TITLE: f64 = 0.60; +const W_DURATION: f64 = 0.25; +const W_NUMBER: f64 = 0.15; + +/// What a signal scores when one side has no value for it. +/// +/// **Not zero.** A track with no number is not evidence *against* a +/// match; scoring it zero would push every untagged file below the +/// threshold and make the feature useless on exactly the libraries +/// that need it most — the badly tagged ones. +pub const UNKNOWN_SCORE: f64 = 0.5; + +/// Above this, the match is offered as settled. +pub const CONFIDENT: f64 = 0.85; +/// Above this but below [`CONFIDENT`], the match is offered for review. +/// Below it there is no match at all. +pub const DOUBTFUL: f64 = 0.55; + +/// How far apart two durations may be before the signal is worthless. +/// +/// Ten seconds: longer than the gap between a catalogue's rounding and +/// a file's real length, shorter than the gap between two different +/// songs of about the same size. +const DURATION_TOLERANCE_MS: f64 = 10_000.0; + +/// One side of a comparison — a local file or a catalogue entry. +#[derive(Debug, Clone, Default)] +pub struct TrackSignals { + pub title: String, + pub duration_ms: Option, + pub track_number: Option, +} + +/// How sure the assignment is. +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize)] +#[serde(rename_all = "snake_case")] +pub enum Confidence { + Confident, + Doubtful, +} + +/// One local track paired with one catalogue entry. +#[derive(Debug, Clone, PartialEq)] +pub struct Assignment { + /// Index into the local slice. + pub local: usize, + /// Index into the remote slice. + pub remote: usize, + pub score: f64, + pub confidence: Confidence, +} + +/// How well two tracks agree, in `0.0..=1.0`. +pub fn score(local: &TrackSignals, remote: &TrackSignals) -> f64 { + let title = title_similarity(&local.title, &remote.title); + let duration = duration_similarity(local.duration_ms, remote.duration_ms); + let number = number_similarity(local.track_number, remote.track_number); + (W_TITLE * title + W_DURATION * duration + W_NUMBER * number).clamp(0.0, 1.0) +} + +/// Pair local tracks with catalogue entries, best pair first. +/// +/// Each side is consumed once, so a catalogue entry cannot be handed to +/// two local files however well it scores against both — the second +/// file keeps looking, which is exactly what stops a generic title from +/// swallowing a record. +/// +/// Pairs scoring below [`DOUBTFUL`] are not returned: a local track +/// with no match is a real answer, and inventing one for it is how this +/// kind of feature corrupts a library. +pub fn assign(locals: &[TrackSignals], remotes: &[TrackSignals]) -> Vec { + let mut pairs: Vec<(f64, usize, usize)> = Vec::with_capacity(locals.len() * remotes.len()); + for (li, local) in locals.iter().enumerate() { + for (ri, remote) in remotes.iter().enumerate() { + let s = score(local, remote); + if s >= DOUBTFUL { + pairs.push((s, li, ri)); + } + } + } + // Descending score; the indices break ties so the result is a + // function of its inputs rather than of the sort's stability. + pairs.sort_by(|a, b| { + b.0.partial_cmp(&a.0) + .unwrap_or(std::cmp::Ordering::Equal) + .then_with(|| a.1.cmp(&b.1)) + .then_with(|| a.2.cmp(&b.2)) + }); + + let mut local_taken = vec![false; locals.len()]; + let mut remote_taken = vec![false; remotes.len()]; + let mut out = Vec::new(); + for (s, li, ri) in pairs { + if local_taken[li] || remote_taken[ri] { + continue; + } + local_taken[li] = true; + remote_taken[ri] = true; + out.push(Assignment { + local: li, + remote: ri, + score: s, + confidence: if s >= CONFIDENT { + Confidence::Confident + } else { + Confidence::Doubtful + }, + }); + } + // Back into the album's own order, which is how the review screen + // reads: a list that jumps around by score is a list nobody can + // check against the record in front of them. + out.sort_by_key(|a| a.local); + out +} + +/// Title agreement, over [`normalize_name`]. +/// +/// The normaliser is the shared one on purpose: it folds NFD combining +/// marks, so a library tagged `Bjo\u{308}rk` matches a catalogue's +/// `Björk`. A transliteration table written for this feature would not, +/// and accented titles are not an edge case in a music library. +pub fn title_similarity(a: &str, b: &str) -> f64 { + let a = normalize_name(a); + let b = normalize_name(b); + if a.is_empty() || b.is_empty() { + return UNKNOWN_SCORE; + } + if a == b { + return 1.0; + } + let distance = levenshtein(&a, &b) as f64; + let longest = a.chars().count().max(b.chars().count()) as f64; + (1.0 - distance / longest).clamp(0.0, 1.0) +} + +fn duration_similarity(a: Option, b: Option) -> f64 { + let (Some(a), Some(b)) = (a, b) else { + return UNKNOWN_SCORE; + }; + let gap = (a - b).abs() as f64; + (1.0 - gap / DURATION_TOLERANCE_MS).clamp(0.0, 1.0) +} + +/// A track number agrees or it does not — there is no near miss. Track +/// 4 is not "almost" track 5; it is a different song. +fn number_similarity(a: Option, b: Option) -> f64 { + match (a, b) { + (Some(a), Some(b)) if a == b => 1.0, + (Some(_), Some(_)) => 0.0, + _ => UNKNOWN_SCORE, + } +} + +/// Edit distance over characters, two rows at a time. +/// +/// Titles are short and an album is a few dozen of them, so the +/// quadratic cost is a few thousand character comparisons — far below +/// the network call that fetched the catalogue side. +fn levenshtein(a: &str, b: &str) -> usize { + let a: Vec = a.chars().collect(); + let b: Vec = b.chars().collect(); + if a.is_empty() { + return b.len(); + } + let mut prev: Vec = (0..=b.len()).collect(); + let mut cur = vec![0usize; b.len() + 1]; + for (i, ca) in a.iter().enumerate() { + cur[0] = i + 1; + for (j, cb) in b.iter().enumerate() { + let cost = usize::from(ca != cb); + cur[j + 1] = (prev[j + 1] + 1).min(cur[j] + 1).min(prev[j] + cost); + } + std::mem::swap(&mut prev, &mut cur); + } + prev[b.len()] +} + +#[cfg(test)] +mod tests { + use super::*; + + fn track(title: &str, duration_ms: Option, track_number: Option) -> TrackSignals { + TrackSignals { + title: title.to_string(), + duration_ms, + track_number, + } + } + + /// The accented-title case the shared normaliser exists for: a + /// decomposed local tag and a precomposed catalogue title are the + /// same song. + #[test] + fn a_decomposed_title_matches_its_precomposed_twin() { + assert_eq!(title_similarity("Bjo\u{308}rk", "Björk"), 1.0); + assert_eq!(title_similarity("Céline", "Celine"), 1.0); + } + + /// Missing data is neutral, not damning. Without this the feature + /// would be useless on the untagged libraries that need it. + #[test] + fn a_missing_track_number_does_not_sink_a_match() { + let local = track("Villanelle", Some(212_000), None); + let remote = track("Villanelle", Some(212_000), Some(3)); + let s = score(&local, &remote); + assert!( + s >= CONFIDENT, + "a title and duration that agree must be enough: {s}" + ); + } + + /// A number that disagrees is evidence, where a missing one is not. + #[test] + fn a_wrong_track_number_costs_more_than_a_missing_one() { + let local = track("Villanelle", Some(212_000), Some(9)); + let missing = track("Villanelle", Some(212_000), None); + let remote = track("Villanelle", Some(212_000), Some(3)); + assert!(score(&local, &remote) < score(&missing, &remote)); + } + + /// The failure this module is shaped against: two generic titles + /// where a per-track best match would hand the same remote entry to + /// both local files. + #[test] + fn a_generic_title_cannot_be_taken_twice() { + let locals = vec![ + track("Intro", Some(60_000), Some(1)), + track("Intro", Some(95_000), Some(7)), + ]; + let remotes = vec![ + track("Intro", Some(60_000), Some(1)), + track("Intro", Some(95_000), Some(7)), + ]; + let out = assign(&locals, &remotes); + assert_eq!(out.len(), 2); + assert_eq!(out[0].remote, 0); + assert_eq!(out[1].remote, 1, "the second file must get the other entry"); + } + + /// The duration is what separates them when the numbers are gone. + #[test] + fn duration_separates_identical_titles() { + let locals = vec![ + track("Interlude", Some(45_000), None), + track("Interlude", Some(180_000), None), + ]; + let remotes = vec![ + track("Interlude", Some(179_000), None), + track("Interlude", Some(46_000), None), + ]; + let out = assign(&locals, &remotes); + assert_eq!(out.len(), 2); + assert_eq!(out[0].remote, 1, "45 s goes with 46 s"); + assert_eq!(out[1].remote, 0, "180 s goes with 179 s"); + } + + /// A local track with nothing like it is left unmatched rather than + /// paired with the least bad option. + #[test] + fn a_track_with_no_counterpart_stays_unmatched() { + let locals = vec![ + track("Hidden Track", Some(30_000), Some(99)), + track("Villanelle", Some(212_000), Some(3)), + ]; + let remotes = vec![track("Villanelle", Some(212_000), Some(3))]; + let out = assign(&locals, &remotes); + assert_eq!(out.len(), 1); + assert_eq!(out[0].local, 1); + } + + /// The middle band is the reason the review screen exists: a match + /// good enough to offer, not good enough to apply unseen. + #[test] + fn a_near_miss_is_doubtful_rather_than_absent() { + let local = track("Villanelle (Remastered)", Some(212_000), Some(3)); + let remote = track("Villanelle", Some(214_000), Some(3)); + let out = assign(&[local], &[remote]); + assert_eq!(out.len(), 1); + assert_eq!(out[0].confidence, Confidence::Doubtful); + } + + /// Results come back in the album's order, not in score order. + #[test] + fn assignments_are_returned_in_local_order() { + let locals = vec![ + track("Bad Match", Some(100_000), Some(1)), + track("Exact", Some(200_000), Some(2)), + ]; + let remotes = vec![ + track("Exact", Some(200_000), Some(2)), + track("Bad Match Indeed", Some(101_000), Some(1)), + ]; + let out = assign(&locals, &remotes); + assert_eq!(out.len(), 2); + assert!(out[0].local < out[1].local); + } + + #[test] + fn levenshtein_counts_edits() { + assert_eq!(levenshtein("kitten", "sitting"), 3); + assert_eq!(levenshtein("", "abc"), 3); + assert_eq!(levenshtein("abc", ""), 3); + assert_eq!(levenshtein("same", "same"), 0); + } +} diff --git a/src-tauri/crates/core/src/metadata/deezer.rs b/src-tauri/crates/core/src/metadata/deezer.rs index 6c87eabb..44afa453 100644 --- a/src-tauri/crates/core/src/metadata/deezer.rs +++ b/src-tauri/crates/core/src/metadata/deezer.rs @@ -186,7 +186,7 @@ pub struct DeezerAlbumHit { pub artist: Option, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Clone, Deserialize)] pub struct DeezerAlbumArtist { pub name: String, } @@ -215,6 +215,32 @@ pub struct DeezerTrackAlbum { pub cover_xl: Option, } +/// One track of an album listing. +/// +/// `duration` is in **seconds** — the only place in this file where a +/// duration is not milliseconds, and the matcher compares against +/// `track.duration_ms`, so the conversion has to happen at the border +/// rather than being noticed later as "every duration disagrees". +#[derive(Debug, Clone, Deserialize)] +pub struct DeezerAlbumTrack { + pub id: i64, + pub title: String, + pub duration: Option, + /// Position on its disc, which is what a track number means on a + /// multi-disc release. + pub track_position: Option, + pub disk_number: Option, + pub artist: Option, +} + +impl DeezerAlbumTrack { + /// The track's length in milliseconds, or `None` when the + /// catalogue did not give one. + pub fn duration_ms(&self) -> Option { + self.duration.map(|s| s * 1000) + } +} + // ── Client implementation ─────────────────────────────────────────── impl Default for DeezerClient { @@ -299,6 +325,20 @@ impl DeezerClient { Self::fetch(self.http.get(format!("{BASE_URL}/album/{deezer_id}"))).await } + /// The tracks of an album, in the catalogue's own order. + /// + /// `/album/{id}/tracks` rather than the `tracks` field of + /// `/album/{id}`: the embedded list is capped, and a box set read + /// through it comes back short with nothing to say it was cut. + pub async fn get_album_tracks(&self, deezer_id: i64) -> DeezerResult> { + let resp: DeezerSearchResponse = Self::fetch( + self.http + .get(format!("{BASE_URL}/album/{deezer_id}/tracks")), + ) + .await?; + Ok(resp.data) + } + /// Fetch artists Deezer reports as related to the given artist. /// Used as a fallback when Last.fm has no API key or returned no /// similar artists. Deezer's `/artist/{id}/related` returns a fixed diff --git a/src-tauri/crates/core/src/metadata/mod.rs b/src-tauri/crates/core/src/metadata/mod.rs index f79faa83..6d56cba9 100644 --- a/src-tauri/crates/core/src/metadata/mod.rs +++ b/src-tauri/crates/core/src/metadata/mod.rs @@ -6,6 +6,7 @@ //! both the desktop app (`crates/app`) and the future //! `waveflow-server` (RFC-001 §6.2) without any glue. +pub mod album_match; pub mod deezer; pub mod lastfm; pub mod lrclib; diff --git a/src-tauri/crates/core/src/mood.rs b/src-tauri/crates/core/src/mood.rs new file mode 100644 index 00000000..f7c997c0 --- /dev/null +++ b/src-tauri/crates/core/src/mood.rs @@ -0,0 +1,392 @@ +//! What a mood is, and how well a track fits one (#616). +//! +//! Mood Radio used to be a tempo window and nothing else: a track +//! inside the window was drawn as readily as any other, so Focus was +//! as likely to open on 109 BPM as on 85, and the home tile's promise +//! of "tempo and energy" was carried by a loudness ceiling on two of +//! the five moods. Two moods could also return the same kind of list, +//! because Chill's window sat entirely inside Focus's. +//! +//! The fix is to stop treating the window as the answer. It stays as a +//! gate — a track far outside a mood's tempo is not that mood — and +//! everything inside it is then **ranked by how well it fits**, so the +//! forty tracks that play are the forty best of the pool rather than +//! the first forty drawn. +//! +//! # Why the scoring lives here +//! +//! It is arithmetic over four numbers, it decides what the user hears, +//! and none of it needs a database. Keeping it in `waveflow-core` is +//! what lets it be tested against values rather than against a +//! library — the tempo octave rule below is impossible to be sure of +//! any other way. + +/// A mood, as a shape rather than a window. +/// +/// `bpm_centre` is what the mood *is*; `bpm_min` / `bpm_max` are only +/// how far from it a track may sit and still be considered. The +/// loudness bounds work the same way: they gate, and the distance from +/// the preferred side ranks. +#[derive(Debug, Clone, Copy)] +pub struct MoodProfile { + /// Inclusive tempo gate. `None` is unbounded on that side. + pub bpm_min: Option, + pub bpm_max: Option, + /// The tempo this mood is built around — the peak of the ranking. + pub bpm_centre: f64, + /// Loudness ceiling in LUFS (a negative number): quieter than this. + pub lufs_max: Option, + /// Loudness floor in LUFS: louder than this. + pub lufs_min: Option, + /// Genre words that suit the mood, matched case-insensitively as + /// substrings of the track's genres. A **bonus only** — never a + /// gate. Genre strings come from whatever tagger wrote the file, + /// so absence proves nothing and a miss must cost nothing. + pub genre_words: &'static [&'static str], +} + +/// How much a tempo read at half or double speed is discounted. +/// +/// Tempo estimators land an octave out often enough that a 170 BPM +/// track can be recorded as 85 — and a library where that happened is +/// a library where Focus quietly fills with drum'n'bass. Both +/// readings are therefore considered, and the corrected one is worth +/// less: it is a guess about a measurement, where the plain reading is +/// only a measurement. +pub const OCTAVE_PENALTY: f64 = 0.65; + +/// What an unmeasured value scores. +/// +/// Neither the reward of a match nor the cost of a miss: a track whose +/// loudness nobody has measured is not evidence that it is loud. It +/// ranks below a track measured inside the mood and above one measured +/// outside it, which is the only honest ordering — and it is what +/// keeps an unanalysed library from returning an empty radio. +pub const UNKNOWN_SCORE: f64 = 0.5; + +/// Weights of the three signals. Tempo carries the mood; loudness +/// confirms it; genre is corroboration from a field nobody validates. +const W_TEMPO: f64 = 0.6; +const W_LOUDNESS: f64 = 0.3; +const W_GENRE: f64 = 0.1; + +/// One track, as the ranking sees it. +#[derive(Debug, Clone)] +pub struct MoodCandidate { + pub track_id: i64, + /// `track.primary_artist` is nullable, and the per-artist cap has + /// to bucket the tracks that lost theirs together rather than + /// treating each as its own artist. + pub primary_artist: Option, + pub bpm: f64, + pub loudness_lufs: Option, + /// The track's genres, already lower-cased and joined — the shape + /// the query hands back. + pub genres: Option, +} + +/// The tempo reading to judge a track by, and what that reading costs. +/// +/// Returns the interpretation that best suits the profile: the plain +/// reading, or half or double it when the plain one falls outside the +/// gate and the corrected one does not. The second element is the +/// factor the tempo score is multiplied by. +pub fn effective_bpm(profile: &MoodProfile, bpm: f64) -> (f64, f64) { + if bpm <= 0.0 { + return (bpm, 1.0); + } + if in_window(profile, bpm) { + return (bpm, 1.0); + } + // Only when the plain reading is out of the window: a track that + // already fits is never reinterpreted, however well the double of + // it would score. Octave correction exists to rescue a + // misestimated track, not to move a correct one. + for candidate in [bpm * 2.0, bpm / 2.0] { + if in_window(profile, candidate) { + return (candidate, OCTAVE_PENALTY); + } + } + (bpm, 1.0) +} + +/// Does this tempo pass the mood's gate? +pub fn in_window(profile: &MoodProfile, bpm: f64) -> bool { + // `map_or(true, …)` rather than `is_none_or`, which is stable only + // since 1.82 — the workspace MSRV is 1.80, and the lint that would + // have caught it only fires on code the build actually compiles. + profile.bpm_min.map_or(true, |min| bpm >= min) && profile.bpm_max.map_or(true, |max| bpm <= max) +} + +/// Does this tempo pass the gate under any reading — plain, doubled or +/// halved? This is what the candidate query has to ask, or the octave +/// correction below would never see the tracks it exists to rescue. +pub fn in_window_any_octave(profile: &MoodProfile, bpm: f64) -> bool { + bpm > 0.0 + && (in_window(profile, bpm) + || in_window(profile, bpm * 2.0) + || in_window(profile, bpm / 2.0)) +} + +/// How well a track fits a mood, in `0.0..=1.0`. +pub fn fit_score(profile: &MoodProfile, candidate: &MoodCandidate) -> f64 { + let (bpm, octave_factor) = effective_bpm(profile, candidate.bpm); + let tempo = tempo_score(profile, bpm) * octave_factor; + let loudness = loudness_score(profile, candidate.loudness_lufs); + let genre = genre_score(profile, candidate.genres.as_deref()); + (W_TEMPO * tempo + W_LOUDNESS * loudness + W_GENRE * genre).clamp(0.0, 1.0) +} + +/// Distance from the mood's centre, normalised by the half-width of +/// its window. A track at the centre scores 1, one at the edge scores +/// close to 0, and the fall is linear because nothing about a tempo +/// preference justifies a sharper curve. +fn tempo_score(profile: &MoodProfile, bpm: f64) -> f64 { + let reach = half_width(profile); + if reach <= 0.0 { + return 1.0; + } + (1.0 - (bpm - profile.bpm_centre).abs() / reach).clamp(0.0, 1.0) +} + +/// How far the centre sits from the furthest edge of the window. An +/// unbounded side is measured from the bounded one, so an open-ended +/// mood (Sleep has no floor) still has a scale to divide by. +fn half_width(profile: &MoodProfile) -> f64 { + let below = profile.bpm_min.map(|min| profile.bpm_centre - min); + let above = profile.bpm_max.map(|max| max - profile.bpm_centre); + match (below, above) { + (Some(a), Some(b)) => a.max(b), + (Some(a), None) => a, + (None, Some(b)) => b, + (None, None) => 0.0, + } +} + +/// Where a measured loudness sits relative to the mood's bounds. +/// +/// A track inside both bounds scores 1; outside one of them the score +/// falls with the overshoot, over a fixed six-decibel run — beyond +/// that the track is simply the wrong loudness for the mood and scores +/// 0. Unmeasured loudness scores [`UNKNOWN_SCORE`]. +fn loudness_score(profile: &MoodProfile, lufs: Option) -> f64 { + let Some(lufs) = lufs else { + return UNKNOWN_SCORE; + }; + /// Decibels of overshoot that take the score from 1 to 0. + const RUN: f64 = 6.0; + let mut overshoot: f64 = 0.0; + if let Some(max) = profile.lufs_max { + overshoot = overshoot.max(lufs - max); + } + if let Some(min) = profile.lufs_min { + overshoot = overshoot.max(min - lufs); + } + (1.0 - overshoot.max(0.0) / RUN).clamp(0.0, 1.0) +} + +/// Whether any of the mood's words appear in the track's genres. +/// +/// A miss scores [`UNKNOWN_SCORE`] rather than 0, for the same reason +/// as an unmeasured loudness: most libraries carry genres nobody +/// curated, and a mood that punished every unrecognised genre would +/// rank a correctly-tagged library worse than an untagged one. +fn genre_score(profile: &MoodProfile, genres: Option<&str>) -> f64 { + if profile.genre_words.is_empty() { + return UNKNOWN_SCORE; + } + let Some(genres) = genres.filter(|g| !g.trim().is_empty()) else { + return UNKNOWN_SCORE; + }; + let haystack = genres.to_lowercase(); + if profile.genre_words.iter().any(|w| haystack.contains(w)) { + 1.0 + } else { + UNKNOWN_SCORE + } +} + +/// Rank a pool and take the best of it, one artist at a time. +/// +/// The pool arrives in random order — that is what keeps two runs of +/// the same mood from being the same list — and this picks the best +/// fits out of it. The per-artist cap is applied while walking the +/// ranking, so the tracks it skips are that artist's *weakest*, not +/// whichever the draw happened to reach first. +pub fn rank_and_cap( + profile: &MoodProfile, + candidates: Vec, + target_len: usize, + per_artist_cap: usize, +) -> Vec { + let mut scored: Vec<(f64, MoodCandidate)> = candidates + .into_iter() + .map(|c| (fit_score(profile, &c), c)) + .collect(); + // Descending fit, with the track id as a tie-break so the order is + // total: an unstable comparison over equal scores would make the + // same pool produce different queues, which is untestable and, for + // a user re-running a mood, inexplicable. + scored.sort_by(|a, b| { + b.0.partial_cmp(&a.0) + .unwrap_or(std::cmp::Ordering::Equal) + .then_with(|| a.1.track_id.cmp(&b.1.track_id)) + }); + + let mut per_artist: std::collections::HashMap, usize> = + std::collections::HashMap::new(); + let mut out = Vec::with_capacity(target_len); + for (_, c) in scored { + if out.len() >= target_len { + break; + } + let count = per_artist.entry(c.primary_artist).or_insert(0); + if *count >= per_artist_cap { + continue; + } + *count += 1; + out.push(c.track_id); + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + const FOCUS: MoodProfile = MoodProfile { + bpm_min: Some(70.0), + bpm_max: Some(105.0), + bpm_centre: 85.0, + lufs_max: Some(-14.0), + lufs_min: None, + genre_words: &["ambient", "classical"], + }; + + fn candidate(track_id: i64, bpm: f64, lufs: Option) -> MoodCandidate { + MoodCandidate { + track_id, + primary_artist: Some(1), + bpm, + loudness_lufs: lufs, + genres: None, + } + } + + /// The whole point of the change: inside one window, the track + /// nearer the mood's centre wins. Before this, both were equally + /// likely. + #[test] + fn a_track_nearer_the_centre_scores_higher() { + let near = fit_score(&FOCUS, &candidate(1, 85.0, Some(-16.0))); + let far = fit_score(&FOCUS, &candidate(2, 104.0, Some(-16.0))); + assert!(near > far, "near={near} far={far}"); + } + + /// A tempo read an octave out is rescued, and discounted for it. + #[test] + fn an_octave_error_is_rescued_but_discounted() { + let (bpm, factor) = effective_bpm(&FOCUS, 170.0); + assert_eq!(bpm, 85.0, "170 must be read as 85 for a mood built on 85"); + assert_eq!(factor, OCTAVE_PENALTY); + + let honest = fit_score(&FOCUS, &candidate(1, 85.0, Some(-16.0))); + let rescued = fit_score(&FOCUS, &candidate(2, 170.0, Some(-16.0))); + assert!( + rescued < honest, + "a corrected reading must rank below a plain one: {rescued} vs {honest}" + ); + assert!(rescued > 0.0, "but it must still be a candidate"); + } + + /// A track already inside the window is never reinterpreted, even + /// when the double of it would land nearer the centre. + #[test] + fn a_tempo_already_in_the_window_is_left_alone() { + let party = MoodProfile { + bpm_min: Some(60.0), + bpm_max: Some(140.0), + bpm_centre: 124.0, + lufs_max: None, + lufs_min: None, + genre_words: &[], + }; + let (bpm, factor) = effective_bpm(&party, 62.0); + assert_eq!((bpm, factor), (62.0, 1.0)); + } + + /// Unmeasured loudness is neither a pass nor a fail. This is the + /// defect the issue names: an unmeasured track used to satisfy the + /// ceiling exactly as well as a measured quiet one. + #[test] + fn unmeasured_loudness_ranks_between_measured_ones() { + let quiet = fit_score(&FOCUS, &candidate(1, 85.0, Some(-20.0))); + let unknown = fit_score(&FOCUS, &candidate(2, 85.0, None)); + let loud = fit_score(&FOCUS, &candidate(3, 85.0, Some(-4.0))); + assert!(quiet > unknown, "quiet={quiet} unknown={unknown}"); + assert!(unknown > loud, "unknown={unknown} loud={loud}"); + } + + /// A genre the mood does not name costs nothing, because most + /// libraries carry genres nobody curated. + #[test] + fn an_unrecognised_genre_costs_nothing_a_missing_one_does_not_either() { + let mut named = candidate(1, 85.0, Some(-16.0)); + named.genres = Some("ambient".into()); + let mut other = candidate(2, 85.0, Some(-16.0)); + other.genres = Some("death metal".into()); + let missing = candidate(3, 85.0, Some(-16.0)); + + let named = fit_score(&FOCUS, &named); + let other = fit_score(&FOCUS, &other); + let missing = fit_score(&FOCUS, &missing); + assert!(named > other, "a named genre is a bonus"); + assert_eq!( + other, missing, + "an unrecognised genre must cost exactly what no genre costs" + ); + } + + /// The cap skips an artist's weakest tracks, not the ones the draw + /// reached last — which is only true because the cap is applied to + /// the ranking rather than to the pool. + #[test] + fn the_per_artist_cap_keeps_the_best_of_that_artist() { + let mut far = candidate(10, 104.0, Some(-16.0)); + far.primary_artist = Some(7); + let mut near = candidate(11, 85.0, Some(-16.0)); + near.primary_artist = Some(7); + let mut other = candidate(12, 86.0, Some(-16.0)); + other.primary_artist = Some(8); + + let picked = rank_and_cap(&FOCUS, vec![far, near, other], 10, 1); + assert!(picked.contains(&11), "the artist's best must be kept"); + assert!( + !picked.contains(&10), + "its weaker track must be the one cut" + ); + assert!(picked.contains(&12), "another artist is unaffected"); + } + + /// Tracks that lost their artist share one bucket, rather than + /// each counting as a different artist and filling the queue. + #[test] + fn artistless_tracks_share_one_bucket() { + let mut a = candidate(1, 85.0, None); + a.primary_artist = None; + let mut b = candidate(2, 86.0, None); + b.primary_artist = None; + assert_eq!(rank_and_cap(&FOCUS, vec![a, b], 10, 1).len(), 1); + } + + /// Every octave reading of a tempo is offered to the gate, or the + /// correction above would never see the tracks it rescues. + #[test] + fn the_gate_accepts_any_octave() { + assert!(in_window_any_octave(&FOCUS, 85.0)); + assert!(in_window_any_octave(&FOCUS, 170.0), "double"); + assert!(in_window_any_octave(&FOCUS, 42.5), "half"); + assert!(!in_window_any_octave(&FOCUS, 130.0)); + assert!(!in_window_any_octave(&FOCUS, 0.0), "no tempo at all"); + } +} diff --git a/src/components/common/TagFetchModal.tsx b/src/components/common/TagFetchModal.tsx new file mode 100644 index 00000000..63c9e6d2 --- /dev/null +++ b/src/components/common/TagFetchModal.tsx @@ -0,0 +1,565 @@ +import { useEffect, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { X, Check, Loader2, ChevronLeft, Download } from "lucide-react"; +import { + fetchAlbumTagProposals, + searchAlbumTagSources, + TAG_FIELDS, + type AlbumProposals, + type AlbumSource, + type TagField, + type TagValues, + type TrackProposal, +} from "../../lib/tauri/tagFetch"; +import { updateTrackTags, type TrackEdit } from "../../lib/tauri/track"; +import { useModalA11y } from "../../hooks/useModalA11y"; +import { AnimatedModalContent, AnimatedModalShell } from "./AnimatedModalShell"; + +/** + * Fetch an album's tags and approve them, field by field (#599). + * + * Three grains of acceptance, because "take all the years but none of + * the titles" is the common case: one value, one track, or one field + * across the whole album. + * + * Confident matches arrive pre-accepted and doubtful ones do not — + * that is the whole reason the matcher reports two thresholds instead + * of one. A doubtful pairing is worth showing and not worth applying + * for you. + * + * Nothing is written until Apply, and Apply goes through + * `updateTrackTags` one track at a time: the path that pauses + * playback, writes through the concrete tag so non-standard frames + * survive, re-hashes and relinks. A track that fails is counted and + * the rest still run — stopping halfway through an album is what + * leaves a folder in a state nobody can describe. + */ +interface TagFetchModalProps { + isOpen: boolean; + onClose: () => void; + albumId: number; + /** Called after at least one track was written, so the view can + * refetch — the rows on screen are now stale. */ + onApplied?: () => void; +} + +/** Which fields of which tracks the user has accepted. */ +type Accepted = Record>>; + +export function TagFetchModal({ + isOpen, + onClose, + albumId, + onApplied, +}: TagFetchModalProps) { + const { t } = useTranslation(); + const dialogRef = useModalA11y(isOpen, onClose); + const [sources, setSources] = useState(null); + const [proposals, setProposals] = useState(null); + const [accepted, setAccepted] = useState({}); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + const [applied, setApplied] = useState<{ + ok: number; + failed: number; + } | null>(null); + + useEffect(() => { + if (!isOpen) return; + let alive = true; + /* eslint-disable react-hooks/set-state-in-effect */ + setSources(null); + setProposals(null); + setAccepted({}); + setApplied(null); + setError(null); + /* eslint-enable react-hooks/set-state-in-effect */ + searchAlbumTagSources(albumId) + .then((s) => { + if (alive) setSources(s); + }) + .catch((err) => { + if (alive) { + console.error("[TagFetch] source search failed", err); + setError(String(err)); + setSources([]); + } + }); + return () => { + alive = false; + }; + }, [isOpen, albumId]); + + const pickSource = async (source: AlbumSource) => { + setBusy(true); + setError(null); + try { + const result = await fetchAlbumTagProposals(albumId, source.deezer_id); + setProposals(result); + setAccepted(defaultAcceptance(result)); + } catch (err) { + console.error("[TagFetch] proposals failed", err); + setError(String(err)); + } finally { + setBusy(false); + } + }; + + const toggle = (trackId: number, field: TagField) => { + setAccepted((prev) => ({ + ...prev, + [trackId]: { ...prev[trackId], [field]: !prev[trackId]?.[field] }, + })); + }; + + const toggleTrack = (proposal: TrackProposal) => { + const fields = changedFields(proposal); + const allOn = fields.every((f) => accepted[proposal.track_id]?.[f]); + setAccepted((prev) => ({ + ...prev, + [proposal.track_id]: Object.fromEntries( + fields.map((f) => [f, !allOn]), + ) as Partial>, + })); + }; + + const toggleColumn = (field: TagField) => { + if (!proposals) return; + const rows = proposals.tracks.filter((p) => + changedFields(p).includes(field), + ); + const allOn = rows.every((p) => accepted[p.track_id]?.[field]); + setAccepted((prev) => { + const next = { ...prev }; + for (const p of rows) { + next[p.track_id] = { ...next[p.track_id], [field]: !allOn }; + } + return next; + }); + }; + + const pendingCount = proposals + ? proposals.tracks.reduce( + (sum, p) => + sum + + changedFields(p).filter((f) => accepted[p.track_id]?.[f]).length, + 0, + ) + : 0; + + const apply = async () => { + if (!proposals || pendingCount === 0) return; + setBusy(true); + setError(null); + let ok = 0; + let failed = 0; + for (const proposal of proposals.tracks) { + const fields = changedFields(proposal).filter( + (f) => accepted[proposal.track_id]?.[f], + ); + if (fields.length === 0) continue; + const edit: TrackEdit = {}; + for (const field of fields) { + // Only the accepted fields are sent: `update_track_tags` + // leaves an omitted field alone, which is what makes accepting + // one value of one track mean exactly that. + Object.assign(edit, editFragment(proposal, field)); + } + try { + await updateTrackTags(proposal.track_id, edit); + ok += 1; + } catch (err) { + console.error("[TagFetch] write failed", proposal.track_id, err); + failed += 1; + } + } + setApplied({ ok, failed }); + setBusy(false); + if (ok > 0) onApplied?.(); + }; + + return ( + + +
+
+ {proposals && !applied && ( + + )} + +

+ {t("tagFetch.title")} +

+
+ +
+ +
+ {applied ? ( + + ) : proposals ? ( + + ) : ( + + )} + {error &&

{error}

} +
+ +
+ + {proposals && !applied && t("tagFetch.hint")} + +
+ + {proposals && !applied && ( + + )} +
+
+ + + ); +} + +// ============================================================================= +// Steps +// ============================================================================= + +function SourceList({ + sources, + busy, + onPick, +}: { + sources: AlbumSource[] | null; + busy: boolean; + onPick: (s: AlbumSource) => void; +}) { + const { t } = useTranslation(); + if (sources == null || busy) { + return ( +

+ + {busy ? t("tagFetch.matching") : t("tagFetch.searching")} +

+ ); + } + if (sources.length === 0) { + return ( +

+ {t("tagFetch.noSources")} +

+ ); + } + return ( + <> +

+ {t("tagFetch.sourcesHint")} +

+
    + {sources.map((source) => ( +
  • + +
    • + ))} +
+ + ); +} + +function ReviewList({ + proposals, + accepted, + onToggle, + onToggleTrack, + onToggleColumn, +}: { + proposals: AlbumProposals; + accepted: Accepted; + onToggle: (trackId: number, field: TagField) => void; + onToggleTrack: (p: TrackProposal) => void; + onToggleColumn: (field: TagField) => void; +}) { + const { t } = useTranslation(); + const changedAnywhere = TAG_FIELDS.filter((field) => + proposals.tracks.some((p) => changedFields(p).includes(field)), + ); + + if (changedAnywhere.length === 0) { + return ( +

+ {t("tagFetch.nothingToChange")} +

+ ); + } + + return ( + <> + {/* One field across the whole album — the grain that makes "all + the years, none of the titles" one click instead of twelve. */} +
+ + {t("tagFetch.acceptColumn")} + + {changedAnywhere.map((field) => ( + + ))} +
+ +
    + {proposals.tracks.map((proposal) => { + const fields = changedFields(proposal); + return ( +
  • +
    + + {proposal.current.title || proposal.file_name} + + {proposal.confidence && ( + + {t(`tagFetch.confidence.${proposal.confidence}`)} + + )} + {fields.length > 0 && ( + + )} +
    + + {proposal.fetched == null ? ( +

    + {t("tagFetch.noMatch", { file: proposal.file_name })} +

    + ) : fields.length === 0 ? ( +

    + {t("tagFetch.trackUnchanged")} +

    + ) : ( +
      + {fields.map((field) => ( +
    • + onToggle(proposal.track_id, field)} + className="accent-sky-500" + /> + +
      • + ))} +
    + )} +
    • + ); + })} +
+ + {proposals.unmatched_remote.length > 0 && ( +

+ {t("tagFetch.unmatchedRemote", { + titles: proposals.unmatched_remote.join(", "), + })} +

+ )} + + ); +} + +function AppliedSummary({ + applied, +}: { + applied: { ok: number; failed: number }; +}) { + const { t } = useTranslation(); + return ( +
+

+ + {t("tagFetch.appliedCount", { count: applied.ok })} +

+ {applied.failed > 0 && ( +

+ {t("tagFetch.failedCount", { count: applied.failed })} +

+ )} +
+ ); +} + +// ============================================================================= +// Field helpers +// ============================================================================= + +/** The fields where the catalogue says something different. */ +function changedFields(proposal: TrackProposal): TagField[] { + if (!proposal.fetched) return []; + const fetched = proposal.fetched; + return TAG_FIELDS.filter((field) => { + const next = fetched[field]; + // A field the catalogue does not carry is not a change to nothing: + // offering it would invite replacing a value the user typed with a + // blank. + if (next == null || next === "") return false; + return String(next) !== String(proposal.current[field] ?? ""); + }); +} + +function display(values: TagValues, field: TagField): string { + const value = values[field]; + return value == null ? "" : String(value); +} + +/** The `TrackEdit` fragment for one accepted field. */ +function editFragment(proposal: TrackProposal, field: TagField): TrackEdit { + const value = proposal.fetched?.[field]; + switch (field) { + case "title": + return { title: value as string }; + case "artist": + return { artist: value as string }; + case "album": + return { album: value as string }; + case "year": + return { year: value as number }; + case "track_number": + return { track_number: value as number }; + } +} + +/** + * What arrives pre-accepted. + * + * Confident matches, and only those: a doubtful pairing is worth + * showing and not worth applying on the user's behalf, which is the + * entire reason the matcher reports two thresholds rather than one. + */ +function defaultAcceptance(proposals: AlbumProposals): Accepted { + const out: Accepted = {}; + for (const proposal of proposals.tracks) { + if (proposal.confidence !== "confident") continue; + const fields = changedFields(proposal); + if (fields.length === 0) continue; + out[proposal.track_id] = Object.fromEntries( + fields.map((f) => [f, true]), + ) as Partial>; + } + return out; +} diff --git a/src/components/views/AlbumDetailView.tsx b/src/components/views/AlbumDetailView.tsx index 4157d99c..752d026c 100644 --- a/src/components/views/AlbumDetailView.tsx +++ b/src/components/views/AlbumDetailView.tsx @@ -7,6 +7,7 @@ import { Music2, Heart, ImageIcon, + DownloadCloud, Film, } from "lucide-react"; import { @@ -20,6 +21,7 @@ import { ArtistLink } from "../common/ArtistLink"; import { EmptyState } from "../common/EmptyState"; import { DetailViewSkeleton } from "../common/DetailViewSkeleton"; import { CreatePlaylistModal } from "../common/CreatePlaylistModal"; +import { TagFetchModal } from "../common/TagFetchModal"; import { CoverPickerModal } from "../common/CoverPickerModal"; import { MotionCoverPickerModal } from "../common/MotionCoverPickerModal"; import { HiResBadge } from "../common/HiResBadge"; @@ -167,6 +169,7 @@ export function AlbumDetailView({ // one-frame "album not found" flash before the fetch schedules. const [isLoading, setIsLoading] = useState(true); const [likedIds, setLikedIds] = useState>(new Set()); + const [isTagFetchOpen, setIsTagFetchOpen] = useState(false); const [isCreatePlaylistModalOpen, setIsCreatePlaylistModalOpen] = useState(false); const [isCoverPickerOpen, setIsCoverPickerOpen] = useState(false); @@ -535,6 +538,15 @@ export function AlbumDetailView({ {t("albumDetail.setMotionCover")} + )} @@ -578,6 +590,16 @@ export function AlbumDetailView({ /> )} + {!remote && albumId != null && ( + setIsTagFetchOpen(false)} + albumId={albumId} + // The rows on screen carry the old tags; a write makes them + // stale, and this is the same stamp a tag edit already bumps. + onApplied={() => setEditRefetch((n) => n + 1)} + /> + )} setIsCreatePlaylistModalOpen(false)} diff --git a/src/components/views/home/MoodRadioGrid.tsx b/src/components/views/home/MoodRadioGrid.tsx index 6dfde482..338e680d 100644 --- a/src/components/views/home/MoodRadioGrid.tsx +++ b/src/components/views/home/MoodRadioGrid.tsx @@ -93,14 +93,22 @@ export function MoodRadioGrid() { } }; - const totalAnalysed = counts + const eligible = counts ? counts.focus + counts.chill + counts.workout + counts.party + counts.sleep : 0; // When no mood matches anything, the library either has no BPM // analysis at all or only a handful of analysed tracks. In that // case we hide the section entirely instead of showing a row of // disabled tiles — feels less broken. - if (counts != null && totalAnalysed === 0) return null; + if (counts != null && eligible === 0) return null; + + // Every mood draws from the analysed part of the library, so a + // partly-analysed one gives thin radios for a reason the tiles + // cannot show: their counts look small without saying small *of + // what*. Shown only while the two numbers differ — once everything + // is analysed the line has nothing to add. + const partlyAnalysed = + counts != null && counts.analysed_tracks < counts.total_tracks; return (
{t("home.moodRadio.title")} - + {t("home.moodRadio.subtitle")} + {partlyAnalysed && ( + <> +
+ {t("home.moodRadio.coverage", { + analysed: counts.analysed_tracks, + total: counts.total_tracks, + })} + + )}
{ return invoke("start_mood_radio", { mood }); } -/** How many qualifying tracks each mood would yield, given the - * library's current state of BPM/loudness analysis. */ +/** + * How many tracks each mood could draw from, plus how much of the + * library has been analysed at all. + * + * The per-mood numbers answer the tempo gate only — loudness and genre + * rank rather than exclude, so they cannot make a mood empty. The + * coverage pair is what lets the UI say *why* a mood is thin instead + * of leaving the user to guess. + */ export function moodRadioCounts(): Promise { return invoke("mood_radio_counts"); } diff --git a/src/lib/tauri/tagFetch.ts b/src/lib/tauri/tagFetch.ts new file mode 100644 index 00000000..b1face38 --- /dev/null +++ b/src/lib/tauri/tagFetch.ts @@ -0,0 +1,94 @@ +import { invoke } from "@tauri-apps/api/core"; + +/** + * Fetching an album's tags from Deezer, for review (#599). + * + * Two steps on purpose. A title and an artist match several releases of + * the same record — an original, a remaster, a deluxe edition with four + * more tracks — and they carry different track lists, so which release + * this is stays the user's call. + * + * Neither call writes anything. What the review screen accepts is + * applied through `updateTrackTags`, the path that pauses playback + * before opening the file, writes through the concrete tag so + * non-standard frames survive, re-hashes and relinks the rows. + */ + +/** One catalogue release that might be this record. */ +export interface AlbumSource { + deezer_id: number; + title: string; + artist: string | null; + track_count: number | null; + year: number | null; + cover_url: string | null; +} + +/** + * The values a fetch can offer for one track. + * + * Every field is optional on both sides: the file may not carry it and + * the catalogue may not either. Composer, genre and disc number are + * absent because Deezer cannot fill them reliably, and a review screen + * that lists a field the source cannot fill invites accepting a blank + * over something the user typed. + */ +export interface TagValues { + title: string | null; + artist: string | null; + album: string | null; + year: number | null; + track_number: number | null; +} + +/** The fields the review screen can accept, one at a time. */ +export const TAG_FIELDS = [ + "title", + "artist", + "album", + "year", + "track_number", +] as const; +export type TagField = (typeof TAG_FIELDS)[number]; + +/** How sure the matcher is about a pairing. */ +export type MatchConfidence = "confident" | "doubtful"; + +export interface TrackProposal { + track_id: number; + /** Shown for a track nothing matched, where a title alone would not + * tell the user which file is meant. */ + file_name: string; + current: TagValues; + /** `null` when nothing in the release matched this file well enough. + * A real answer — the screen must not apply "the best available". */ + fetched: TagValues | null; + score: number | null; + confidence: MatchConfidence | null; +} + +export interface AlbumProposals { + album_id: number; + deezer_id: number; + /** Tracks in the album's own order, matched or not. */ + tracks: TrackProposal[]; + /** Catalogue tracks no local file claimed — a deluxe edition's + * extras, or the songs a partial rip is missing. */ + unmatched_remote: string[]; +} + +/** Catalogue releases that might be this album. */ +export function searchAlbumTagSources(albumId: number): Promise { + return invoke("search_album_tag_sources", { albumId }); +} + +/** Pair the chosen release's tracks with the local files. */ +export function fetchAlbumTagProposals( + albumId: number, + deezerAlbumId: number, +): Promise { + return invoke("fetch_album_tag_proposals", { + albumId, + deezerAlbumId, + }); +} From 6779a82e67564f00e35479e41598e72e148f91c3 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 16 Sep 2026 01:02:10 +0200 Subject: [PATCH 02/16] fix(radio, tags): four findings from the local review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The one that mattered: an unbounded side of a mood had a "too far". Sleep has no tempo floor, so a 30 BPM drone scored 0.35 against the 0.95 of a track at its 52 BPM centre — the slowest tracks in the library were the worst fit for the mood built on slowness. Below the centre with no floor now scores as well as the centre; the bounded side still ranks. A test that fails without the guard. Deezer's album track listing paginates, and reading one page of it is exactly the defect the doc comment above it claimed to avoid: a box set came back short with nothing in the response to say it was cut. The pages are walked now, by computed index rather than by following the URL the response carries, with two independent stops and a cap. GROUP_CONCAT was ordered by a clause that runs after the aggregation, so it ordered one row and the names inside the credit came out in scan order. Moved into an inner subquery, the shape the rest of the codebase already uses — ordering inside the aggregate call is SQLite 3.44, newer than we can require. And a confident match no longer arrives with "replace both artists with the first" pre-ticked: Deezer gives a track one artist, so a local credit of "A; B" always reads as a change. The row is still there to accept by hand; only the default refuses to throw away a credit the library models better than the catalogue does. --- .../crates/app/src/commands/tag_fetch.rs | 19 +++++-- src-tauri/crates/core/src/metadata/deezer.rs | 55 ++++++++++++++++--- src-tauri/crates/core/src/mood.rs | 38 +++++++++++++ src/components/common/TagFetchModal.tsx | 17 +++++- 4 files changed, 114 insertions(+), 15 deletions(-) diff --git a/src-tauri/crates/app/src/commands/tag_fetch.rs b/src-tauri/crates/app/src/commands/tag_fetch.rs index ecb0c8b7..2d961df5 100644 --- a/src-tauri/crates/app/src/commands/tag_fetch.rs +++ b/src-tauri/crates/app/src/commands/tag_fetch.rs @@ -300,6 +300,13 @@ struct LocalTrack { /// library's one spelling for a multi-artist credit — reading /// `track.primary_artist` instead would offer to replace a full credit /// with its first name, which is a silent loss dressed as a fix. +/// +/// The ordering goes in an **inner subquery**, the shape the rest of +/// the codebase uses: an `ORDER BY` in the aggregate's own query runs +/// after the aggregation and orders one row, so the names inside the +/// string come out in whatever order the scan happened to reach them. +/// Ordering inside the aggregate call is SQLite 3.44, newer than what +/// we can require. async fn local_tracks(pool: &SqlitePool, album_id: i64) -> AppResult> { #[derive(sqlx::FromRow)] struct Row { @@ -321,11 +328,13 @@ async fn local_tracks(pool: &SqlitePool, album_id: i64) -> AppResult { pub data: Vec, + /// Link to the following page, absent on the last one. Deezer sends + /// it on every list endpoint; the search paths here read one page + /// on purpose, so it is only consulted where a short answer would + /// be a wrong answer. + #[serde(default)] + pub next: Option, } /// Deezer's in-band error object. @@ -325,18 +331,49 @@ impl DeezerClient { Self::fetch(self.http.get(format!("{BASE_URL}/album/{deezer_id}"))).await } - /// The tracks of an album, in the catalogue's own order. + /// The tracks of an album, in the catalogue's own order, **every + /// page of them**. /// /// `/album/{id}/tracks` rather than the `tracks` field of - /// `/album/{id}`: the embedded list is capped, and a box set read - /// through it comes back short with nothing to say it was cut. + /// `/album/{id}`: the embedded list is capped. But so is this one — + /// it is a paged list like every other Deezer collection, and a box + /// set read as one page comes back short with nothing in the + /// response to say it was cut. The pages are walked until the API + /// stops offering a next one. + /// + /// The following page is requested by `index`, computed here, + /// rather than by following the `next` URL the response carries: it + /// keeps every request this client makes one this code built, and + /// it costs a line. pub async fn get_album_tracks(&self, deezer_id: i64) -> DeezerResult> { - let resp: DeezerSearchResponse = Self::fetch( - self.http - .get(format!("{BASE_URL}/album/{deezer_id}/tracks")), - ) - .await?; - Ok(resp.data) + /// Deezer's own maximum page size for a collection. + const PAGE: usize = 100; + /// Ten thousand tracks. A real release is three orders of + /// magnitude below this; the cap is here so a catalogue that + /// keeps saying "there is more" cannot spin forever. + const MAX_PAGES: usize = 100; + + let mut out: Vec = Vec::new(); + for page in 0..MAX_PAGES { + let resp: DeezerSearchResponse = Self::fetch( + self.http + .get(format!("{BASE_URL}/album/{deezer_id}/tracks")) + .query(&[ + ("index", (page * PAGE).to_string()), + ("limit", PAGE.to_string()), + ]), + ) + .await?; + let received = resp.data.len(); + out.extend(resp.data); + // Two independent stops, because either alone has been seen + // to lie: a short page is the end whatever `next` says, and + // a full page with no `next` is the end too. + if received < PAGE || resp.next.is_none() { + break; + } + } + Ok(out) } /// Fetch artists Deezer reports as related to the given artist. diff --git a/src-tauri/crates/core/src/mood.rs b/src-tauri/crates/core/src/mood.rs index f7c997c0..571a8986 100644 --- a/src-tauri/crates/core/src/mood.rs +++ b/src-tauri/crates/core/src/mood.rs @@ -141,7 +141,18 @@ pub fn fit_score(profile: &MoodProfile, candidate: &MoodCandidate) -> f64 { /// its window. A track at the centre scores 1, one at the edge scores /// close to 0, and the fall is linear because nothing about a tempo /// preference justifies a sharper curve. +/// +/// **An unbounded side has no "too far".** Sleep has no floor, so a +/// 30 BPM drone is not a worse fit for it than one at its 52 BPM +/// centre — measuring the distance anyway scored the slowest tracks in +/// the library worst for the mood built on slowness. fn tempo_score(profile: &MoodProfile, bpm: f64) -> f64 { + if bpm < profile.bpm_centre && profile.bpm_min.is_none() { + return 1.0; + } + if bpm > profile.bpm_centre && profile.bpm_max.is_none() { + return 1.0; + } let reach = half_width(profile); if reach <= 0.0 { return 1.0; @@ -368,6 +379,33 @@ mod tests { assert!(picked.contains(&12), "another artist is unaffected"); } + /// A mood open on one side does not punish tracks that go further + /// that way. Sleep is the case: it has no floor, and scoring by + /// distance from its centre made the slowest track in the library + /// the worst fit for the mood built on slowness. + #[test] + fn an_unbounded_side_has_no_too_far() { + const SLEEP: MoodProfile = MoodProfile { + bpm_min: None, + bpm_max: Some(68.0), + bpm_centre: 52.0, + lufs_max: Some(-18.0), + lufs_min: None, + genre_words: &[], + }; + let centre = fit_score(&SLEEP, &candidate(1, 52.0, Some(-20.0))); + let slower = fit_score(&SLEEP, &candidate(2, 30.0, Some(-20.0))); + assert_eq!( + slower, centre, + "below the centre, with no floor, is as good" + ); + let edge = fit_score(&SLEEP, &candidate(3, 68.0, Some(-20.0))); + assert!( + edge < centre, + "the bounded side still ranks: {edge} vs {centre}" + ); + } + /// Tracks that lost their artist share one bucket, rather than /// each counting as a different artist and filling the queue. #[test] diff --git a/src/components/common/TagFetchModal.tsx b/src/components/common/TagFetchModal.tsx index 63c9e6d2..551e71e9 100644 --- a/src/components/common/TagFetchModal.tsx +++ b/src/components/common/TagFetchModal.tsx @@ -544,18 +544,33 @@ function editFragment(proposal: TrackProposal, field: TagField): TrackEdit { } } +/** The library's one spelling for a multi-artist credit. */ +const MULTI_ARTIST_SEPARATOR = "; "; + /** * What arrives pre-accepted. * * Confident matches, and only those: a doubtful pairing is worth * showing and not worth applying on the user's behalf, which is the * entire reason the matcher reports two thresholds rather than one. + * + * With one exception. Deezer gives a track **one** artist, and a local + * credit of "A; B" therefore always reads as a change — so a confident + * match would arrive with "replace both names with the first" already + * ticked. The row stays visible and can still be accepted by hand; it + * is only the default that refuses to throw away a credit the library + * models better than the catalogue does. */ function defaultAcceptance(proposals: AlbumProposals): Accepted { const out: Accepted = {}; for (const proposal of proposals.tracks) { if (proposal.confidence !== "confident") continue; - const fields = changedFields(proposal); + const multiArtist = (proposal.current.artist ?? "").includes( + MULTI_ARTIST_SEPARATOR, + ); + const fields = changedFields(proposal).filter( + (f) => !(f === "artist" && multiArtist), + ); if (fields.length === 0) continue; out[proposal.track_id] = Object.fromEntries( fields.map((f) => [f, true]), From f6ea9f90861357d5a88b147cd2666750a23661f2 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 16 Sep 2026 01:16:03 +0200 Subject: [PATCH 03/16] refactor(radio): one spelling of the tempo gate, and tests that run the SQL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plain/doubled/halved predicate was written out three times — the track pool, the album pool and the count behind the home tile. A copy that drifted would make a mood report a number it cannot deliver, or hide one it can. It is a macro now, pasted by concat! so every query stays a literal and sqlx is never handed SQL it cannot verify. Which exposed the other half: none of these queries is compile-time checked, so a missing parenthesis compiles perfectly and fails the first time somebody presses a mood tile. Two tests against the real profile migrations — every query of every mood, and a 170 BPM track reaching a mood built on 88 as a halved reading, because the scorer cannot rescue a row the query never returned. --- .../crates/app/src/commands/mood_radio.rs | 220 +++++++++++++----- 1 file changed, 160 insertions(+), 60 deletions(-) diff --git a/src-tauri/crates/app/src/commands/mood_radio.rs b/src-tauri/crates/app/src/commands/mood_radio.rs index 894d6416..8593f090 100644 --- a/src-tauri/crates/app/src/commands/mood_radio.rs +++ b/src-tauri/crates/app/src/commands/mood_radio.rs @@ -61,6 +61,24 @@ const ALBUM_POOL_SIZE: i64 = 60; /// the same artist in a row. const ALBUM_PER_ARTIST_CAP: usize = 2; +/// The tempo gate, in SQL: does any octave reading of `ta.bpm` fall +/// inside `?1 .. ?2`? +/// +/// Written once because three queries ask it — the track pool, the +/// album pool and the count behind the home tile — and a copy that +/// drifted would make a mood report a number it cannot deliver, or +/// hide one it can. A macro rather than a `const`, so the fragment is +/// pasted by `concat!` and every query stays a `&'static str` literal: +/// building the string at runtime would mean handing sqlx SQL it +/// cannot verify. +macro_rules! bpm_any_octave { + () => { + "( ((?1 IS NULL OR ta.bpm >= ?1) AND (?2 IS NULL OR ta.bpm <= ?2)) \ + OR ((?1 IS NULL OR ta.bpm * 2.0 >= ?1) AND (?2 IS NULL OR ta.bpm * 2.0 <= ?2)) \ + OR ((?1 IS NULL OR ta.bpm / 2.0 >= ?1) AND (?2 IS NULL OR ta.bpm / 2.0 <= ?2)))" + }; +} + #[derive(Debug, Deserialize)] #[serde(rename_all = "lowercase")] pub enum Mood { @@ -251,27 +269,24 @@ async fn candidate_pool( genres: Option, } - let rows: Vec = sqlx::query_as::<_, Row>( - r#" - SELECT t.id AS track_id, - t.primary_artist AS primary_artist, - ta.bpm AS bpm, - ta.loudness_lufs AS loudness_lufs, - (SELECT GROUP_CONCAT(g.name, ' ') - FROM track_genre tg JOIN genre g ON g.id = tg.genre_id - WHERE tg.track_id = t.id) AS genres - FROM track t - JOIN track_analysis ta ON ta.track_id = t.id - WHERE t.is_available = 1 - AND ta.bpm IS NOT NULL - AND ta.bpm > 0 - AND ( ((?1 IS NULL OR ta.bpm >= ?1) AND (?2 IS NULL OR ta.bpm <= ?2)) - OR ((?1 IS NULL OR ta.bpm * 2.0 >= ?1) AND (?2 IS NULL OR ta.bpm * 2.0 <= ?2)) - OR ((?1 IS NULL OR ta.bpm / 2.0 >= ?1) AND (?2 IS NULL OR ta.bpm / 2.0 <= ?2))) - ORDER BY RANDOM() - LIMIT ?3 - "#, - ) + let rows: Vec = sqlx::query_as::<_, Row>(concat!( + "SELECT t.id AS track_id, + t.primary_artist AS primary_artist, + ta.bpm AS bpm, + ta.loudness_lufs AS loudness_lufs, + (SELECT GROUP_CONCAT(g.name, ' ') + FROM track_genre tg JOIN genre g ON g.id = tg.genre_id + WHERE tg.track_id = t.id) AS genres + FROM track t + JOIN track_analysis ta ON ta.track_id = t.id + WHERE t.is_available = 1 + AND ta.bpm IS NOT NULL + AND ta.bpm > 0 + AND ", + bpm_any_octave!(), + " ORDER BY RANDOM() + LIMIT ?3" + )) .bind(profile.bpm_min) .bind(profile.bpm_max) .bind(limit) @@ -320,32 +335,25 @@ async fn mood_radio_by_album(pool: &SqlitePool, profile: &MoodProfile) -> AppRes avg_lufs: Option, } - let rows: Vec = sqlx::query_as::<_, Row>( - r#" - SELECT t.album_id AS album_id, - COUNT(*) AS track_count, - MIN(t.primary_artist) AS primary_artist, - AVG(ta.bpm) AS avg_bpm, - AVG(ta.loudness_lufs) AS avg_lufs - FROM track t - LEFT JOIN track_analysis ta ON ta.track_id = t.id - WHERE t.is_available = 1 - AND t.album_id IS NOT NULL - GROUP BY t.album_id - HAVING SUM(CASE WHEN ta.bpm IS NOT NULL THEN 1 ELSE 0 END) >= ?4 - AND SUM(CASE WHEN ta.bpm IS NOT NULL AND ta.bpm > 0 - AND ( ((?1 IS NULL OR ta.bpm >= ?1) - AND (?2 IS NULL OR ta.bpm <= ?2)) - OR ((?1 IS NULL OR ta.bpm * 2.0 >= ?1) - AND (?2 IS NULL OR ta.bpm * 2.0 <= ?2)) - OR ((?1 IS NULL OR ta.bpm / 2.0 >= ?1) - AND (?2 IS NULL OR ta.bpm / 2.0 <= ?2))) - THEN 1 ELSE 0 END) * 1.0 - / SUM(CASE WHEN ta.bpm IS NOT NULL THEN 1 ELSE 0 END) >= ?3 - ORDER BY RANDOM() - LIMIT ?5 - "#, - ) + let rows: Vec = sqlx::query_as::<_, Row>(concat!( + "SELECT t.album_id AS album_id, + COUNT(*) AS track_count, + MIN(t.primary_artist) AS primary_artist, + AVG(ta.bpm) AS avg_bpm, + AVG(ta.loudness_lufs) AS avg_lufs + FROM track t + LEFT JOIN track_analysis ta ON ta.track_id = t.id + WHERE t.is_available = 1 + AND t.album_id IS NOT NULL + GROUP BY t.album_id + HAVING SUM(CASE WHEN ta.bpm IS NOT NULL THEN 1 ELSE 0 END) >= ?4 + AND SUM(CASE WHEN ta.bpm IS NOT NULL AND ta.bpm > 0 AND ", + bpm_any_octave!(), + " THEN 1 ELSE 0 END) * 1.0 + / SUM(CASE WHEN ta.bpm IS NOT NULL THEN 1 ELSE 0 END) >= ?3 + ORDER BY RANDOM() + LIMIT ?5" + )) .bind(profile.bpm_min) .bind(profile.bpm_max) .bind(ALBUM_FIT) @@ -460,22 +468,114 @@ async fn analysis_coverage(pool: &SqlitePool) -> AppResult<(i64, i64)> { } async fn count_for_mood(pool: &SqlitePool, profile: &MoodProfile) -> AppResult { - let n: i64 = sqlx::query_scalar( - r#" - SELECT COUNT(*) - FROM track t - JOIN track_analysis ta ON ta.track_id = t.id - WHERE t.is_available = 1 - AND ta.bpm IS NOT NULL - AND ta.bpm > 0 - AND ( ((?1 IS NULL OR ta.bpm >= ?1) AND (?2 IS NULL OR ta.bpm <= ?2)) - OR ((?1 IS NULL OR ta.bpm * 2.0 >= ?1) AND (?2 IS NULL OR ta.bpm * 2.0 <= ?2)) - OR ((?1 IS NULL OR ta.bpm / 2.0 >= ?1) AND (?2 IS NULL OR ta.bpm / 2.0 <= ?2))) - "#, - ) + let n: i64 = sqlx::query_scalar(concat!( + "SELECT COUNT(*) + FROM track t + JOIN track_analysis ta ON ta.track_id = t.id + WHERE t.is_available = 1 + AND ta.bpm IS NOT NULL + AND ta.bpm > 0 + AND ", + bpm_any_octave!() + )) .bind(profile.bpm_min) .bind(profile.bpm_max) .fetch_one(pool) .await?; Ok(n) } + +#[cfg(test)] +mod tests { + use super::*; + use sqlx::sqlite::{SqliteConnectOptions, SqlitePoolOptions}; + use std::str::FromStr; + + /// The repo's own profile migrations, with `foreign_keys` on. + /// + /// A hand-written fixture would let these queries pass against a + /// schema the app never has — the same reason the inventory tests + /// run the real migrations. + async fn pool() -> SqlitePool { + let options = SqliteConnectOptions::from_str(":memory:") + .unwrap() + .foreign_keys(true); + let pool = SqlitePoolOptions::new() + .max_connections(1) + .connect_with(options) + .await + .unwrap(); + sqlx::migrate!("../../migrations/profile") + .run(&pool) + .await + .unwrap(); + pool + } + + /// Every query in this module runs, against the real schema, for + /// every mood. + /// + /// The tempo gate is pasted into three statements by a macro and + /// none of them is a compile-time-checked query: a missing + /// parenthesis or a column that does not exist would compile + /// perfectly and fail the first time a user pressed a mood tile. + /// An empty library is enough to prove the SQL parses and that the + /// placeholders line up with the binds. + #[tokio::test] + async fn every_mood_query_parses_and_binds() { + let pool = pool().await; + for mood in [ + Mood::Focus, + Mood::Chill, + Mood::Workout, + Mood::Party, + Mood::Sleep, + ] { + let profile = mood.profile(); + assert!(candidate_pool(&pool, &profile, 10) + .await + .unwrap() + .is_empty()); + assert!(mood_radio_by_album(&pool, &profile) + .await + .unwrap() + .is_empty()); + assert_eq!(count_for_mood(&pool, &profile).await.unwrap(), 0); + } + assert_eq!(analysis_coverage(&pool).await.unwrap(), (0, 0)); + } + + /// An octave-out tempo reaches the pool, which is what the gate in + /// SQL exists for — the scorer cannot rescue a row the query never + /// returned. + #[tokio::test] + async fn the_pool_accepts_a_doubled_tempo() { + let pool = pool().await; + sqlx::query( + "INSERT INTO library (id, name, path, created_at) VALUES (1, 'l', '/l', 0); + INSERT INTO track (id, library_id, file_path, file_hash, file_size, + file_modified, title, duration_ms, added_at) + VALUES (1, 1, '/l/a.flac', 'h1', 1, 0, 'Fast', 200000, 0), + (2, 1, '/l/b.flac', 'h2', 1, 0, 'Slow', 200000, 0); + INSERT INTO track_analysis (track_id, bpm, analyzed_at) + VALUES (1, 170.0, 0), (2, 40.0, 0);", + ) + .execute(&pool) + .await + .unwrap(); + + let focus = Mood::Focus.profile(); + let ids: Vec = candidate_pool(&pool, &focus, 10) + .await + .unwrap() + .into_iter() + .map(|c| c.track_id) + .collect(); + assert!( + ids.contains(&1), + "170 BPM must reach a mood built on 88, as a halved reading" + ); + assert!(!ids.contains(&2), "40 BPM is out under every reading"); + assert_eq!(count_for_mood(&pool, &focus).await.unwrap(), 1); + } +} From b3a6f3a7ba05aa636a50f3617b83ddf5461ab17a Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 16 Sep 2026 01:23:05 +0200 Subject: [PATCH 04/16] fix(radio): a guess must not take a pool slot from a measured tempo --- .../crates/app/src/commands/mood_radio.rs | 55 ++++++++++++++++--- 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/src-tauri/crates/app/src/commands/mood_radio.rs b/src-tauri/crates/app/src/commands/mood_radio.rs index 8593f090..b123ec7d 100644 --- a/src-tauri/crates/app/src/commands/mood_radio.rs +++ b/src-tauri/crates/app/src/commands/mood_radio.rs @@ -73,9 +73,28 @@ const ALBUM_PER_ARTIST_CAP: usize = 2; /// cannot verify. macro_rules! bpm_any_octave { () => { - "( ((?1 IS NULL OR ta.bpm >= ?1) AND (?2 IS NULL OR ta.bpm <= ?2)) \ - OR ((?1 IS NULL OR ta.bpm * 2.0 >= ?1) AND (?2 IS NULL OR ta.bpm * 2.0 <= ?2)) \ - OR ((?1 IS NULL OR ta.bpm / 2.0 >= ?1) AND (?2 IS NULL OR ta.bpm / 2.0 <= ?2)))" + concat!( + "( ", + bpm_plain_in_window!(), + " OR ((?1 IS NULL OR ta.bpm * 2.0 >= ?1) AND (?2 IS NULL OR ta.bpm * 2.0 <= ?2))", + " OR ((?1 IS NULL OR ta.bpm / 2.0 >= ?1) AND (?2 IS NULL OR ta.bpm / 2.0 <= ?2)))" + ) + }; +} + +/// The tempo gate for the **measured** reading alone. +/// +/// Half of [`bpm_any_octave!`], and also what orders the draw: a +/// corrected reading is a guess, and a guess must not take a place in +/// the pool from a track that really is this tempo. Sleep is where it +/// shows — its window has no floor, so everything up to 136 BPM +/// qualifies once halved, which is most of a library. Drawing at +/// random across all of that would fill the mood built on slowness +/// with halved dance records whenever the genuinely slow ones are +/// rare, which is the complaint the issue opened on. +macro_rules! bpm_plain_in_window { + () => { + "((?1 IS NULL OR ta.bpm >= ?1) AND (?2 IS NULL OR ta.bpm <= ?2))" }; } @@ -284,7 +303,15 @@ async fn candidate_pool( AND ta.bpm > 0 AND ", bpm_any_octave!(), - " ORDER BY RANDOM() + // Measured matches first, still shuffled among themselves; the + // corrected ones only top up what is left. The album path does + // not need this: it is ranked on the record's average tempo, + // which the scorer discounts the same way, and its gate already + // asks that most of the record fit — a record of corrected + // readings rarely clears that. + " ORDER BY CASE WHEN ", + bpm_plain_in_window!(), + " THEN 0 ELSE 1 END, RANDOM() LIMIT ?3" )) .bind(profile.bpm_min) @@ -556,9 +583,12 @@ mod tests { INSERT INTO track (id, library_id, file_path, file_hash, file_size, file_modified, title, duration_ms, added_at) VALUES (1, 1, '/l/a.flac', 'h1', 1, 0, 'Fast', 200000, 0), - (2, 1, '/l/b.flac', 'h2', 1, 0, 'Slow', 200000, 0); + (2, 1, '/l/b.flac', 'h2', 1, 0, 'Slow', 200000, 0), + (3, 1, '/l/c.flac', 'h3', 1, 0, 'Plain', 200000, 0); + -- 30 BPM, not 40: doubled, 40 lands on 80, which is + -- inside Focus. Every reading of 30 (30, 60, 15) is out. INSERT INTO track_analysis (track_id, bpm, analyzed_at) - VALUES (1, 170.0, 0), (2, 40.0, 0);", + VALUES (1, 170.0, 0), (2, 30.0, 0), (3, 90.0, 0);", ) .execute(&pool) .await @@ -575,7 +605,16 @@ mod tests { ids.contains(&1), "170 BPM must reach a mood built on 88, as a halved reading" ); - assert!(!ids.contains(&2), "40 BPM is out under every reading"); - assert_eq!(count_for_mood(&pool, &focus).await.unwrap(), 1); + assert!(!ids.contains(&2), "30 BPM is out under every reading"); + assert_eq!(count_for_mood(&pool, &focus).await.unwrap(), 2); + + // A measured match comes before a corrected one, whatever the + // shuffle does inside each group: a guess must not take a place + // in the pool from a track that really is this tempo. + assert_eq!( + ids.first(), + Some(&3), + "90 BPM is measured inside the window; 170 is a rescue" + ); } } From b3aa3c7f2506a8df869d1d69db4d725ca998c280 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 16 Sep 2026 01:27:34 +0200 Subject: [PATCH 05/16] test(radio): the library table has no path column --- src-tauri/crates/app/src/commands/mood_radio.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src-tauri/crates/app/src/commands/mood_radio.rs b/src-tauri/crates/app/src/commands/mood_radio.rs index b123ec7d..a1700d95 100644 --- a/src-tauri/crates/app/src/commands/mood_radio.rs +++ b/src-tauri/crates/app/src/commands/mood_radio.rs @@ -579,7 +579,7 @@ mod tests { async fn the_pool_accepts_a_doubled_tempo() { let pool = pool().await; sqlx::query( - "INSERT INTO library (id, name, path, created_at) VALUES (1, 'l', '/l', 0); + "INSERT INTO library (id, name, created_at, updated_at) VALUES (1, 'l', 0, 0); INSERT INTO track (id, library_id, file_path, file_hash, file_size, file_modified, title, duration_ms, added_at) VALUES (1, 1, '/l/a.flac', 'h1', 1, 0, 'Fast', 200000, 0), From 22e26261519cf82abf4134297198b70e3099e8b7 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 16 Sep 2026 01:40:31 +0200 Subject: [PATCH 06/16] fix(tags): a write in flight cannot be dismissed, and both reads are tested --- docs/features/smart-playlists.md | 8 ++- .../crates/app/src/commands/tag_fetch.rs | 67 +++++++++++++++++++ src-tauri/crates/core/src/metadata/deezer.rs | 20 +++--- src/components/common/TagFetchModal.tsx | 24 +++++-- 4 files changed, 102 insertions(+), 17 deletions(-) diff --git a/docs/features/smart-playlists.md b/docs/features/smart-playlists.md index ac2313e3..0a905687 100644 --- a/docs/features/smart-playlists.md +++ b/docs/features/smart-playlists.md @@ -244,9 +244,11 @@ pool rather than the first forty drawn out of it (#616). | Party | 110–132 | 122 | prefers ≥ −12 LUFS | | Sleep | ≤ 68 | 52 | prefers ≤ −18 LUFS | -The pool is still drawn with `ORDER BY RANDOM() LIMIT 400`, which is -what keeps two runs of one mood from being the same queue; the ranking -then decides which of it plays. +The pool of 400 is drawn **measured readings first**, shuffled within +each group, and the ranking then decides which forty of it play. The +shuffle is what keeps two runs of one mood from being the same queue; +the priority is what stops a guess from taking a slot from a track that +really is this tempo — see the octave note below. ### Why only tempo gates diff --git a/src-tauri/crates/app/src/commands/tag_fetch.rs b/src-tauri/crates/app/src/commands/tag_fetch.rs index 2d961df5..ccf81f3c 100644 --- a/src-tauri/crates/app/src/commands/tag_fetch.rs +++ b/src-tauri/crates/app/src/commands/tag_fetch.rs @@ -370,6 +370,8 @@ fn file_name_of(path: &str) -> String { #[cfg(test)] mod tests { use super::*; + use sqlx::sqlite::{SqliteConnectOptions, SqlitePoolOptions}; + use std::str::FromStr; #[test] fn file_name_handles_both_separators() { @@ -377,4 +379,69 @@ mod tests { assert_eq!(file_name_of("/home/u/Music/a/b.flac"), "b.flac"); assert_eq!(file_name_of("bare.flac"), "bare.flac"); } + + /// The repo's own profile migrations, with `foreign_keys` on. + async fn pool() -> SqlitePool { + let options = SqliteConnectOptions::from_str(":memory:") + .unwrap() + .foreign_keys(true); + let pool = SqlitePoolOptions::new() + .max_connections(1) + .connect_with(options) + .await + .unwrap(); + sqlx::migrate!("../../migrations/profile") + .run(&pool) + .await + .unwrap(); + pool + } + + /// Both reads run against the real schema, and the credit comes + /// back in `track_artist.position` order. + /// + /// Neither query is compile-time checked, and the credit is built + /// by a correlated `GROUP_CONCAT` over an ordered subquery — the + /// shape that is easy to write in a way that compiles, runs, and + /// quietly returns the names in whatever order the scan reached + /// them. + #[tokio::test] + async fn the_reads_run_and_the_credit_keeps_its_order() { + let pool = pool().await; + sqlx::raw_sql( + "INSERT INTO library (id, name, created_at, updated_at) + VALUES (1, 'l', 0, 0); + INSERT INTO artist (id, name) VALUES (1, 'Second'), (2, 'First'); + INSERT INTO album (id, title, artist_id) VALUES (1, 'Record', 2); + INSERT INTO track (id, library_id, file_path, file_hash, file_size, + file_modified, title, duration_ms, added_at, + album_id, track_number, year) + VALUES (1, 1, '/l/a.flac', 'h1', 1, 0, 'Song', 200000, 0, 1, 1, 1999); + -- Inserted out of order on purpose: position decides, not + -- the insertion order and not the artist id. + INSERT INTO track_artist (track_id, artist_id, position) + VALUES (1, 1, 1), (1, 2, 0);", + ) + .execute(&pool) + .await + .unwrap(); + + let (title, artist) = album_identity(&pool, 1).await.unwrap(); + assert_eq!(title, "Record"); + assert_eq!(artist.as_deref(), Some("First")); + + let tracks = local_tracks(&pool, 1).await.unwrap(); + assert_eq!(tracks.len(), 1); + assert_eq!(tracks[0].artist.as_deref(), Some("First; Second")); + assert_eq!(tracks[0].album.as_deref(), Some("Record")); + assert_eq!(tracks[0].year, Some(1999)); + } + + /// An album nobody has is an error, not an empty answer: the + /// review screen would otherwise open on nothing and say nothing. + #[tokio::test] + async fn a_missing_album_is_an_error() { + let pool = pool().await; + assert!(album_identity(&pool, 404).await.is_err()); + } } diff --git a/src-tauri/crates/core/src/metadata/deezer.rs b/src-tauri/crates/core/src/metadata/deezer.rs index e59b2e31..cfb4a5e9 100644 --- a/src-tauri/crates/core/src/metadata/deezer.rs +++ b/src-tauri/crates/core/src/metadata/deezer.rs @@ -354,24 +354,26 @@ impl DeezerClient { const MAX_PAGES: usize = 100; let mut out: Vec = Vec::new(); - for page in 0..MAX_PAGES { + let mut index = 0usize; + for _ in 0..MAX_PAGES { let resp: DeezerSearchResponse = Self::fetch( self.http .get(format!("{BASE_URL}/album/{deezer_id}/tracks")) - .query(&[ - ("index", (page * PAGE).to_string()), - ("limit", PAGE.to_string()), - ]), + .query(&[("index", index.to_string()), ("limit", PAGE.to_string())]), ) .await?; let received = resp.data.len(); out.extend(resp.data); - // Two independent stops, because either alone has been seen - // to lie: a short page is the end whatever `next` says, and - // a full page with no `next` is the end too. - if received < PAGE || resp.next.is_none() { + // `next` is the authority on whether there is more — a page + // shorter than asked for is not the end, and treating it as + // one truncated the listing. The offset advances by what + // actually arrived rather than by the page size, or a short + // page would leave a hole. An empty page stops the walk + // whatever `next` claims, since nothing else would. + if resp.next.is_none() || received == 0 { break; } + index += received; } Ok(out) } diff --git a/src/components/common/TagFetchModal.tsx b/src/components/common/TagFetchModal.tsx index 551e71e9..b745dfee 100644 --- a/src/components/common/TagFetchModal.tsx +++ b/src/components/common/TagFetchModal.tsx @@ -53,11 +53,23 @@ export function TagFetchModal({ onApplied, }: TagFetchModalProps) { const { t } = useTranslation(); - const dialogRef = useModalA11y(isOpen, onClose); + const [busy, setBusy] = useState(false); + /** + * Dismissal, unless a write is in flight. + * + * Apply walks the album one file at a time and closing does not stop + * it: the loop keeps writing into a folder whose review screen is + * gone, and the summary of what was written and what failed is lost + * with it. Every path is routed through here — the X, the footer + * button, the backdrop, and the Escape key `useModalA11y` binds. + */ + const closeUnlessBusy = () => { + if (!busy) onClose(); + }; + const dialogRef = useModalA11y(isOpen, closeUnlessBusy); const [sources, setSources] = useState(null); const [proposals, setProposals] = useState(null); const [accepted, setAccepted] = useState({}); - const [busy, setBusy] = useState(false); const [error, setError] = useState(null); const [applied, setApplied] = useState<{ ok: number; @@ -179,7 +191,7 @@ export function TagFetchModal({ }; return ( - + @@ -242,7 +255,8 @@ export function TagFetchModal({ From 7005743afc6a566c9faf499913d05b78803ccb8b Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 16 Sep 2026 01:43:33 +0200 Subject: [PATCH 07/16] test(tags): artist and album carry a canonical name, and it is NOT NULL --- src-tauri/crates/app/src/commands/tag_fetch.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src-tauri/crates/app/src/commands/tag_fetch.rs b/src-tauri/crates/app/src/commands/tag_fetch.rs index ccf81f3c..febfb4a6 100644 --- a/src-tauri/crates/app/src/commands/tag_fetch.rs +++ b/src-tauri/crates/app/src/commands/tag_fetch.rs @@ -411,8 +411,10 @@ mod tests { sqlx::raw_sql( "INSERT INTO library (id, name, created_at, updated_at) VALUES (1, 'l', 0, 0); - INSERT INTO artist (id, name) VALUES (1, 'Second'), (2, 'First'); - INSERT INTO album (id, title, artist_id) VALUES (1, 'Record', 2); + INSERT INTO artist (id, name, canonical_name) + VALUES (1, 'Second', 'second'), (2, 'First', 'first'); + INSERT INTO album (id, title, canonical_title, artist_id) + VALUES (1, 'Record', 'record', 2); INSERT INTO track (id, library_id, file_path, file_hash, file_size, file_modified, title, duration_ms, added_at, album_id, track_number, year) From be2b9255138120dd08f6e15dd1e57eb409d8eb8f Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 16 Sep 2026 01:50:55 +0200 Subject: [PATCH 08/16] fix(tags): route the two remaining close paths through the busy guard --- src/components/common/TagFetchModal.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/common/TagFetchModal.tsx b/src/components/common/TagFetchModal.tsx index b745dfee..df803e13 100644 --- a/src/components/common/TagFetchModal.tsx +++ b/src/components/common/TagFetchModal.tsx @@ -221,7 +221,7 @@ export function TagFetchModal({
- ))} + {changedAnywhere.map((field) => { + // A toggle with no state tells the reader nothing about what + // the next click will do — they have to infer it from the + // checkboxes below. `aria-pressed` says it outright, and the + // filled style says it to everyone else. + const allOn = columnFullyAccepted(proposals, accepted, field); + return ( + + ); + })}
    @@ -512,7 +540,12 @@ function ReviewList({ @@ -605,6 +638,31 @@ function AppliedSummary({ // Field helpers // ============================================================================= +/** + * Is every field this track would change already accepted? + * + * Takes the map it should judge rather than reading one from a + * closure, so the updaters can ask it about `prev` and the render can + * ask it about the current state — one answer, two callers, no way for + * them to disagree. + */ +function trackFullyAccepted(proposal: TrackProposal, accepted: Accepted) { + const fields = changedFields(proposal); + return ( + fields.length > 0 && fields.every((f) => accepted[proposal.track_id]?.[f]) + ); +} + +/** The same question about one field, across every track that changes it. */ +function columnFullyAccepted( + proposals: AlbumProposals, + accepted: Accepted, + field: TagField, +) { + const rows = proposals.tracks.filter((p) => changedFields(p).includes(field)); + return rows.length > 0 && rows.every((p) => accepted[p.track_id]?.[field]); +} + /** The fields where the catalogue says something different. */ function changedFields(proposal: TrackProposal): TagField[] { if (!proposal.fetched) return []; From 7a3c7132316ccc1c52062ad91efa955925afc250 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 16 Sep 2026 03:42:16 +0200 Subject: [PATCH 16/16] fix(locales): the german hint was missing the noun it selects --- src/i18n/locales/de.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/i18n/locales/de.json b/src/i18n/locales/de.json index 40cd24cc..fc481ea7 100644 --- a/src/i18n/locales/de.json +++ b/src/i18n/locales/de.json @@ -2727,7 +2727,7 @@ "searching": "Suche nach Veröffentlichungen…", "matching": "Titel werden zugeordnet…", "noSources": "Keine Veröffentlichung zu diesem Album gefunden.", - "sourcesHint": "Dasselbe Album gibt es oft in mehreren Veröffentlichungen mit unterschiedlichen Titellisten. Wählen Sie Ihre.", + "sourcesHint": "Dasselbe Album gibt es oft in mehreren Veröffentlichungen mit unterschiedlichen Titellisten. Wählen Sie die passende aus.", "back": "Andere Veröffentlichung wählen", "hint": "Es wird nichts geschrieben, bevor Sie übernehmen.", "acceptColumn": "Ganzes Feld:",