Fix correctness and robustness issues found in a full code review

Correctness:

- MPD command reads no longer abandon a response half-way. The socket's
  500 ms read timeout exists so the idle loop can poll the shutdown flag,
  but it applied to every read, so a slow reply left unread bytes that the
  next command parsed as its own. Add MpdConn::read_line/read_exact, which
  retry across that timeout up to a 15 s deadline, and a desynced flag that
  forces a reconnect when a read is genuinely abandoned. read_response now
  returns Option so a truncated reply is a failure rather than an empty
  result -- a timed-out currentsong used to look like "MPD is stopped".
- resolve_cover returned a covers/<name> path even when the file was
  missing, so a deleted image produced a broken <img> instead of the
  placeholder. Gate the return value on the same existence check as the
  copy list.
- enrich --force no longer runs DELETE FROM album_cache. That discarded
  pin-album entries, MPD-extracted cover paths, and any genre the re-fetch
  then failed to find. Select every album via all_scrobbled_albums and
  overwrite rows in place instead.
- Album grouping, cover fallback and source attribution disagreed: the
  dominant_source subquery and artist_cover matched on album name alone, so
  two records sharing a title cross-contaminated. Extract one
  album_group_key_expr and use it everywhere.
- Escape quotes and backslashes in MusicBrainz Lucene queries; a title
  containing a quote closed the term early and matched nothing.
- Size the HTML cover grid from the period's own limit, so all-time no
  longer shows fewer covers than the table beneath it.
- Fix a panic in the terminal report: top_genres ranks broad single-word
  genres last, so first() is not the busiest row, and a later genre with
  more plays overflowed the bar width. Scale to the true maximum and clamp
  the bar arithmetic.
- Bound readpicture transfers by size and chunk count so a bogus size:
  header cannot drive an unbounded allocation or an endless loop.

Robustness:

- Kill the playerctl children before joining their reader threads. The
  threads block until stdout closes, which only happened because an
  interactive Ctrl+C signals the whole process group; under a supervisor
  that signals one PID, shutdown hung and left orphans.
- Reject --limit and --all-time-limit below 1. SQLite reads a negative
  LIMIT as unlimited, silently dumping the whole library.
- Set a 5 s busy_timeout, so running report against a live watch does not
  fail immediately with "database is locked".
- Replace panics in default_db_path, covers_dir and the enrich queries with
  reported errors, and check PRAGMA table_info for the source column
  migration instead of discarding every ALTER TABLE error.

Cleanups:

- Collapse eight reset_*_timestamps functions into reset_retry_timestamps
  with a RetryScope enum.
- Share one FNV-1a helper (enrich::cover_stem) between the MPD and iTunes
  filename generators.
- Use params_from_iter to remove six duplicated query_map branches.
- Extract source_dot_cell and reuse db::split_genre_list in the report.
- Skip the genre lookup when only covers were requested, saving a request
  and a rate-limit sleep per album.
- Keep the cached genre in pin-album when MusicBrainz returns nothing and a
  --cover-url was supplied.

README.md

@@ -268,13 +268,20 @@ scrbblr report [OPTIONS]
     --db-path <PATH>     Path to the SQLite database
 
 scrbblr enrich [OPTIONS]
-    --online             Fetch metadata and covers from MusicBrainz / iTunes / CAA
+    --pipeline <MODE>    Enrichment stage: mpd, online, or both [default: both]
-    --force              Re-fetch all albums from MusicBrainz (implies --online)
+    --fetch <WHAT>       Online payload: all, covers, or genres [default: all]
-    --retry-covers       Reset the 7-day cooldown for albums missing covers
+    --retry <MODE>       Retry selector: none, covers, genres, all, mpd-genres [default: none]
-    --no-itunes          Skip iTunes; fall back directly to Cover Art Archive
+    --cover-source <SRC> Cover source policy: itunes-caa or caa [default: itunes-caa]
-    --no-mpd-covers      Skip MPD embedded cover extraction
+    --force              Re-fetch all albums from MusicBrainz (online pipeline only)
-    --mpd-host <HOST>    MPD host for cover extraction [default: localhost]
+    --artist <ARTIST>    Limit enrichment to one artist (case-insensitive)
-    --mpd-port <PORT>    MPD port for cover extraction [default: 6600]
+    --mpd-host <HOST>    MPD host for MPD pipeline [default: localhost]
+    --mpd-port <PORT>    MPD port for MPD pipeline [default: 6600]
+    --db-path <PATH>     Path to the SQLite database
+
+scrbblr repair-mpd-covers [OPTIONS]
+    --artist <ARTIST>    Limit repair to one artist (case-insensitive)
+    --mpd-host <HOST>    MPD host for cover validation [default: localhost]
+    --mpd-port <PORT>    MPD port for cover validation [default: 6600]
     --db-path <PATH>     Path to the SQLite database
 
 scrbblr last-scrobble [OPTIONS]
@@ -290,28 +297,36 @@ scrbblr pin-album [OPTIONS]
 
 ### Enrichment (covers + genres)
 
-Enrichment runs in two stages:
+`enrich` has a clean 3-part model:
+
+- `--pipeline` decides *where* to run enrichment (`mpd`, `online`, `both`)
+- `--fetch` decides *what* to fetch online (`covers`, `genres`, `all`)
+- `--retry` decides *which missing rows* should bypass cooldown before online lookup
+
+Defaults are: `--pipeline both --fetch all --retry none`.
+
+The two pipelines are:
 
 **Stage 1 -- MPD embedded covers (offline, no network)**
 
-By default, `enrich` connects to MPD and extracts embedded cover art from your music files using MPD's `readpicture` command. This is fast, works entirely offline, and only processes albums scrobbled via MPD that don't yet have a cover:
+By default (or with `--pipeline mpd`), `enrich` connects to MPD and extracts embedded cover art from your music files using MPD's `readpicture` command. This is fast, works entirely offline, and only processes albums scrobbled via MPD that don't yet have a cover:
 
 ```bash
-scrbblr enrich
+scrbblr enrich --pipeline mpd
 ```
 
 **Stage 2 -- Online lookup (MusicBrainz + iTunes + Cover Art Archive)**
 
-Pass `--online` to also query MusicBrainz for album metadata (MBID, genres) and download covers for albums that still have none after the MPD pass:
+Use the online pipeline to query MusicBrainz for metadata (MBID, genres) and fetch covers:
 
 ```bash
-scrbblr enrich --online
+scrbblr enrich --pipeline online --fetch all
 ```
 
-Cover art is sourced from **iTunes first** (fast, no rate limiting), then the **Cover Art Archive** as fallback. Pass `--no-itunes` to skip iTunes and use only CAA:
+Cover art is sourced from **iTunes first** (fast, no rate limiting), then the **Cover Art Archive** as fallback. To use only CAA:
 
 ```bash
-scrbblr enrich --online --no-itunes
+scrbblr enrich --pipeline online --fetch covers --cover-source caa
 ```
 
 When MusicBrainz matching is tricky, enrichment retries with normalised album variants (strips parenthetical suffixes, progressively shortens trailing words), tries artist aliases for symbol-heavy names, and falls back to recording search before giving up.
@@ -323,18 +338,33 @@ Genre extraction order:
 3. Release-group `genres`
 4. Release-group `tags`
 
-Automatic enrichment (triggered by `report --html`) uses a 7-day retry cooldown for incomplete cache entries (missing cover or genre) so it doesn't hammer MusicBrainz on every report run. Use `--retry-covers` to reset that cooldown and retry albums missing covers immediately:
+Automatic enrichment (triggered by `report --html`) uses a 7-day retry cooldown for incomplete cache entries (missing cover or genre) so it doesn't hammer MusicBrainz on every report run.
+
+Retry missing covers immediately:
 
 ```bash
-scrbblr enrich --online --retry-covers
+scrbblr enrich --pipeline online --retry covers
 ```
 
-Use `--force` to re-fetch all albums from scratch (ignores the cooldown entirely):
+Retry missing genres only for MPD-sourced albums:
 
 ```bash
-scrbblr enrich --online --force
+scrbblr enrich --pipeline online --fetch genres --retry mpd-genres
 ```
 
+You can combine this with `--artist` to target one artist.
+
+Use `--force` to re-fetch all albums from scratch (online pipeline only):
+
+```bash
+scrbblr enrich --pipeline online --fetch all --force
+```
+
+Note: `--force` revisits every scrobbled album and overwrites each cache row in
+place; it does not empty the cache first. An album whose re-fetch turns up
+nothing keeps the data it already had, so covers pinned with `pin-album` and
+covers extracted from MPD survive a forced run.
+
 Genre normalisation notes:
 
 - Genre labels are passed through from MusicBrainz with light cleanup only.
@@ -348,6 +378,26 @@ Downloaded covers are stored in:
 
 `~/.local/share/scrbblr/covers/`
 
+#### Repairing suspect MPD covers (`repair-mpd-covers`)
+
+If you already have cached MPD-local covers (`mpd_*.jpg`) and suspect some are
+incomplete/corrupted, run:
+
+```bash
+scrbblr repair-mpd-covers
+```
+
+This command re-extracts embedded art from MPD for albums with MPD-local cover
+paths in `album_cache`, applies the same resize/re-encode pipeline, and
+compares bytes with the existing file. Missing or mismatched files are
+automatically overwritten.
+
+Target one artist if needed:
+
+```bash
+scrbblr repair-mpd-covers --artist "Deftones"
+```
+
 #### Manually pinning an album (`pin-album`)
 
 Sometimes automatic search fails -- most commonly for classical recordings where

src/enrich.rs

@@ -13,8 +13,8 @@
 //! 2. For each, searches MusicBrainz for matching releases (to get MBID and
 //!    genre/tag data — always done regardless of cover source).
 //! 3. Tries iTunes Search API first for cover art (fast, no rate limit,
-//!    good commercial coverage). Pass `--no-itunes` to skip.
+//!    good commercial coverage). Can be switched to CAA-only mode.
-//! 4. If iTunes has no cover (or `--no-itunes`), falls back to the Cover Art
+//! 4. If iTunes has no cover (or iTunes is disabled), falls back to the Cover Art
 //!    Archive using the MBID from step 2:
 //!    a. The specific release endpoint
 //!    b. The release-group endpoint (covers any edition of the album)
@@ -57,6 +57,7 @@ use image::codecs::jpeg::JpegEncoder;
 use reqwest::blocking::Client;
 use rusqlite::Connection;
 use serde::Deserialize;
+use std::fmt::Write as _;
 use std::fs;
 use std::io::Write;
 use std::path::{Path, PathBuf};
@@ -97,6 +98,52 @@ const MAX_COVER_DIM: u32 = 500;
 /// JPEG quality used when re-encoding resized cover art (0–100).
 const COVER_JPEG_QUALITY: u8 = 85;
 
+/// What the online enrichment stage should fetch.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum OnlineFetchMode {
+    /// Fetch genres and covers.
+    All,
+    /// Fetch covers only.
+    Covers,
+    /// Fetch genres only.
+    Genres,
+}
+
+impl OnlineFetchMode {
+    fn wants_covers(self) -> bool {
+        matches!(self, Self::All | Self::Covers)
+    }
+
+    fn wants_genres(self) -> bool {
+        matches!(self, Self::All | Self::Genres)
+    }
+}
+
+/// Cover source policy for the online stage.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum CoverSourceMode {
+    /// Try iTunes first, then fall back to Cover Art Archive.
+    ItunesThenCaa,
+    /// Skip iTunes and use only Cover Art Archive.
+    CaaOnly,
+}
+
+/// Options controlling online enrichment behaviour.
+#[derive(Debug, Clone, Copy)]
+pub struct OnlineEnrichOptions {
+    pub fetch_mode: OnlineFetchMode,
+    pub cover_source: CoverSourceMode,
+}
+
+impl Default for OnlineEnrichOptions {
+    fn default() -> Self {
+        Self {
+            fetch_mode: OnlineFetchMode::All,
+            cover_source: CoverSourceMode::ItunesThenCaa,
+        }
+    }
+}
+
 // ---------------------------------------------------------------------------
 // MusicBrainz API response types (only the fields we need)
 // ---------------------------------------------------------------------------
@@ -199,14 +246,23 @@ struct MbTag {
 ///
 /// Default: `~/.local/share/scrbblr/covers/`
 /// (respects `$XDG_DATA_HOME`)
-pub fn covers_dir() -> PathBuf {
+///
-    let data_dir = std::env::var("XDG_DATA_HOME").unwrap_or_else(|_| {
+/// Returns an error rather than panicking when neither `$XDG_DATA_HOME` nor
-        let home = std::env::var("HOME").expect("HOME not set");
+/// `$HOME` is set, or the directory cannot be created — cover caching is an
+/// optional convenience and should not take down a `watch` session that is
+/// otherwise scrobbling fine.
+pub fn covers_dir() -> Result<PathBuf, String> {
+    let data_dir = match std::env::var("XDG_DATA_HOME") {
+        Ok(d) if !d.is_empty() => d,
+        _ => {
+            let home = std::env::var("HOME")
+                .map_err(|_| "neither XDG_DATA_HOME nor HOME is set".to_string())?;
             format!("{}/.local/share", home)
-    });
+        }
+    };
     let dir = PathBuf::from(format!("{}/scrbblr/covers", data_dir));
-    fs::create_dir_all(&dir).expect("Failed to create covers directory");
+    fs::create_dir_all(&dir).map_err(|e| format!("could not create {}: {}", dir.display(), e))?;
-    dir
+    Ok(dir)
 }
 
 // ---------------------------------------------------------------------------
@@ -242,7 +298,8 @@ fn search_releases(client: &Client, artist: &str, album: &str) -> Vec<String> {
         }
         let query = format!(
             "artist:\"{}\" AND release:\"{}\"",
-            artist_variants[0], variant
+            lucene_escape(&artist_variants[0]),
+            lucene_escape(variant)
         );
         let results = run_mb_search(client, &query);
         if !results.is_empty() {
@@ -261,7 +318,11 @@ fn search_releases(client: &Client, artist: &str, album: &str) -> Vec<String> {
                 eprintln!("  Retrying with alternate album name: \"{}\"...", variant);
             }
             thread::sleep(RATE_LIMIT_DELAY);
-            let query = format!("artist:\"{}\" AND release:\"{}\"", alt_artist, variant);
+            let query = format!(
+                "artist:\"{}\" AND release:\"{}\"",
+                lucene_escape(alt_artist),
+                lucene_escape(variant)
+            );
             let results = run_mb_search_with_min_score(client, &query, 60);
             if !results.is_empty() {
                 return results;
@@ -284,7 +345,8 @@ fn search_releases(client: &Client, artist: &str, album: &str) -> Vec<String> {
         client,
         &format!(
             "artist:\"{}\" AND recording:\"{}\"",
-            artist_variants[0], recording_query
+            lucene_escape(&artist_variants[0]),
+            lucene_escape(&recording_query)
         ),
         60,
     );
@@ -301,7 +363,8 @@ fn search_releases(client: &Client, artist: &str, album: &str) -> Vec<String> {
             client,
             &format!(
                 "artist:\"{}\" AND recording:\"{}\"",
-                artist_variants[0], shortest
+                lucene_escape(&artist_variants[0]),
+                lucene_escape(shortest)
             ),
             50,
         );
@@ -326,7 +389,7 @@ fn search_releases(client: &Client, artist: &str, album: &str) -> Vec<String> {
             eprintln!("  Retrying with title-only search (no artist constraint)...");
         }
         thread::sleep(RATE_LIMIT_DELAY);
-        let query = format!("release:\"{}\"", variant);
+        let query = format!("release:\"{}\"", lucene_escape(variant));
         let results = run_mb_search_with_min_score(client, &query, 85);
         if !results.is_empty() {
             return results;
@@ -434,6 +497,24 @@ fn normalise_spaces(s: &str) -> String {
     s.split_whitespace().collect::<Vec<_>>().join(" ")
 }
 
+/// Escape a value for use inside a quoted Lucene term in a MusicBrainz query.
+///
+/// Searches are built as `artist:"X" AND release:"Y"`. Without escaping, a
+/// title containing a double quote closes the term early and the rest is
+/// parsed as query syntax, so MusicBrainz rejects the query or silently
+/// matches nothing — precisely on the titles most likely to need the retry
+/// ladder. Inside a quoted phrase Lucene only treats `\` and `"` as special.
+fn lucene_escape(s: &str) -> String {
+    let mut out = String::with_capacity(s.len() + 4);
+    for c in s.chars() {
+        if c == '\\' || c == '"' {
+            out.push('\\');
+        }
+        out.push(c);
+    }
+    out
+}
+
 /// Trim common punctuation/noise from both title ends.
 fn trim_title_edges(s: &str) -> String {
     s.trim_matches(|c: char| {
@@ -601,6 +682,27 @@ fn fetch_release_details(client: &Client, mbid: &str) -> (Option<String>, Option
     (genre, rgid)
 }
 
+/// Fetch just the release-group ID for a release.
+///
+/// Used when the caller wants covers but not genres: the release-group is the
+/// Cover Art Archive's "any edition" fallback key, so it is still needed, but
+/// requesting it alone skips the follow-up genre lookup that
+/// [`fetch_release_details`] may perform.
+fn fetch_release_group_id(client: &Client, mbid: &str) -> Option<String> {
+    let url = format!(
+        "https://musicbrainz.org/ws/2/release/{}?inc=release-groups&fmt=json",
+        mbid
+    );
+
+    let response = client.get(&url).send().ok()?;
+    if !response.status().is_success() {
+        return None;
+    }
+
+    let details: MbReleaseDetails = response.json().ok()?;
+    details.release_group.map(|rg| rg.id)
+}
+
 /// Fetch release-group genres/tags and return a genre string if available.
 fn fetch_release_group_genre(client: &Client, release_group_id: &str) -> Option<String> {
     let url = format!(
@@ -673,11 +775,15 @@ const TAG_BLOCKLIST: &[&str] = &[
 ///
 /// Rejects:
 /// - Tags containing digits (years like "2022", encodings like "iso-8859-1")
+/// - Domain-like tags (review-site names such as "musikexpress.de")
 /// - Known non-genre technical / language tags
 fn is_genre_tag(tag: &str) -> bool {
     if tag.chars().any(|c| c.is_ascii_digit()) {
         return false;
     }
+    if db::looks_like_url(tag) {
+        return false;
+    }
     let lower = tag.to_ascii_lowercase();
     !TAG_BLOCKLIST.contains(&lower.as_str())
 }
@@ -689,7 +795,11 @@ fn is_genre_tag(tag: &str) -> bool {
 /// 2. Community `tags` (filtered to entries with at least one vote and
 ///    passing the genre-tag heuristic to strip technical metadata)
 fn pick_genre_string(genres: Vec<MbGenre>, tags: Vec<MbTag>) -> Option<String> {
-    let mut names: Vec<String> = genres.into_iter().map(|g| g.name).collect();
+    let mut names: Vec<String> = genres
+        .into_iter()
+        .map(|g| g.name)
+        .filter(|n| !db::looks_like_url(n))
+        .collect();
     if names.is_empty() {
         names = tags
             .into_iter()
@@ -716,13 +826,14 @@ fn download_cover_with_fallback(
     mbid: &str,
     release_group_id: Option<&str>,
     covers_dir: &Path,
+    force_redownload: bool,
 ) -> Option<String> {
     // Use the MBID as the filename regardless of which endpoint succeeds,
     // so we have a consistent cache key.
     let dest = covers_dir.join(format!("{}.jpg", mbid));
 
-    // Skip download if we already have the file from a previous run.
+    // Reuse cached file unless force mode explicitly asks for a refresh.
-    if dest.exists() {
+    if should_reuse_existing_cover(dest.exists(), force_redownload) {
         return Some(dest.to_string_lossy().to_string());
     }
 
@@ -746,6 +857,11 @@ fn download_cover_with_fallback(
     None
 }
 
+/// Decide whether an existing cover file should be reused as-is.
+fn should_reuse_existing_cover(dest_exists: bool, force_redownload: bool) -> bool {
+    dest_exists && !force_redownload
+}
+
 /// Decode image bytes, downscale if either dimension exceeds `MAX_COVER_DIM`,
 /// and re-encode as JPEG at `COVER_JPEG_QUALITY`.
 ///
@@ -776,12 +892,20 @@ pub fn resize_cover_bytes(bytes: &[u8]) -> Option<Vec<u8>> {
 // iTunes Search API — cover art fallback
 // ---------------------------------------------------------------------------
 
-/// Generate a stable filename stem for a cover sourced from iTunes.
+/// Generate a stable filename stem for a locally-cached cover.
 ///
-/// Uses the same FNV-1a 64-bit hash approach as the MPD cover extractor,
+/// Uses a 64-bit FNV-1a hash of `"artist\talbum"`, formatted as a 16-char hex
-/// keyed on `"artist\talbum"`, so the filename is deterministic across runs.
+/// string behind `prefix`. This gives a compact, deterministic filename that:
-/// Prefixed with `"itunes_"` to distinguish from MBID-based filenames.
+///
-fn itunes_cover_stem(artist: &str, album: &str) -> String {
+/// - Is stable across runs (same artist+album always maps to the same name).
+/// - Is visually distinct from MBID-based filenames (MBIDs contain hyphens).
+/// - Has negligible collision probability across a typical music library.
+///
+/// `prefix` names the source the cover came from, so the origin of a cached
+/// file is obvious on disk and MPD-local covers can be found again for
+/// revalidation. Example: `cover_stem("mpd", ..)` -> `mpd_a3f7c2b91e045d68`.
+pub fn cover_stem(prefix: &str, artist: &str, album: &str) -> String {
+    // FNV-1a 64-bit hash constants.
     const FNV_OFFSET: u64 = 14695981039346656037;
     const FNV_PRIME: u64 = 1099511628211;
 
@@ -791,7 +915,77 @@ fn itunes_cover_stem(artist: &str, album: &str) -> String {
         hash ^= byte as u64;
         hash = hash.wrapping_mul(FNV_PRIME);
     }
-    format!("itunes_{:016x}", hash)
+    format!("{}_{:016x}", prefix, hash)
+}
+
+/// Build a relaxed normalised key for fuzzy text matching.
+///
+/// Lowercases and keeps only alphanumeric characters and spaces, then
+/// collapses whitespace. This makes matching resilient to punctuation,
+/// symbols, and repeated spacing differences.
+fn normalise_match_key(s: &str) -> String {
+    let mapped: String = s
+        .chars()
+        .map(|c| {
+            if c.is_alphanumeric() || c.is_whitespace() {
+                c
+            } else {
+                ' '
+            }
+        })
+        .collect();
+    normalise_spaces(&mapped.to_ascii_lowercase())
+}
+
+/// Return true when two strings match exactly or one contains the other,
+/// after relaxed normalisation.
+fn loosely_matches(query: &str, candidate: &str) -> bool {
+    let q = normalise_match_key(query);
+    let c = normalise_match_key(candidate);
+    if q.is_empty() || c.is_empty() {
+        return false;
+    }
+    q == c || q.contains(&c) || c.contains(&q)
+}
+
+/// Score an iTunes result for the requested artist/album.
+///
+/// The score prefers exact artist/album matches, but still accepts useful
+/// near-matches (for example "The Division Bell - Single" for album field).
+fn score_itunes_result(result: &serde_json::Value, artist: &str, album: &str) -> Option<i32> {
+    let result_artist = result.get("artistName")?.as_str()?;
+    let result_album = result.get("collectionName")?.as_str()?;
+    let _ = result.get("artworkUrl100")?.as_str()?;
+
+    if !loosely_matches(artist, result_artist) || !loosely_matches(album, result_album) {
+        return None;
+    }
+
+    let artist_q = normalise_match_key(artist);
+    let artist_r = normalise_match_key(result_artist);
+    let album_q = normalise_match_key(album);
+    let album_r = normalise_match_key(result_album);
+
+    let artist_score = if artist_q == artist_r { 4 } else { 2 };
+    let album_score = if album_q == album_r { 4 } else { 2 };
+    Some(artist_score + album_score)
+}
+
+/// Pick the best-matching iTunes artwork URL for the requested artist/album.
+fn pick_best_itunes_cover_url(
+    results: &[serde_json::Value],
+    artist: &str,
+    album: &str,
+) -> Option<String> {
+    let best = results
+        .iter()
+        .filter_map(|r| score_itunes_result(r, artist, album).map(|score| (score, r)))
+        .max_by_key(|(score, _)| *score)?;
+
+    best.1
+        .get("artworkUrl100")?
+        .as_str()
+        .map(|s| s.replace("100x100bb", "600x600bb"))
 }
 
 /// Search the iTunes Store for the front cover of an album.
@@ -818,13 +1012,9 @@ fn fetch_itunes_cover_url(client: &Client, artist: &str, album: &str) -> Option<
     let json: serde_json::Value = response.json().ok()?;
     let results = json.get("results")?.as_array()?;
 
-    // Take the first result's artwork URL and bump the resolution.
+    // Prefer a result whose artist and album actually match the query, so
-    // Apple's URLs end in "100x100bb.jpg" — replace with "600x600bb".
+    // unrelated top hits don't produce obviously wrong covers.
-    results.iter().find_map(|r| {
+    pick_best_itunes_cover_url(results, artist, album)
-        r.get("artworkUrl100")?
-            .as_str()
-            .map(|s| s.replace("100x100bb", "600x600bb"))
-    })
 }
 
 /// Attempt to download an image from the given URL and save it to `dest`.
@@ -861,14 +1051,17 @@ fn try_download(client: &Client, url: &str, dest: &Path) -> Option<String> {
 /// Simple URL encoding for query parameters.
 /// Encodes characters that are not unreserved per RFC 3986.
 fn urlencoded(s: &str) -> String {
-    let mut result = String::with_capacity(s.len() * 2);
+    let mut result = String::with_capacity(s.len() * 3);
-    for c in s.chars() {
+    // Encode straight from the UTF-8 bytes: a multi-byte character is encoded
-        match c {
+    // one byte at a time anyway, and going through `char` would allocate a
-            'A'..='Z' | 'a'..='z' | '0'..='9' | '-' | '_' | '.' | '~' => result.push(c),
+    // fresh `String` per character.
-            _ => {
+    for &byte in s.as_bytes() {
-                for byte in c.to_string().as_bytes() {
+        match byte {
-                    result.push_str(&format!("%{:02X}", byte));
+            b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => {
+                result.push(byte as char)
             }
+            _ => {
+                let _ = write!(result, "%{:02X}", byte);
             }
         }
     }
@@ -887,8 +1080,43 @@ pub fn run_enrich_targeted(
     conn: &Connection,
     needed: &std::collections::HashSet<(String, String)>,
     quiet: bool,
-    no_itunes: bool,
 ) {
+    run_enrich_targeted_with_options(conn, needed, quiet, false, OnlineEnrichOptions::default());
+}
+
+/// Like [`run_enrich_targeted`], but supports force refresh for the provided
+/// subset.
+pub fn run_enrich_targeted_with_options(
+    conn: &Connection,
+    needed: &std::collections::HashSet<(String, String)>,
+    quiet: bool,
+    force: bool,
+    options: OnlineEnrichOptions,
+) {
+    if needed.is_empty() {
+        if !quiet {
+            eprintln!("No matching albums to enrich.");
+        }
+        return;
+    }
+
+    if force {
+        let mut albums: Vec<db::UncachedAlbum> = needed
+            .iter()
+            .map(|(artist, album)| db::UncachedAlbum {
+                artist: artist.clone(),
+                album: album.clone(),
+            })
+            .collect();
+        albums.sort_by(|a, b| a.artist.cmp(&b.artist).then_with(|| a.album.cmp(&b.album)));
+        eprintln!(
+            "Force mode enabled: refreshing {} selected album(s).",
+            albums.len()
+        );
+        run_enrich_albums(conn, albums, quiet, true, options);
+        return;
+    }
+
     let all_uncached = match db::uncached_albums(conn) {
         Ok(v) => v,
         Err(e) => {
@@ -901,7 +1129,7 @@ pub fn run_enrich_targeted(
         .into_iter()
         .filter(|a| needed.contains(&(a.artist.clone(), a.album.clone())))
         .collect();
-    run_enrich_albums(conn, albums, quiet, no_itunes);
+    run_enrich_albums(conn, albums, quiet, false, options);
 }
 
 /// Run the enrichment process: find all uncached albums and fetch their
@@ -915,15 +1143,35 @@ pub fn run_enrich_targeted(
 /// - `force` — if true, re-fetch all albums even if already cached
 /// - `quiet` — if true, suppress the "nothing to do" hint (used when called
 ///   from `report --html` where the user didn't explicitly ask for enrichment)
-/// - `no_itunes` — if true, skip the iTunes cover lookup and go straight to CAA
+/// - `options` — which metadata to fetch and which cover source to prefer
-pub fn run_enrich(conn: &Connection, force: bool, quiet: bool, no_itunes: bool) {
+pub fn run_enrich_with_options(
-    if force {
+    conn: &Connection,
-        conn.execute("DELETE FROM album_cache", [])
+    force: bool,
-            .expect("Failed to clear album_cache");
+    quiet: bool,
-        eprintln!("Cleared album cache (force mode).");
+    options: OnlineEnrichOptions,
+) {
+    // Force mode selects every scrobbled album instead of clearing the cache
+    // first. `DELETE FROM album_cache` would also discard albums pinned by
+    // hand with `pin-album` (which exist precisely because automatic search
+    // cannot find them) and the paths of covers extracted from MPD, orphaning
+    // those files on disk — and anything the re-fetch then failed to find
+    // would be lost for good. Each row is instead overwritten in place by
+    // `upsert_album_cache`, so a failed lookup leaves the old value standing.
+    let albums = if force {
+        eprintln!("Force mode enabled: refreshing existing cover files when re-fetching.");
+        db::all_scrobbled_albums(conn)
+    } else {
+        db::uncached_albums(conn)
+    };
+
+    let albums = match albums {
+        Ok(v) => v,
+        Err(e) => {
+            eprintln!("[error] Failed to query albums to enrich: {}", e);
+            return;
         }
-    let albums = db::uncached_albums(conn).expect("Failed to query uncached albums");
+    };
-    run_enrich_albums(conn, albums, quiet, no_itunes);
+    run_enrich_albums(conn, albums, quiet, force, options);
 }
 
 /// Inner enrichment loop shared by `run_enrich` and `run_enrich_targeted`.
@@ -931,7 +1179,8 @@ fn run_enrich_albums(
     conn: &Connection,
     albums: Vec<db::UncachedAlbum>,
     quiet: bool,
-    no_itunes: bool,
+    force_redownload: bool,
+    options: OnlineEnrichOptions,
 ) {
     if albums.is_empty() {
         if !quiet {
@@ -942,12 +1191,22 @@ fn run_enrich_albums(
     }
 
     let client = build_client();
-    let covers = covers_dir();
+    let covers = match covers_dir() {
+        Ok(d) => d,
+        Err(e) => {
+            eprintln!("[error] Cover cache unavailable: {}", e);
+            return;
+        }
+    };
+    let fetch_covers = options.fetch_mode.wants_covers();
+    let fetch_genres = options.fetch_mode.wants_genres();
+    let use_itunes = fetch_covers && matches!(options.cover_source, CoverSourceMode::ItunesThenCaa);
 
     eprintln!("Found {} album(s) to enrich.", albums.len());
 
     let mut success_count = 0;
     let mut cover_count = 0;
+    let mut genre_count = 0;
 
     for (i, album) in albums.iter().enumerate() {
         eprintln!(
@@ -958,6 +1217,12 @@ fn run_enrich_albums(
             album.album
         );
 
+        let existing = db::album_cache_meta(conn, &album.artist, &album.album)
+            .ok()
+            .flatten();
+        let existing_genre = existing.as_ref().and_then(|m| m.genre.clone());
+        let existing_mbid = existing.as_ref().and_then(|m| m.mbid.clone());
+
         // Step 1: Search MusicBrainz for matching releases.
         // Always done — we need the MBID for genre data and as the CAA
         // lookup key, even if we end up getting the cover from iTunes.
@@ -969,13 +1234,15 @@ fn run_enrich_albums(
 
             // Without an MBID we can't query CAA, so iTunes is the only
             // cover option. Use a hash-based filename since there's no MBID.
-            let cover = if no_itunes {
+            let cover = if !fetch_covers {
+                None
+            } else if !use_itunes {
                 eprintln!("  iTunes disabled, no cover available.");
                 None
             } else {
                 eprintln!("  Trying iTunes...");
                 fetch_itunes_cover_url(&client, &album.artist, &album.album).and_then(|art_url| {
-                    let stem = itunes_cover_stem(&album.artist, &album.album);
+                    let stem = cover_stem("itunes", &album.artist, &album.album);
                     let dest = covers.join(format!("{}.jpg", stem));
                     try_download(&client, &art_url, &dest)
                 })
@@ -984,7 +1251,7 @@ fn run_enrich_albums(
             if cover.is_some() {
                 eprintln!("  Cover downloaded (from iTunes).");
                 cover_count += 1;
-            } else if !no_itunes {
+            } else if fetch_covers && use_itunes {
                 eprintln!("  No cover art available from any source.");
             }
 
@@ -992,9 +1259,9 @@ fn run_enrich_albums(
             let entry = AlbumCacheEntry {
                 artist: album.artist.clone(),
                 album: album.album.clone(),
-                musicbrainz_id: None,
+                musicbrainz_id: existing_mbid,
                 cover_url: cover,
-                genre: None,
+                genre: existing_genre,
                 fetched_at: now_str(),
             };
             match db::upsert_album_cache(conn, &entry) {
@@ -1008,19 +1275,32 @@ fn run_enrich_albums(
         let primary_mbid = &candidates[0];
         eprintln!("  Found MBID: {}", primary_mbid);
 
-        // Step 2: Fetch genres/tags and release-group ID from the primary release.
+        // Step 2: Fetch genres/tags and release-group ID from the primary
+        // release. Skip the genre half of the work when only covers were asked
+        // for: the release-group ID is still needed as a CAA fallback key, but
+        // chasing genres would cost an extra request and rate-limit sleep per
+        // album for a value that is then thrown away.
+        let (fetched_genre, release_group_id) = if fetch_genres {
             thread::sleep(RATE_LIMIT_DELAY);
-        let (genre, release_group_id) = fetch_release_details(&client, primary_mbid);
+            fetch_release_details(&client, primary_mbid)
-        if let Some(ref g) = genre {
+        } else if fetch_covers {
+            thread::sleep(RATE_LIMIT_DELAY);
+            (None, fetch_release_group_id(&client, primary_mbid))
+        } else {
+            (None, None)
+        };
+
+        if let Some(ref g) = fetched_genre {
+            if fetch_genres {
+                genre_count += 1;
+            }
             eprintln!("  Genres: {}", g);
         }
 
         // Step 3: Try iTunes first — it's fast, has no rate limit, and has
         // good commercial coverage. Reuse the MBID-based filename so the file
         // is still associated with the correct release regardless of source.
-        let mut cover = if no_itunes {
+        let mut cover = if fetch_covers && use_itunes {
-            None
-        } else {
             let itunes_cover = fetch_itunes_cover_url(&client, &album.artist, &album.album)
                 .and_then(|art_url| {
                     let dest = covers.join(format!("{}.jpg", primary_mbid));
@@ -1030,13 +1310,15 @@ fn run_enrich_albums(
                 eprintln!("  Cover downloaded (from iTunes).");
             }
             itunes_cover
+        } else {
+            None
         };
 
         // Step 4: If iTunes had no cover, fall back to the Cover Art Archive.
         // CAA is better for obscure or non-commercial releases, and has the
         // highest possible resolution for releases it does have.
-        if cover.is_none() {
+        if fetch_covers && cover.is_none() {
-            if !no_itunes {
+            if use_itunes {
                 eprintln!("  No iTunes cover. Trying Cover Art Archive...");
             }
 
@@ -1047,6 +1329,7 @@ fn run_enrich_albums(
                 primary_mbid,
                 release_group_id.as_deref(),
                 &covers,
+                force_redownload,
             );
 
             // If still nothing, try alternate candidate releases from the search.
@@ -1070,17 +1353,23 @@ fn run_enrich_albums(
 
         if cover.is_some() {
             cover_count += 1;
-        } else {
+        } else if fetch_covers {
             eprintln!("  No cover art available from any source.");
         }
 
+        let genre_to_store = if fetch_genres {
+            fetched_genre.or(existing_genre)
+        } else {
+            existing_genre
+        };
+
         // Step 5: Store the result in album_cache.
         let entry = AlbumCacheEntry {
             artist: album.artist.clone(),
             album: album.album.clone(),
             musicbrainz_id: Some(primary_mbid.clone()),
             cover_url: cover,
-            genre,
+            genre: genre_to_store,
             fetched_at: now_str(),
         };
 
@@ -1094,7 +1383,12 @@ fn run_enrich_albums(
     eprintln!("Enrichment complete:");
     eprintln!("  Albums processed: {}", albums.len());
     eprintln!("  Successfully cached: {}", success_count);
+    if fetch_covers {
         eprintln!("  Covers downloaded: {}", cover_count);
+    }
+    if fetch_genres {
+        eprintln!("  Albums with genres fetched: {}", genre_count);
+    }
     eprintln!("  Covers directory: {}", covers.display());
 }
 
@@ -1109,8 +1403,24 @@ pub fn enrich_by_mbid(
     mbid: &str,
     cover_url_override: Option<&str>,
 ) {
+    // Reject MBIDs that contain path-traversal sequences or directory
+    // separators — a valid UUID contains only hex digits and hyphens.
+    if mbid.contains("..") || mbid.contains('/') || mbid.contains('\\') {
+        eprintln!(
+            "[error] Invalid MBID '{}': must not contain path components.",
+            mbid
+        );
+        return;
+    }
+
     let client = build_client();
-    let covers = covers_dir();
+    let covers = match covers_dir() {
+        Ok(d) => d,
+        Err(e) => {
+            eprintln!("[error] Cover cache unavailable: {}", e);
+            return;
+        }
+    };
 
     eprintln!("Fetching release details for MBID {}...", mbid);
     let (genre, release_group_id) = fetch_release_details(&client, mbid);
@@ -1151,8 +1461,13 @@ pub fn enrich_by_mbid(
             );
             return;
         }
-        let result =
+        let result = download_cover_with_fallback(
-            download_cover_with_fallback(&client, mbid, release_group_id.as_deref(), &covers);
+            &client,
+            mbid,
+            release_group_id.as_deref(),
+            &covers,
+            false,
+        );
         if result.is_some() {
             eprintln!("  Cover downloaded.");
         } else {
@@ -1161,6 +1476,17 @@ pub fn enrich_by_mbid(
         result
     };
 
+    // `upsert_album_cache` overwrites `genre` unconditionally. When the caller
+    // supplied `--cover-url` and MusicBrainz returned nothing, writing None
+    // would wipe a perfectly good genre that a previous run had found, so keep
+    // whatever is already cached in that case.
+    let genre = genre.or_else(|| {
+        db::album_cache_meta(conn, artist, album)
+            .ok()
+            .flatten()
+            .and_then(|m| m.genre)
+    });
+
     let entry = AlbumCacheEntry {
         artist: artist.to_string(),
         album: album.to_string(),
@@ -1212,6 +1538,37 @@ mod tests {
         assert_eq!(urlencoded("A-Z_a-z.0~9"), "A-Z_a-z.0~9");
     }
 
+    #[test]
+    fn test_lucene_escape_quotes_and_backslashes() {
+        assert_eq!(lucene_escape("Purple Rain"), "Purple Rain");
+        // A quote inside a title would otherwise close the quoted term early
+        // and turn the remainder into query syntax.
+        assert_eq!(lucene_escape("Say \"Hello\""), "Say \\\"Hello\\\"");
+        assert_eq!(lucene_escape("AC\\DC"), "AC\\\\DC");
+        // Colons are meaningful outside a quoted phrase but not inside one,
+        // so they are left alone.
+        assert_eq!(lucene_escape("Vivaldi: Gloria"), "Vivaldi: Gloria");
+    }
+
+    #[test]
+    fn test_cover_stem_prefix_and_stability() {
+        let a = cover_stem("mpd", "Bonobo", "Black Sands");
+        assert_eq!(a, cover_stem("mpd", "Bonobo", "Black Sands"));
+        assert!(a.starts_with("mpd_"), "stem = {}", a);
+        assert_eq!(a.len(), "mpd_".len() + 16);
+
+        // The prefix distinguishes sources without changing the hash.
+        let b = cover_stem("itunes", "Bonobo", "Black Sands");
+        assert_eq!(
+            a.trim_start_matches("mpd_"),
+            b.trim_start_matches("itunes_")
+        );
+
+        // Different albums must not collide.
+        assert_ne!(a, cover_stem("mpd", "Bonobo", "The North Borders"));
+        assert_ne!(a, cover_stem("mpd", "Deftones", "Black Sands"));
+    }
+
     #[test]
     fn test_strip_bracketed_segments() {
         assert_eq!(strip_bracketed_segments("If (Killing Eve)"), "If");
@@ -1254,6 +1611,67 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_pick_best_itunes_cover_url_prefers_matching_artist_album() {
+        let results = vec![
+            serde_json::json!({
+                "artistName": "Amiteve",
+                "collectionName": "The Division Bell - Single",
+                "artworkUrl100": "https://example.com/amiteve/100x100bb.jpg"
+            }),
+            serde_json::json!({
+                "artistName": "Pink Floyd",
+                "collectionName": "The Division Bell",
+                "artworkUrl100": "https://example.com/pinkfloyd/100x100bb.jpg"
+            }),
+        ];
+
+        let chosen = pick_best_itunes_cover_url(&results, "Pink Floyd", "The Division Bell");
+        assert_eq!(
+            chosen.as_deref(),
+            Some("https://example.com/pinkfloyd/600x600bb.jpg")
+        );
+    }
+
+    #[test]
+    fn test_pick_best_itunes_cover_url_rejects_wrong_artist() {
+        let results = vec![serde_json::json!({
+            "artistName": "Amiteve",
+            "collectionName": "The Division Bell - Single",
+            "artworkUrl100": "https://example.com/amiteve/100x100bb.jpg"
+        })];
+
+        let chosen = pick_best_itunes_cover_url(&results, "Pink Floyd", "The Division Bell");
+        assert!(chosen.is_none());
+    }
+
+    #[test]
+    fn test_pick_best_itunes_cover_url_accepts_album_suffix_variant() {
+        let results = vec![serde_json::json!({
+            "artistName": "Pink Floyd",
+            "collectionName": "The Division Bell - Single",
+            "artworkUrl100": "https://example.com/pinkfloyd-single/100x100bb.jpg"
+        })];
+
+        let chosen = pick_best_itunes_cover_url(&results, "Pink Floyd", "The Division Bell");
+        assert_eq!(
+            chosen.as_deref(),
+            Some("https://example.com/pinkfloyd-single/600x600bb.jpg")
+        );
+    }
+
+    #[test]
+    fn test_should_reuse_existing_cover_default_mode() {
+        assert!(should_reuse_existing_cover(true, false));
+        assert!(!should_reuse_existing_cover(false, false));
+    }
+
+    #[test]
+    fn test_should_reuse_existing_cover_force_mode() {
+        assert!(!should_reuse_existing_cover(true, true));
+        assert!(!should_reuse_existing_cover(false, true));
+    }
+
     #[test]
     fn test_pick_genre_string_prefers_genres_then_tags() {
         let genre = pick_genre_string(
@@ -1283,6 +1701,43 @@ mod tests {
         assert_eq!(from_tags.as_deref(), Some("electronic"));
     }
 
+    #[test]
+    fn test_pick_genre_string_rejects_domain_tags() {
+        // Review-site tags such as "musikexpress.de" must never be picked.
+        let from_tags = pick_genre_string(
+            vec![],
+            vec![
+                MbTag {
+                    name: "musikexpress.de".to_string(),
+                    count: 3,
+                },
+                MbTag {
+                    name: "rollingstone.de".to_string(),
+                    count: 2,
+                },
+                MbTag {
+                    name: "shoegaze".to_string(),
+                    count: 1,
+                },
+            ],
+        );
+        assert_eq!(from_tags.as_deref(), Some("shoegaze"));
+
+        // Domains must also be stripped from the curated-genres path.
+        let from_genres = pick_genre_string(
+            vec![
+                MbGenre {
+                    name: "pitchfork.com".to_string(),
+                },
+                MbGenre {
+                    name: "dream pop".to_string(),
+                },
+            ],
+            vec![],
+        );
+        assert_eq!(from_genres.as_deref(), Some("dream pop"));
+    }
+
     #[test]
     fn test_uncached_albums_query() {
         // Verify the uncached_albums query works correctly.

src/main.rs

@@ -1,12 +1,14 @@
 //! scrbblr — a local music scrobbler for MPRIS and MPD on Linux.
 //!
-//! This is the CLI entry point. It provides five subcommands:
+//! This is the CLI entry point. It provides six subcommands:
 //!
 //! - `watch` — monitors a player via `playerctl` and/or MPD, recording
 //!   scrobbles to SQLite. Both sources can run simultaneously.
 //! - `report` — generates listening statistics from the stored data
 //! - `enrich` — fetches album art and genre info from MusicBrainz,
 //!   and/or extracts embedded covers from MPD
+//! - `repair-mpd-covers` — revalidate MPD-local cached covers and repair
+//!   any mismatches by re-fetching from MPD
 //! - `last-scrobble`— prints the newest scrobble timestamp
 //! - `pin-album`    — manually assign a MusicBrainz ID to an album
 //!
@@ -53,7 +55,7 @@ mod mpd;
 mod report;
 mod watcher;
 
-use clap::{Parser, Subcommand};
+use clap::{Parser, Subcommand, ValueEnum};
 use std::io::BufRead;
 use std::process::{Command, Stdio};
 use std::sync::atomic::{AtomicBool, Ordering};
@@ -75,6 +77,53 @@ struct Cli {
     command: Commands,
 }
 
+#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)]
+#[value(rename_all = "kebab-case")]
+enum EnrichPipeline {
+    Mpd,
+    Online,
+    Both,
+}
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)]
+#[value(rename_all = "kebab-case")]
+enum EnrichFetch {
+    All,
+    Covers,
+    Genres,
+}
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)]
+#[value(rename_all = "kebab-case")]
+enum EnrichRetry {
+    None,
+    Covers,
+    Genres,
+    All,
+    MpdGenres,
+}
+
+impl EnrichRetry {
+    /// The database-level scope this retry mode selects, or `None` when the
+    /// normal cooldown should be left alone.
+    fn scope(self) -> Option<db::RetryScope> {
+        match self {
+            Self::None => None,
+            Self::Covers => Some(db::RetryScope::Covers),
+            Self::Genres => Some(db::RetryScope::Genres),
+            Self::All => Some(db::RetryScope::All),
+            Self::MpdGenres => Some(db::RetryScope::MpdGenres),
+        }
+    }
+}
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)]
+#[value(rename_all = "kebab-case")]
+enum CoverSource {
+    ItunesCaa,
+    Caa,
+}
+
 #[derive(Subcommand)]
 enum Commands {
     /// Watch playerctl metadata and MPD, scrobbling tracks to the local database.
@@ -162,52 +211,76 @@ enum Commands {
     },
     /// Fetch album art and genre info for all scrobbled albums.
     ///
-    /// By default, connects to MPD and extracts embedded cover art from music
+    /// By default, runs both stages:
-    /// files via `readpicture` — fully offline, no network access required.
-    /// Pass `--no-mpd-covers` to skip this step.
     ///
-    /// Pass `--online` to also query MusicBrainz for album metadata (MBID,
+    /// 1. MPD embedded-cover extraction (`readpicture`, offline)
-    /// genre) and download cover art. Cover art is sourced from iTunes first
+    /// 2. Online metadata/cover enrichment (MusicBrainz + iTunes/CAA)
-    /// (fast, no rate limit), then the Cover Art Archive as fallback. Pass
+    ///
-    /// `--no-itunes` to skip iTunes and use only the Cover Art Archive.
+    /// Use `--pipeline`, `--fetch`, and `--retry` to narrow the behaviour.
     Enrich {
-        /// Query MusicBrainz for album metadata (MBID, genre) and download
+        /// Which enrichment stage(s) to run.
-        /// cover art from the Cover Art Archive for albums that still have
+        #[arg(long, value_enum, default_value = "both")]
-        /// no cover after local extraction.
+        pipeline: EnrichPipeline,
-        #[arg(long)]
+
-        online: bool,
+        /// Which metadata to fetch during the online stage.
+        #[arg(long, value_enum, default_value = "all")]
+        fetch: EnrichFetch,
+
+        /// Reset cooldown before online enrichment.
+        ///
+        /// - `none`: keep normal 7-day cooldown
+        /// - `covers`: retry cover-missing rows
+        /// - `genres`: retry genre-missing rows
+        /// - `all`: retry rows missing either cover or genre
+        /// - `mpd-genres`: retry genre-missing rows only for MPD-sourced albums
+        #[arg(long, value_enum, default_value = "none")]
+        retry: EnrichRetry,
+
+        /// Cover source policy for the online stage.
+        #[arg(long, value_enum, default_value = "itunes-caa")]
+        cover_source: CoverSource,
 
         /// Re-fetch metadata for all albums from MusicBrainz, even those
-        /// already cached. Only meaningful together with `--online`.
+        /// already cached. Only meaningful when `--pipeline` includes `online`.
         #[arg(long)]
         force: bool,
 
-        /// Re-attempt cover downloads for albums that have metadata cached
+        /// Limit enrichment to albums by this artist (case-insensitive match).
-        /// but no cover art yet. Resets the 7-day cooldown for those albums
+        ///
-        /// only, so the next `--online` run retries them. Does not affect
+        /// Useful when fixing one artist's covers/metadata without touching
-        /// albums that already have a cover.
+        /// the rest of the library. With `--force`, only this artist's albums
+        /// are re-fetched.
         #[arg(long)]
-        retry_covers: bool,
+        artist: Option<String>,
-
-        /// Skip the iTunes Search API when fetching cover art. Falls back
-        /// directly to the Cover Art Archive (CAA). Useful if you prefer
-        /// open-data sources or iTunes results are poor for your library.
-        #[arg(long)]
-        no_itunes: bool,
-
-        /// Disable the MPD embedded cover extraction step. By default,
-        /// `enrich` connects to MPD and extracts embedded cover art from
-        /// music files via `readpicture` before doing anything else.
-        #[arg(long)]
-        no_mpd_covers: bool,
 
         /// MPD server hostname, IP address, or Unix socket path.
-        /// Used for embedded cover extraction (unless `--no-mpd-covers`).
+        /// Used for MPD extraction when `--pipeline` includes `mpd`.
         #[arg(long, default_value = "localhost")]
         mpd_host: String,
 
-        /// MPD server TCP port. Used for embedded cover extraction when
+        /// MPD server TCP port. Ignored for Unix socket paths.
-        /// connecting via TCP (ignored for Unix socket paths).
+        #[arg(long, default_value = "6600")]
+        mpd_port: u16,
+
+        /// Path to the SQLite database file. Same default as `watch`.
+        #[arg(long)]
+        db_path: Option<String>,
+    },
+    /// Revalidate cached MPD-local covers and automatically repair mismatches.
+    ///
+    /// This checks albums whose `cover_url` points to an MPD-generated local
+    /// file (`mpd_*.jpg`), re-extracts embedded art from MPD, and compares the
+    /// processed bytes. Missing or mismatched files are overwritten.
+    RepairMpdCovers {
+        /// Limit revalidation to albums by this artist (case-insensitive match).
+        #[arg(long)]
+        artist: Option<String>,
+
+        /// MPD server hostname, IP address, or Unix socket path.
+        #[arg(long, default_value = "localhost")]
+        mpd_host: String,
+
+        /// MPD server TCP port. Ignored for Unix socket paths.
         #[arg(long, default_value = "6600")]
         mpd_port: u16,
 
@@ -259,13 +332,28 @@ enum Commands {
 /// Falls back to: ~/.local/share/scrbblr/scrobbles.db
 ///
 /// Creates the parent directory if it doesn't exist.
+/// Exits with a diagnostic instead of panicking when the location cannot be
+/// determined or created — an unset `$HOME` is a misconfiguration to report,
+/// not a bug to dump a backtrace for. Pass `--db-path` to bypass this.
 fn default_db_path() -> String {
-    let data_dir = std::env::var("XDG_DATA_HOME").unwrap_or_else(|_| {
+    let data_dir = match std::env::var("XDG_DATA_HOME") {
-        let home = std::env::var("HOME").expect("HOME not set");
+        Ok(d) if !d.is_empty() => d,
-        format!("{}/.local/share", home)
+        _ => match std::env::var("HOME") {
-    });
+            Ok(home) => format!("{}/.local/share", home),
+            Err(_) => {
+                eprintln!(
+                    "Cannot determine the database location: neither XDG_DATA_HOME \
+                     nor HOME is set. Pass --db-path explicitly."
+                );
+                std::process::exit(1);
+            }
+        },
+    };
     let dir = format!("{}/scrbblr", data_dir);
-    std::fs::create_dir_all(&dir).expect("Failed to create data directory");
+    if let Err(e) = std::fs::create_dir_all(&dir) {
+        eprintln!("Failed to create data directory {}: {}", dir, e);
+        std::process::exit(1);
+    }
     format!("{}/scrobbles.db", dir)
 }
 
@@ -324,13 +412,21 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option<mpd::MpdConfig>,
     let (tx, rx) = mpsc::channel::<watcher::Event>();
 
     // Track how many reader threads have sent Eof so we know when both
-    // playerctl processes have ended. Set to 2 when MPRIS is disabled so the
+    // playerctl processes have ended. Set to 0 when MPRIS is disabled so the
-    // main loop exits immediately on Ctrl+C (no processes to wait for).
+    // first Eof (from the Ctrl+C handler) ends the loop immediately — there
+    // are no reader threads to wait for in that case.
     let expected_eofs: usize = if use_mpris { 2 } else { 0 };
 
     let mut meta_handle = None;
     let mut status_handle = None;
 
+    // Live playerctl children. They are killed explicitly on shutdown: their
+    // reader threads block in `read_line` until stdout closes, so joining them
+    // without terminating the child first would hang. An interactive Ctrl+C
+    // happens to signal the whole process group, but a supervisor that signals
+    // only our PID (systemd, a wrapper script) would not.
+    let mut children: Vec<std::process::Child> = Vec::new();
+
     if use_mpris {
         // --- Spawn playerctl metadata follower ---
         // Outputs one tab-separated line per track change:
@@ -357,15 +453,27 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option<mpd::MpdConfig>,
             .spawn();
 
         match (metadata_proc, status_proc) {
-            (Ok(meta_proc), Ok(stat_proc)) => {
+            (Ok(mut meta_proc), Ok(mut stat_proc)) => {
+                // Take the pipes out of the children so the reader threads own
+                // the streams while we retain the handles needed to kill them.
+                let meta_stdout = meta_proc
+                    .stdout
+                    .take()
+                    .expect("No stdout for metadata process");
+                let stat_stdout = stat_proc
+                    .stdout
+                    .take()
+                    .expect("No stdout for status process");
+                children.push(meta_proc);
+                children.push(stat_proc);
+
                 // --- Metadata reader thread ---
                 // Reads lines from the metadata process stdout, parses them into
                 // `Event::Metadata` values, and sends them through the channel.
                 // Sends `Event::Eof` when the process exits (stdout closes).
                 let tx_meta = tx.clone();
                 meta_handle = Some(thread::spawn(move || {
-                    let stdout = meta_proc.stdout.expect("No stdout for metadata process");
+                    let reader = std::io::BufReader::new(meta_stdout);
-                    let reader = std::io::BufReader::new(stdout);
                     for line in reader.lines() {
                         match line {
                             Ok(l) => {
@@ -385,8 +493,7 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option<mpd::MpdConfig>,
                 // Same pattern as the metadata reader, but parses status lines.
                 let tx_status = tx.clone();
                 status_handle = Some(thread::spawn(move || {
-                    let stdout = stat_proc.stdout.expect("No stdout for status process");
+                    let reader = std::io::BufReader::new(stat_stdout);
-                    let reader = std::io::BufReader::new(stdout);
                     for line in reader.lines() {
                         match line {
                             Ok(l) => {
@@ -402,12 +509,19 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option<mpd::MpdConfig>,
                     let _ = tx_status.send(watcher::Event::Eof);
                 }));
             }
-            (meta_result, _) => {
+            (meta_result, status_result) => {
                 // At least one spawn failed — MPRIS watcher cannot run.
-                // Print the error and send synthetic Eofs so the main loop
+                // Print the error, reap whichever process did start so it is
+                // not left orphaned, and send synthetic Eofs so the main loop
                 // below exits cleanly; the MPD watcher (if any) continues.
-                if let Err(e) = meta_result {
+                for result in [meta_result, status_result] {
-                    eprintln!("[warn] Failed to spawn playerctl: {}", e);
+                    match result {
+                        Ok(mut child) => {
+                            let _ = child.kill();
+                            let _ = child.wait();
+                        }
+                        Err(e) => eprintln!("[warn] Failed to spawn playerctl: {}", e),
+                    }
                 }
                 eprintln!("[warn] MPRIS watcher will be inactive.");
                 let _ = tx.send(watcher::Event::Eof);
@@ -465,26 +579,53 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option<mpd::MpdConfig>,
         tracker.handle_event(watcher::Event::Eof);
     }
 
+    // Terminate the playerctl children. `playerctl --follow` never exits on
+    // its own, so its stdout would stay open and the reader threads below
+    // would block forever. Do this before any join.
+    for mut child in children {
+        let _ = child.kill();
+        let _ = child.wait();
+    }
+
     // Wait for the MPD watcher thread to finish its graceful shutdown.
     // It will notice `running == false` on the next idle timeout (≤ 500 ms).
     if let Some(handle) = mpd_handle {
         let _ = handle.join();
     }
 
-    eprintln!("Goodbye.");
+    // Wait for MPRIS reader threads to finish. Their pipes are now closed.
-
-    // Wait for MPRIS reader threads to finish.
     if let Some(h) = meta_handle {
         let _ = h.join();
     }
     if let Some(h) = status_handle {
         let _ = h.join();
     }
+
+    eprintln!("Goodbye.");
 }
 
-/// Round a value to the nearest multiple of 5.
+/// Round a non-negative value to the nearest multiple of 5.
+///
+/// Only ever called with a validated positive `--limit`, so integer division
+/// truncating toward zero is not a concern here.
 fn round_to_5(n: i64) -> i64 {
-    ((n + 2) / 5) * 5
+    ((n.max(0) + 2) / 5) * 5
+}
+
+/// Everything [`run_report`] needs, bundled so the call stays readable.
+struct ReportOptions<'a> {
+    period: &'a str,
+    json: bool,
+    html: bool,
+    /// Output directory for `--html`. `None` prints to stdout.
+    output: Option<&'a str>,
+    /// Top-N size for the per-period sections.
+    limit: i64,
+    /// Top-N size for the all-time sections.
+    all_time_limit: i64,
+    no_enrich: bool,
+    mpd_cfg: &'a mpd::MpdConfig,
+    db_path: &'a str,
 }
 
 // ---------------------------------------------------------------------------
@@ -501,17 +642,26 @@ fn round_to_5(n: i64) -> i64 {
 /// - **HTML** (`--html`): generates a multi-period report (Today / Week / Month /
 ///   All Time) with bar charts and album cover art.
 ///   With `--output <dir>`, writes `index.html` + `covers/` to a directory.
-fn run_report(
+fn run_report(opts: &ReportOptions<'_>) {
-    period: &str,
+    let ReportOptions {
-    json: bool,
+        period,
-    html: bool,
+        json,
-    output: Option<&str>,
+        html,
-    limit: i64,
+        output,
-    all_time_limit: i64,
+        limit,
-    no_enrich: bool,
+        all_time_limit,
-    mpd_cfg: &mpd::MpdConfig,
+        no_enrich,
-    db_path: &str,
+        mpd_cfg,
-) {
+        db_path,
+    } = *opts;
+
+    // Validate output-format flags before doing any work, so an obvious
+    // mistake fails immediately rather than after opening the database.
+    if json && html {
+        eprintln!("Please choose one output format: either --json or --html.");
+        std::process::exit(1);
+    }
+
     let conn = match db::open_db(db_path) {
         Ok(c) => c,
         Err(e) => {
@@ -531,11 +681,6 @@ fn run_report(
         std::process::exit(1);
     }
 
-    if json && html {
-        eprintln!("Please choose one output format: either --json or --html.");
-        std::process::exit(1);
-    }
-
     // Cover enrichment is only meaningful for HTML reports — terminal and JSON
     // output never displays cover art, so there is nothing to gain from
     // extracting or downloading covers for those modes.
@@ -556,7 +701,7 @@ fn run_report(
         // iTunes is tried first (fast, no rate limit), then CAA as fallback.
         // Respects the 7-day cooldown so we don't hammer MusicBrainz on every
         // report run.
-        enrich::run_enrich_targeted(&conn, &needed, true, false);
+        enrich::run_enrich_targeted(&conn, &needed, true);
     }
 
     if html {
@@ -696,42 +841,74 @@ fn main() {
             mpd_port,
             db_path,
         } => {
+            // Both limits are interpolated directly into SQL `LIMIT` clauses.
+            // They are plain integers so there is no injection risk, but SQLite
+            // treats a negative LIMIT as "no limit", which would silently dump
+            // the entire library instead of the requested top-N.
+            if limit < 1 {
+                eprintln!("--limit must be at least 1 (got {}).", limit);
+                std::process::exit(1);
+            }
+            if let Some(atl) = all_time_limit
+                && atl < 1
+            {
+                eprintln!("--all-time-limit must be at least 1 (got {}).", atl);
+                std::process::exit(1);
+            }
+
             let path = db_path.unwrap_or_else(default_db_path);
-            let atl =
+            let atl = all_time_limit
-                all_time_limit.unwrap_or_else(|| round_to_5((limit as f64 * 2.5).round() as i64));
+                .unwrap_or_else(|| round_to_5((limit as f64 * 2.5).round() as i64))
+                .max(1);
             let mpd_cfg = mpd::MpdConfig {
                 host: mpd_host,
                 port: mpd_port,
             };
-            run_report(
+            run_report(&ReportOptions {
-                &period,
+                period: &period,
                 json,
                 html,
-                output.as_deref(),
+                output: output.as_deref(),
                 limit,
-                atl,
+                all_time_limit: atl,
                 no_enrich,
-                &mpd_cfg,
+                mpd_cfg: &mpd_cfg,
-                &path,
+                db_path: &path,
-            );
+            });
         }
         Commands::Enrich {
-            online,
+            pipeline,
+            fetch,
+            retry,
+            cover_source,
             force,
-            retry_covers,
+            artist,
-            no_itunes,
-            no_mpd_covers,
             mpd_host,
             mpd_port,
             db_path,
         } => {
-            if no_mpd_covers && !online {
+            let run_mpd = matches!(pipeline, EnrichPipeline::Mpd | EnrichPipeline::Both);
-                eprintln!("Nothing to do. Pass --online and/or omit --no-mpd-covers.");
+            let run_online = matches!(pipeline, EnrichPipeline::Online | EnrichPipeline::Both);
+            let wants_covers = matches!(fetch, EnrichFetch::All | EnrichFetch::Covers);
+
+            if force && !run_online {
+                eprintln!("--force requires --pipeline online or --pipeline both.");
+                std::process::exit(1);
+            }
+
+            if !matches!(retry, EnrichRetry::None) && !run_online {
+                eprintln!("--retry requires --pipeline online or --pipeline both.");
+                std::process::exit(1);
+            }
+
+            if run_mpd && !wants_covers && !run_online {
                 eprintln!(
-                    "  (default)      Extract embedded covers from music files via MPD (offline)"
+                    "Nothing to do: --pipeline mpd only handles covers, but --fetch {} excludes covers.",
-                );
+                    match fetch {
-                eprintln!(
+                        EnrichFetch::Genres => "genres",
-                    "  --online       Fetch metadata and covers from MusicBrainz / iTunes / CAA"
+                        EnrichFetch::All => "all",
+                        EnrichFetch::Covers => "covers",
+                    }
                 );
                 std::process::exit(0);
             }
@@ -745,32 +922,102 @@ fn main() {
                 }
             };
 
-            // Run MPD cover extraction first (fast, local, no rate limits).
+            let artist_needed = if let Some(ref artist_name) = artist {
-            // Pre-populating cover_url means the online enrichment that follows
+                match db::albums_for_artist(&conn, artist_name) {
-            // will skip fetching covers for albums that already have one.
+                    Ok(v) => {
-            if !no_mpd_covers {
+                        if v.is_empty() {
+                            eprintln!("No scrobbled albums found for artist \"{}\".", artist_name);
+                            return;
+                        }
+                        Some(
+                            v.into_iter()
+                                .map(|a| (a.artist, a.album))
+                                .collect::<std::collections::HashSet<_>>(),
+                        )
+                    }
+                    Err(e) => {
+                        eprintln!(
+                            "[error] Failed to query albums for artist \"{}\": {}",
+                            artist_name, e
+                        );
+                        std::process::exit(1);
+                    }
+                }
+            } else {
+                None
+            };
+
+            // Run MPD cover extraction first (fast, local, no rate limits)
+            // when the selected pipeline includes MPD and the fetch mode asks
+            // for covers.
+            if run_mpd && wants_covers {
                 let mpd_cfg = mpd::MpdConfig {
                     host: mpd_host,
                     port: mpd_port,
                 };
+                if let Some(ref needed) = artist_needed {
+                    mpd::run_mpd_cover_enrich_targeted(&mpd_cfg, &conn, needed);
+                } else {
                     mpd::run_mpd_cover_enrich(&mpd_cfg, &conn);
                 }
+            }
 
-            // Online enrichment: MusicBrainz lookup for MBID + genre, Cover Art
+            if run_online {
-            // Archive for any albums still missing a cover after the MPD pass.
+                // Optional retry selector: reset cooldown timestamps so the
-            // Reset the cooldown for cover-missing albums before the online
+                // online stage revisits selected missing fields immediately.
-            // pass so they are picked up by uncached_albums.
+                if let Some(scope) = retry.scope() {
-            if retry_covers {
+                    match db::reset_retry_timestamps(&conn, scope, artist.as_deref()) {
-                match db::reset_missing_cover_timestamps(&conn) {
+                        Ok(n) => eprintln!(
-                    Ok(n) => eprintln!("Reset cooldown for {} album(s) missing covers.", n),
+                            "Reset cooldown for {} album(s) via --retry {}.",
-                    Err(e) => eprintln!("[warn] Failed to reset cover timestamps: {}", e),
+                            n,
+                            scope.as_str()
+                        ),
+                        Err(e) => eprintln!("[warn] Failed to reset retry timestamps: {}", e),
                     }
                 }
 
-            if online {
+                let cover_source_mode = match cover_source {
-                enrich::run_enrich(&conn, force, false, no_itunes);
+                    CoverSource::ItunesCaa => enrich::CoverSourceMode::ItunesThenCaa,
+                    CoverSource::Caa => enrich::CoverSourceMode::CaaOnly,
+                };
+                let fetch_mode = match fetch {
+                    EnrichFetch::All => enrich::OnlineFetchMode::All,
+                    EnrichFetch::Covers => enrich::OnlineFetchMode::Covers,
+                    EnrichFetch::Genres => enrich::OnlineFetchMode::Genres,
+                };
+                let options = enrich::OnlineEnrichOptions {
+                    fetch_mode,
+                    cover_source: cover_source_mode,
+                };
+
+                if let Some(ref needed) = artist_needed {
+                    enrich::run_enrich_targeted_with_options(&conn, needed, false, force, options);
+                } else {
+                    enrich::run_enrich_with_options(&conn, force, false, options);
                 }
             }
+        }
+        Commands::RepairMpdCovers {
+            artist,
+            mpd_host,
+            mpd_port,
+            db_path,
+        } => {
+            let path = db_path.unwrap_or_else(default_db_path);
+            let conn = match db::open_db(&path) {
+                Ok(c) => c,
+                Err(e) => {
+                    eprintln!("Failed to open database at {}: {}", path, e);
+                    std::process::exit(1);
+                }
+            };
+
+            let mpd_cfg = mpd::MpdConfig {
+                host: mpd_host,
+                port: mpd_port,
+            };
+            mpd::run_mpd_cover_revalidate(&mpd_cfg, &conn, artist.as_deref());
+        }
         Commands::LastScrobble { db_path } => {
             let path = db_path.unwrap_or_else(default_db_path);
             let conn = match db::open_db(&path) {

src/mpd.rs

@@ -73,9 +73,10 @@ use std::io::{self, BufRead, BufReader, Read, Write};
 use std::net::TcpStream;
 #[cfg(unix)]
 use std::os::unix::net::UnixStream;
+use std::path::Path;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Mutex};
-use std::time::Duration;
+use std::time::{Duration, Instant};
 
 use rusqlite::Connection;
 
@@ -128,6 +129,27 @@ impl MpdConfig {
 // Low-level connection
 // ---------------------------------------------------------------------------
 
+/// How long to keep waiting for a command's response before giving up.
+///
+/// The socket's own read timeout is deliberately short (see [`READ_TIMEOUT`])
+/// so the idle loop can poll the shutdown flag. Every other exchange must wait
+/// for the real reply instead: abandoning a response half-read leaves unread
+/// bytes in the socket that the next command would parse as its own reply.
+const RESPONSE_TIMEOUT: Duration = Duration::from_secs(15);
+
+/// Socket read timeout. Short enough that the idle loop notices shutdown
+/// promptly; [`MpdConn`] transparently retries across it for command replies.
+const READ_TIMEOUT: Duration = Duration::from_millis(500);
+
+/// Return true when an I/O error is just the socket's read timeout firing,
+/// rather than a genuine failure.
+fn is_timeout(e: &io::Error) -> bool {
+    matches!(
+        e.kind(),
+        io::ErrorKind::WouldBlock | io::ErrorKind::TimedOut | io::ErrorKind::Interrupted
+    )
+}
+
 /// An open, authenticated connection to MPD.
 ///
 /// Holds a buffered reader for receiving response lines (and binary chunks)
@@ -135,12 +157,96 @@ impl MpdConfig {
 /// the same underlying socket.
 ///
 /// The read half has a short timeout (500 ms) so that the idle loop can
-/// periodically check the shutdown flag without blocking indefinitely.
+/// periodically check the shutdown flag without blocking indefinitely. All
+/// command/response reads go through [`MpdConn::read_line`] and
+/// [`MpdConn::read_exact`], which retry across that timeout so a slow reply is
+/// never mistaken for a finished one.
 struct MpdConn {
     /// Buffered reader wrapping the socket's read half.
     reader: BufReader<Box<dyn Read + Send>>,
     /// Write handle for sending commands.
     writer: Box<dyn Write + Send>,
+    /// Set once we abandon a read part-way through a response. The stream can
+    /// no longer be trusted to be positioned at a message boundary, so the
+    /// connection must be torn down and re-established rather than reused.
+    desynced: bool,
+}
+
+impl MpdConn {
+    /// Read one `\n`-terminated line, retrying while the socket's short read
+    /// timeout fires. Returns `Ok(None)` at EOF.
+    ///
+    /// Partial data accumulated before a timeout is retained and the read
+    /// resumes, so a reply split across timeouts is reassembled correctly.
+    /// Giving up marks the connection desynced.
+    fn read_line(&mut self) -> io::Result<Option<String>> {
+        let deadline = Instant::now() + RESPONSE_TIMEOUT;
+        let mut buf: Vec<u8> = Vec::new();
+        loop {
+            match self.reader.read_until(b'\n', &mut buf) {
+                Ok(_) => {
+                    if buf.last() == Some(&b'\n') {
+                        return Ok(Some(String::from_utf8_lossy(&buf).into_owned()));
+                    }
+                    // No delimiter and no error means EOF.
+                    if buf.is_empty() {
+                        return Ok(None);
+                    }
+                    return Ok(Some(String::from_utf8_lossy(&buf).into_owned()));
+                }
+                Err(ref e) if is_timeout(e) => {
+                    if Instant::now() >= deadline {
+                        self.desynced = true;
+                        return Err(io::Error::new(
+                            io::ErrorKind::TimedOut,
+                            "timed out waiting for MPD response line",
+                        ));
+                    }
+                }
+                Err(e) => {
+                    self.desynced = true;
+                    return Err(e);
+                }
+            }
+        }
+    }
+
+    /// Fill `buf` completely, retrying while the socket's short read timeout
+    /// fires.
+    ///
+    /// `Read::read_exact` cannot be used here: on error it does not report how
+    /// many bytes it consumed, so a timeout mid-chunk would silently swallow
+    /// part of the binary payload. This loop tracks progress itself.
+    fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> {
+        let deadline = Instant::now() + RESPONSE_TIMEOUT;
+        let mut filled = 0;
+        while filled < buf.len() {
+            match self.reader.read(&mut buf[filled..]) {
+                Ok(0) => {
+                    self.desynced = true;
+                    return Err(io::Error::new(
+                        io::ErrorKind::UnexpectedEof,
+                        "MPD closed the connection mid-chunk",
+                    ));
+                }
+                Ok(n) => filled += n,
+                Err(ref e) if is_timeout(e) => {
+                    if Instant::now() >= deadline {
+                        self.desynced = true;
+                        return Err(io::Error::new(
+                            io::ErrorKind::TimedOut,
+                            "timed out reading MPD binary chunk",
+                        ));
+                    }
+                }
+                Err(e) => {
+                    self.desynced = true;
+                    return Err(e);
+                }
+            }
+        }
+        Ok(())
+    }
 }
 
 /// Open a TCP connection to MPD, set a read timeout, and consume the
@@ -153,7 +259,7 @@ fn connect_tcp(config: &MpdConfig) -> Result<MpdConn, String> {
     // A short read timeout lets the idle loop notice shutdown requests without
     // blocking for potentially minutes waiting for the next player event.
     stream
-        .set_read_timeout(Some(Duration::from_millis(500)))
+        .set_read_timeout(Some(READ_TIMEOUT))
         .map_err(|e| format!("Failed to set MPD read timeout: {}", e))?;
 
     // Clone the stream so we have independent read and write handles.
@@ -164,6 +270,7 @@ fn connect_tcp(config: &MpdConfig) -> Result<MpdConn, String> {
     let mut conn = MpdConn {
         reader: BufReader::new(Box::new(stream)),
         writer: Box::new(write_stream),
+        desynced: false,
     };
 
     // Consume the "OK MPD x.y.z" greeting before any command can be sent.
@@ -178,7 +285,7 @@ fn connect_unix(config: &MpdConfig) -> Result<MpdConn, String> {
         .map_err(|e| format!("Could not connect to MPD socket {}: {}", config.host, e))?;
 
     stream
-        .set_read_timeout(Some(Duration::from_millis(500)))
+        .set_read_timeout(Some(READ_TIMEOUT))
         .map_err(|e| format!("Failed to set MPD read timeout: {}", e))?;
 
     let write_stream = stream
@@ -188,6 +295,7 @@ fn connect_unix(config: &MpdConfig) -> Result<MpdConn, String> {
     let mut conn = MpdConn {
         reader: BufReader::new(Box::new(stream)),
         writer: Box::new(write_stream),
+        desynced: false,
     };
 
     consume_welcome(&mut conn)?;
@@ -213,10 +321,10 @@ fn connect(config: &MpdConfig) -> Result<MpdConn, String> {
 /// Returns `Err` if the connection is closed immediately or the greeting
 /// doesn't start with `"OK MPD"`, which would indicate a wrong host/port.
 fn consume_welcome(conn: &mut MpdConn) -> Result<(), String> {
-    let mut line = String::new();
+    let line = conn
-    conn.reader
+        .read_line()
-        .read_line(&mut line)
+        .map_err(|e| format!("Failed to read MPD welcome: {}", e))?
-        .map_err(|e| format!("Failed to read MPD welcome: {}", e))?;
+        .ok_or_else(|| "MPD closed the connection before sending a greeting".to_string())?;
 
     if !line.starts_with("OK MPD") {
         return Err(format!("Unexpected MPD greeting: {:?}", line));
@@ -238,26 +346,37 @@ fn send_command(conn: &mut MpdConn, cmd: &str) -> io::Result<()> {
 /// Read a `key: value` response from MPD until `OK\n` is seen.
 ///
 /// Returns a `HashMap<key, value>` of all lines parsed before `OK`. An `ACK`
-/// line is treated as an error and returns an empty map (the caller ignores
+/// line is treated as an error and returns what was parsed so far (usually
-/// the result for most queries).
+/// nothing) — the `ACK` itself terminates the response, so the stream stays in
+/// sync.
+///
+/// Returns `None` when the response could not be read to completion (EOF or a
+/// read failure). In that case the connection is left desynced and the caller
+/// must reconnect rather than issue another command: the unread remainder of
+/// this reply would otherwise be parsed as the next command's response.
 ///
 /// Lines not matching `"key: value"` are silently skipped — MPD sometimes
 /// includes continuation data that doesn't follow this pattern.
-fn read_response(conn: &mut MpdConn) -> HashMap<String, String> {
+fn read_response(conn: &mut MpdConn) -> Option<HashMap<String, String>> {
     let mut map = HashMap::new();
     loop {
-        let mut line = String::new();
+        let raw = match conn.read_line() {
-        match conn.reader.read_line(&mut line) {
+            Ok(Some(l)) => l,
-            Ok(0) => break, // EOF — connection closed
+            // EOF part-way through a response is a lost connection, not an
-            Ok(_) => {}
+            // empty result.
-            Err(_) => break,
+            Ok(None) => {
+                conn.desynced = true;
+                return None;
             }
-        let line = line.trim_end_matches('\n').trim_end_matches('\r');
+            Err(_) => return None,
+        };
+        let line = raw.trim_end_matches('\n').trim_end_matches('\r');
         if line == "OK" {
             break;
         }
         if line.starts_with("ACK") {
-            // Protocol error — log it and return what we have (usually nothing).
+            // Protocol-level error. The ACK line ends the response, so the
+            // stream remains usable.
             break;
         }
         // Split on the first ": " only, in case a value contains ": ".
@@ -267,7 +386,7 @@ fn read_response(conn: &mut MpdConn) -> HashMap<String, String> {
             map.insert(key, val);
         }
     }
-    map
+    Some(map)
 }
 
 /// Read the `readpicture` response header lines until `binary: N` is seen.
@@ -278,9 +397,8 @@ fn read_response(conn: &mut MpdConn) -> HashMap<String, String> {
 fn read_picture_header(conn: &mut MpdConn) -> Option<(u64, u64)> {
     let mut total_size: u64 = 0;
     loop {
-        let mut line = String::new();
+        let raw = conn.read_line().ok()??;
-        conn.reader.read_line(&mut line).ok()?;
+        let line = raw.trim_end_matches('\n').trim_end_matches('\r');
-        let line = line.trim_end_matches('\n').trim_end_matches('\r');
 
         if line == "OK" {
             // Reached OK without a binary field — unexpected, treat as no data.
@@ -302,19 +420,47 @@ fn read_picture_header(conn: &mut MpdConn) -> Option<(u64, u64)> {
 /// Read exactly `n` raw bytes from MPD's response stream.
 ///
 /// Used after parsing the `binary: N` header to extract the image chunk.
-/// `BufReader::read_exact` correctly drains buffered data first and then
+/// [`MpdConn::read_exact`] drains buffered data first and then reads the
-/// reads the remainder from the socket, so mixing text line reads and binary
+/// remainder from the socket, so mixing text line reads and binary chunk reads
-/// chunk reads on the same `BufReader` is safe.
+/// on the same `BufReader` is safe, and it retries across the socket's short
+/// read timeout so a chunk that arrives slowly is not abandoned half-read.
 fn read_exact_bytes(conn: &mut MpdConn, n: u64) -> Option<Vec<u8>> {
     let mut buf = vec![0u8; n as usize];
-    conn.reader.read_exact(&mut buf).ok()?;
+    conn.read_exact(&mut buf).ok()?;
     Some(buf)
 }
 
-/// Read the single `OK\n` line that terminates a binary chunk response.
+/// Consume the terminator after a binary chunk (`\nOK\n`).
-fn read_ok_line(conn: &mut MpdConn) {
+///
-    let mut line = String::new();
+/// MPD's binary responses place a newline byte immediately after the raw
-    let _ = conn.reader.read_line(&mut line);
+/// chunk payload, then an `OK` line. That means the first `read_line` after
+/// `read_exact(binary_len)` often returns an empty line, not `OK`.
+///
+/// Returns `Some(())` only when an `OK` line is observed; returns `None` on
+/// EOF, protocol errors, or I/O errors.
+fn read_chunk_terminator(conn: &mut MpdConn) -> Option<()> {
+    loop {
+        let raw = conn.read_line().ok()??;
+
+        let line = raw.trim_end_matches('\n').trim_end_matches('\r');
+        if line.is_empty() {
+            // Expected: this is the newline that follows the raw binary bytes.
+            continue;
+        }
+        if line == "OK" {
+            return Some(());
+        }
+        if line.starts_with("ACK") {
+            // An ACK terminates the response, so the stream is still aligned.
+            return None;
+        }
+
+        // Any other non-empty line here means the stream got out of sync:
+        // we cannot tell how much of the binary payload we mis-parsed, so the
+        // connection must not be reused.
+        conn.desynced = true;
+        return None;
+    }
 }
 
 // ---------------------------------------------------------------------------
@@ -350,7 +496,7 @@ struct MpdStatus {
 /// Returns `None` if MPD is stopped (no current song) or if the query fails.
 fn query_currentsong(conn: &mut MpdConn) -> Option<MpdSong> {
     send_command(conn, "currentsong").ok()?;
-    let map = read_response(conn);
+    let map = read_response(conn)?;
 
     // If MPD is stopped, currentsong returns an empty response.
     if map.is_empty() {
@@ -383,9 +529,13 @@ fn query_status(conn: &mut MpdConn) -> MpdStatus {
     if send_command(conn, "status").is_err() {
         return MpdStatus::default();
     }
-    let map = read_response(conn);
+    match read_response(conn) {
-    MpdStatus {
+        Some(map) => MpdStatus {
             state: map.get("state").cloned().unwrap_or_default(),
+        },
+        // Unreadable response: the connection is already marked desynced and
+        // the event loop will tear it down. Report an empty state meanwhile.
+        None => MpdStatus::default(),
     }
 }
 
@@ -411,11 +561,17 @@ fn search_song_for_album(conn: &mut MpdConn, artist: &str, album: &str) -> Optio
         escape_mpd_string(artist),
         escape_mpd_string(album)
     );
-    if send_command(conn, &cmd).is_ok() {
+    if send_command(conn, &cmd).is_ok()
-        let map = read_response(conn);
+        && let Some(map) = read_response(conn)
-        if let Some(file) = map.get("file") {
+        && let Some(file) = map.get("file")
+    {
         return Some(file.clone());
     }
+
+    // A failed read leaves the stream mid-response; issuing the fallback
+    // search on the same connection would misparse it.
+    if conn.desynced {
+        return None;
     }
 
     // Fall back to the Artist tag (handles single-artist albums and tracks
@@ -425,12 +581,12 @@ fn search_song_for_album(conn: &mut MpdConn, artist: &str, album: &str) -> Optio
         escape_mpd_string(artist),
         escape_mpd_string(album)
     );
-    if send_command(conn, &cmd).is_ok() {
+    if send_command(conn, &cmd).is_ok()
-        let map = read_response(conn);
+        && let Some(map) = read_response(conn)
-        if let Some(file) = map.get("file") {
+        && let Some(file) = map.get("file")
+    {
         return Some(file.clone());
     }
-    }
 
     None
 }
@@ -455,11 +611,17 @@ fn escape_mpd_string(s: &str) -> String {
 ///
 /// Returns `None` if the file has no embedded picture, if the file is not
 /// found in MPD's database, or if a protocol error occurs.
+///
+/// The transfer is bounded on both size and iteration count. MPD is normally
+/// trustworthy, but the loop is driven entirely by numbers MPD supplies, so a
+/// buggy or hostile server could otherwise keep it allocating forever — see
+/// [`MAX_PICTURE_BYTES`] and [`MAX_PICTURE_CHUNKS`].
 fn read_picture(conn: &mut MpdConn, file_uri: &str) -> Option<Vec<u8>> {
     let mut all_bytes: Vec<u8> = Vec::new();
     let mut offset: u64 = 0;
+    let mut expected_total: Option<u64> = None;
 
-    loop {
+    for _ in 0..MAX_PICTURE_CHUNKS {
         // Request the next chunk starting at `offset`.
         let cmd = format!("readpicture \"{}\" {}", escape_mpd_string(file_uri), offset);
         send_command(conn, &cmd).ok()?;
@@ -467,11 +629,24 @@ fn read_picture(conn: &mut MpdConn, file_uri: &str) -> Option<Vec<u8>> {
         // Parse the `size`, `type`, and `binary` header fields.
         let (total_size, chunk_size) = read_picture_header(conn)?;
 
+        if total_size > MAX_PICTURE_BYTES || chunk_size > MAX_PICTURE_BYTES {
+            eprintln!("  [warn] Embedded cover art is implausibly large; skipping.");
+            return None;
+        }
+
+        if total_size > 0 {
+            match expected_total {
+                None => expected_total = Some(total_size),
+                Some(prev) if prev != total_size => return None,
+                Some(_) => {}
+            }
+        }
+
         // `binary: 0` means no embedded picture (or we've read everything).
         if chunk_size == 0 {
             // If we have already accumulated bytes, we're done; otherwise there
             // was no picture at all.
-            break;
+            return finish_picture(all_bytes, expected_total);
         }
 
         // Read the raw binary chunk from the stream.
@@ -479,14 +654,49 @@ fn read_picture(conn: &mut MpdConn, file_uri: &str) -> Option<Vec<u8>> {
         all_bytes.extend_from_slice(&chunk);
 
         // Each chunk response is terminated by a bare `OK` line.
-        read_ok_line(conn);
+        read_chunk_terminator(conn)?;
 
         offset += chunk_size;
 
-        // Stop when we've read the complete image.
+        if all_bytes.len() as u64 > MAX_PICTURE_BYTES {
-        if total_size > 0 && offset >= total_size {
+            eprintln!("  [warn] Embedded cover art exceeded the size limit; skipping.");
-            break;
+            return None;
         }
+
+        // Stop when we've read the complete image. A server that never
+        // declares a total size falls through to the chunk-count guard rather
+        // than looping forever.
+        if total_size > 0 && offset >= total_size {
+            return finish_picture(all_bytes, expected_total);
+        }
+    }
+
+    // Ran out of iterations without MPD ever signalling completion.
+    None
+}
+
+/// Largest embedded picture we are willing to buffer, in bytes.
+///
+/// Cover art is a JPEG or PNG of a few megabytes at most; the cap only exists
+/// so a bogus `size:`/`binary:` header cannot drive an unbounded allocation.
+const MAX_PICTURE_BYTES: u64 = 64 * 1024 * 1024;
+
+/// Upper bound on `readpicture` round-trips for a single image.
+///
+/// MPD's default chunk size is 8 KiB, so this allows well over
+/// [`MAX_PICTURE_BYTES`] worth of chunks while still terminating if a server
+/// reports no total size and keeps returning data.
+const MAX_PICTURE_CHUNKS: usize = 16_384;
+
+/// Validate a completed picture transfer and hand back the bytes.
+///
+/// Returns `None` when MPD declared a total size we did not fully receive, or
+/// when the file simply had no embedded art.
+fn finish_picture(all_bytes: Vec<u8>, expected_total: Option<u64>) -> Option<Vec<u8>> {
+    if let Some(total) = expected_total
+        && all_bytes.len() as u64 != total
+    {
+        return None;
     }
 
     if all_bytes.is_empty() {
@@ -502,27 +712,12 @@ fn read_picture(conn: &mut MpdConn, file_uri: &str) -> Option<Vec<u8>> {
 
 /// Generate a stable filename stem for a locally-extracted MPD cover.
 ///
-/// Uses a 64-bit FNV-1a hash of `"artist\talbum"`, formatted as a 16-char
+/// Thin wrapper over [`enrich::cover_stem`] that pins the `mpd_` prefix, which
-/// hex string prefixed with `"mpd_"`. This gives a compact, deterministic
+/// `repair-mpd-covers` relies on to find these files again.
-/// filename that:
-///
-/// - Is stable across runs (same artist+album always maps to the same name).
-/// - Is visually distinct from MBID-based filenames (MBIDs contain hyphens).
-/// - Has negligible collision probability across a typical music library.
 ///
 /// Example: `mpd_a3f7c2b91e045d68.jpg`
 fn album_cover_stem(artist: &str, album: &str) -> String {
-    // FNV-1a 64-bit hash constants.
+    enrich::cover_stem("mpd", artist, album)
-    const FNV_OFFSET: u64 = 14695981039346656037;
-    const FNV_PRIME: u64 = 1099511628211;
-
-    let input = format!("{}\t{}", artist, album);
-    let mut hash = FNV_OFFSET;
-    for byte in input.bytes() {
-        hash ^= byte as u64;
-        hash = hash.wrapping_mul(FNV_PRIME);
-    }
-    format!("mpd_{:016x}", hash)
 }
 
 // ---------------------------------------------------------------------------
@@ -707,6 +902,15 @@ fn run_event_loop(
         // Query MPD's current state now that we know something changed.
         let song = query_currentsong(mpd_conn);
         let status = query_status(mpd_conn);
+
+        // If either query failed part-way, the stream is no longer positioned
+        // at a message boundary. Reconnect instead of issuing more commands
+        // against a connection whose replies we can no longer line up.
+        if mpd_conn.desynced {
+            eprintln!("[mpd] Lost sync with MPD protocol stream.");
+            return false;
+        }
+
         let new = MpdPlayerState::from_queries(song, status);
 
         // Determine what changed and dispatch the appropriate events.
@@ -716,6 +920,11 @@ fn run_event_loop(
         // cover for the new album if we don't already have one cached.
         if new.file != prev.file && !new.file.is_empty() && !new.album.is_empty() {
             try_extract_cover(mpd_conn, db_conn, &new);
+            if mpd_conn.desynced {
+                eprintln!("[mpd] Lost sync while reading embedded cover art.");
+                *prev = new;
+                return false;
+            }
         }
 
         *prev = new;
@@ -744,12 +953,24 @@ enum IdleResult {
 /// of whether a `changed: player` line appeared. In both cases the caller
 /// re-queries MPD state, so the distinction doesn't matter.
 fn wait_for_idle_response(conn: &mut MpdConn, running: &Arc<AtomicBool>) -> IdleResult {
+    // This is the one place that reads the socket directly rather than going
+    // through `MpdConn::read_line`: here a timeout is the *point*, since it is
+    // what lets us poll the shutdown flag while MPD is silent.
+    //
+    // `line` lives outside the loop so that a line split across two timeouts
+    // is reassembled rather than truncated. `read_until` appends what it got
+    // before erroring and leaves it in the buffer, so resuming is correct.
+    let mut line: Vec<u8> = Vec::new();
     loop {
-        let mut line = String::new();
+        match conn.reader.read_until(b'\n', &mut line) {
-        match conn.reader.read_line(&mut line) {
             Ok(0) => return IdleResult::ConnectionLost, // EOF
             Ok(_) => {
-                let trimmed = line.trim();
+                if line.last() != Some(&b'\n') {
+                    // Delimiter missing without an error means EOF mid-line.
+                    return IdleResult::ConnectionLost;
+                }
+                let text = String::from_utf8_lossy(&line);
+                let trimmed = text.trim();
                 if trimmed == "OK" {
                     // End of idle response — re-query state regardless of whether
                     // "changed: player" appeared. With `idle player` the line
@@ -759,10 +980,9 @@ fn wait_for_idle_response(conn: &mut MpdConn, running: &Arc<AtomicBool>) -> Idle
                 }
                 // "changed: player" and any other key/value lines are consumed
                 // and discarded here — we query currentsong/status ourselves.
+                line.clear();
             }
-            Err(ref e)
+            Err(ref e) if is_timeout(e) => {
-                if e.kind() == io::ErrorKind::WouldBlock || e.kind() == io::ErrorKind::TimedOut =>
-            {
                 // Read timeout — check if we should shut down, then keep waiting.
                 if !running.load(Ordering::SeqCst) {
                     return IdleResult::Shutdown;
@@ -897,8 +1117,13 @@ fn try_extract_cover(
         None => return,
     };
 
-    // Save the processed image to the covers cache directory.
+    // Save the processed image to the covers cache directory. Caching a cover
-    let covers = enrich::covers_dir();
+    // is opportunistic, so an unusable cache directory just means no cover —
+    // it must not interrupt the watch loop.
+    let covers = match enrich::covers_dir() {
+        Ok(d) => d,
+        Err(_) => return,
+    };
     let stem = album_cover_stem(&state.artist, &state.album);
     let dest = covers.join(format!("{}.jpg", stem));
     if std::fs::write(&dest, &processed).is_err() {
@@ -994,6 +1219,169 @@ pub fn run_mpd_cover_enrich_targeted(
     run_mpd_cover_enrich_albums(config, conn, albums);
 }
 
+/// Revalidate already-cached local MPD covers and re-fetch mismatches.
+///
+/// This command targets albums that already have `cover_url` pointing to an
+/// MPD-local path (`mpd_*.jpg`). For each candidate album it re-reads embedded
+/// art from MPD, applies the same resize/re-encode pipeline, then compares the
+/// resulting bytes to the existing file.
+///
+/// If bytes differ (or the file is missing), the local file is overwritten and
+/// `album_cache.fetched_at` is refreshed via [`db::set_local_cover`].
+pub fn run_mpd_cover_revalidate(config: &MpdConfig, conn: &Connection, artist: Option<&str>) {
+    let mut albums = match db::albums_with_local_mpd_cover(conn) {
+        Ok(a) => a,
+        Err(e) => {
+            eprintln!("[error] Failed to query MPD-local covers: {}", e);
+            return;
+        }
+    };
+
+    if let Some(artist_name) = artist {
+        albums.retain(|a| a.artist.eq_ignore_ascii_case(artist_name));
+    }
+
+    if albums.is_empty() {
+        if let Some(artist_name) = artist {
+            eprintln!(
+                "No MPD-local covers found for artist \"{}\". Nothing to revalidate.",
+                artist_name
+            );
+        } else {
+            eprintln!("No MPD-local covers found. Nothing to revalidate.");
+        }
+        return;
+    }
+
+    // Resolve the covers directory once, up front: it is the boundary the
+    // per-album path check below is measured against, so failing to resolve it
+    // must stop the run rather than weaken the check.
+    let covers_root = match enrich::covers_dir() {
+        Ok(d) => d,
+        Err(e) => {
+            eprintln!("[error] Cover cache unavailable: {}", e);
+            return;
+        }
+    };
+
+    let mut mpd_conn = match connect(config) {
+        Ok(c) => c,
+        Err(e) => {
+            eprintln!("[warn] MPD unreachable, skipping cover revalidation: {}", e);
+            return;
+        }
+    };
+
+    eprintln!("Revalidating {} MPD-local cover(s)...", albums.len());
+
+    let mut unchanged = 0;
+    let mut repaired = 0;
+    let mut skipped = 0;
+
+    for (i, album) in albums.iter().enumerate() {
+        eprintln!(
+            "[{}/{}] {} - {}",
+            i + 1,
+            albums.len(),
+            album.artist,
+            album.album
+        );
+
+        if mpd_conn.desynced {
+            eprintln!("  [warn] Lost sync with MPD; aborting revalidation.");
+            skipped += albums.len() - i;
+            break;
+        }
+
+        let file_uri = match search_song_for_album(&mut mpd_conn, &album.artist, &album.album) {
+            Some(f) => f,
+            None => {
+                eprintln!("  Not found in MPD database, skipping.");
+                skipped += 1;
+                continue;
+            }
+        };
+
+        let picture_bytes = match read_picture(&mut mpd_conn, &file_uri) {
+            Some(b) => b,
+            None => {
+                eprintln!("  No embedded cover art found, skipping.");
+                skipped += 1;
+                continue;
+            }
+        };
+
+        let processed = match enrich::resize_cover_bytes(&picture_bytes) {
+            Some(b) => b,
+            None => {
+                eprintln!("  [warn] Could not decode embedded cover, skipping.");
+                skipped += 1;
+                continue;
+            }
+        };
+
+        let dest = Path::new(&album.cover_url);
+
+        // Sanity-check: only write to paths inside the scrbblr covers
+        // directory. Rejects any DB row whose cover_url was tampered with to
+        // point outside our own data directory.
+        if !dest.starts_with(&covers_root) {
+            eprintln!(
+                "  [warn] cover_url '{}' is outside the covers directory; skipping.",
+                album.cover_url
+            );
+            skipped += 1;
+            continue;
+        }
+
+        let needs_repair = match std::fs::read(dest) {
+            Ok(existing) => existing != processed,
+            Err(_) => true,
+        };
+
+        if !needs_repair {
+            eprintln!("  OK: existing file matches MPD extraction.");
+            unchanged += 1;
+            continue;
+        }
+
+        if let Some(parent) = dest.parent()
+            && let Err(e) = std::fs::create_dir_all(parent)
+        {
+            eprintln!("  [warn] Failed to create parent directory: {}", e);
+            skipped += 1;
+            continue;
+        }
+
+        if let Err(e) = std::fs::write(dest, &processed) {
+            eprintln!("  [warn] Failed to write repaired cover: {}", e);
+            skipped += 1;
+            continue;
+        }
+
+        match db::set_local_cover(conn, &album.artist, &album.album, &album.cover_url) {
+            Ok(_) => {
+                eprintln!("  Repaired: {}", album.cover_url);
+                repaired += 1;
+            }
+            Err(e) => {
+                eprintln!(
+                    "  [error] Repaired file but failed to update album_cache: {}",
+                    e
+                );
+                skipped += 1;
+            }
+        }
+    }
+
+    eprintln!();
+    eprintln!("MPD cover revalidation complete:");
+    eprintln!("  Albums checked: {}", albums.len());
+    eprintln!("  Covers unchanged: {}", unchanged);
+    eprintln!("  Covers repaired: {}", repaired);
+    eprintln!("  Skipped: {}", skipped);
+}
+
 /// Inner loop shared by `run_mpd_cover_enrich` and
 /// `run_mpd_cover_enrich_targeted`. Connects to MPD and processes the given
 /// album list.
@@ -1011,7 +1399,13 @@ fn run_mpd_cover_enrich_albums(
         }
     };
 
-    let covers = enrich::covers_dir();
+    let covers = match enrich::covers_dir() {
+        Ok(d) => d,
+        Err(e) => {
+            eprintln!("[error] Cover cache unavailable: {}", e);
+            return;
+        }
+    };
     eprintln!(
         "Extracting covers from MPD for {} album(s)...",
         albums.len()
@@ -1029,6 +1423,12 @@ fn run_mpd_cover_enrich_albums(
             album.album
         );
 
+        if mpd_conn.desynced {
+            eprintln!("  [warn] Lost sync with MPD; aborting cover extraction.");
+            skipped += albums.len() - i;
+            break;
+        }
+
         // Step 1: Ask MPD for any file from this album so we have a path to
         // hand to readpicture.
         let file_uri = match search_song_for_album(&mut mpd_conn, &album.artist, &album.album) {
@@ -1101,8 +1501,29 @@ fn run_mpd_cover_enrich_albums(
 mod tests {
     use super::*;
     use std::cell::RefCell;
+    use std::io::Cursor;
     use std::rc::Rc;
 
+    struct NullWriter;
+
+    impl Write for NullWriter {
+        fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
+            Ok(buf.len())
+        }
+
+        fn flush(&mut self) -> io::Result<()> {
+            Ok(())
+        }
+    }
+
+    fn fake_conn(input: &[u8]) -> MpdConn {
+        MpdConn {
+            reader: BufReader::new(Box::new(Cursor::new(input.to_vec()))),
+            writer: Box::new(NullWriter),
+            desynced: false,
+        }
+    }
+
     fn state(
         file: &str,
         artist: &str,
@@ -1215,6 +1636,88 @@ mod tests {
         assert_eq!(parse_mpd_state("unknown"), PlayerStatus::Stopped);
     }
 
+    // -----------------------------------------------------------------------
+    // readpicture protocol handling
+    // -----------------------------------------------------------------------
+
+    #[test]
+    fn test_read_chunk_terminator_consumes_blank_then_ok() {
+        let mut conn = fake_conn(b"\nOK\n");
+        assert!(read_chunk_terminator(&mut conn).is_some());
+    }
+
+    #[test]
+    fn test_read_picture_reads_multiple_chunks_fully() {
+        // Two chunks: "hello" + " world!" = 12 bytes total.
+        let mut conn =
+            fake_conn(b"size: 12\nbinary: 5\nhello\nOK\nsize: 12\nbinary: 7\n world!\nOK\n");
+        let bytes = read_picture(&mut conn, "music/test.flac").unwrap();
+        assert_eq!(bytes, b"hello world!");
+    }
+
+    #[test]
+    fn test_read_picture_rejects_truncated_transfer() {
+        // MPD says total is 12, but only 5 bytes arrive before binary: 0.
+        let mut conn = fake_conn(b"size: 12\nbinary: 5\nhello\nOK\nsize: 12\nbinary: 0\nOK\n");
+        assert!(read_picture(&mut conn, "music/test.flac").is_none());
+    }
+
+    #[test]
+    fn test_read_picture_terminates_when_total_size_never_declared() {
+        // A server that keeps returning chunks but never declares a total size
+        // must not loop forever. Two chunks then EOF: the header read fails and
+        // the transfer is abandoned rather than spinning.
+        let mut conn = fake_conn(b"size: 0\nbinary: 5\nhello\nOK\nsize: 0\nbinary: 5\nworld\nOK\n");
+        assert!(read_picture(&mut conn, "music/test.flac").is_none());
+    }
+
+    #[test]
+    fn test_read_picture_rejects_implausible_size() {
+        // A bogus `size:` header must not drive a multi-gigabyte allocation.
+        let mut conn = fake_conn(b"size: 999999999999\nbinary: 4\ndata\nOK\n");
+        assert!(read_picture(&mut conn, "music/test.flac").is_none());
+    }
+
+    #[test]
+    fn test_read_chunk_terminator_flags_desync_on_unexpected_line() {
+        // A line that is neither blank nor OK means we mis-parsed the binary
+        // payload boundary. The connection must be marked unusable so the
+        // caller reconnects rather than reading the rest as a fresh response.
+        let mut conn = fake_conn(b"size: 99\n");
+        assert!(read_chunk_terminator(&mut conn).is_none());
+        assert!(
+            conn.desynced,
+            "unexpected line must mark the stream desynced"
+        );
+    }
+
+    #[test]
+    fn test_read_chunk_terminator_ack_keeps_stream_usable() {
+        // An ACK terminates the response cleanly, so the connection is fine.
+        let mut conn = fake_conn(b"ACK [50@0] {readpicture} No file exists\n");
+        assert!(read_chunk_terminator(&mut conn).is_none());
+        assert!(!conn.desynced, "an ACK is a normal response terminator");
+    }
+
+    #[test]
+    fn test_read_response_reports_eof_as_failure() {
+        // A response that ends without its terminating OK is a lost
+        // connection, not an empty result set. Returning an empty map here
+        // would make `currentsong` look like "MPD is stopped".
+        let mut conn = fake_conn(b"file: a.flac\nTitle: Song\n");
+        assert!(read_response(&mut conn).is_none());
+        assert!(conn.desynced);
+    }
+
+    #[test]
+    fn test_read_response_parses_complete_reply() {
+        let mut conn = fake_conn(b"file: a.flac\nTitle: Song\nOK\n");
+        let map = read_response(&mut conn).expect("complete response");
+        assert_eq!(map.get("file").map(String::as_str), Some("a.flac"));
+        assert_eq!(map.get("Title").map(String::as_str), Some("Song"));
+        assert!(!conn.desynced);
+    }
+
     // -----------------------------------------------------------------------
     // dispatch_events
     // -----------------------------------------------------------------------

src/report.rs

@@ -51,24 +51,44 @@ const MIN_BAR_WIDTH: usize = 8;
 
 /// Format a duration in seconds into a human-readable string.
 ///
+/// Once the duration reaches a full day it is reported in days and hours
+/// (minutes are dropped, with the leftover hour rounded to the nearest hour).
+///
 /// Examples:
 ///   - 45 seconds   → "45s"
 ///   - 120 seconds  → "2m"
 ///   - 3660 seconds → "1h 01m"
 ///   - 0 seconds    → "0s"
+///   - 90000 secs   → "1d 1h"
+///   - 86400 secs   → "1d"
 pub fn format_duration(secs: i64) -> String {
+    const HOUR: i64 = 3600;
+    const DAY: i64 = 86400;
     if secs < 60 {
         format!("{}s", secs)
-    } else if secs < 3600 {
+    } else if secs < HOUR {
         format!("{}m", secs / 60)
-    } else {
+    } else if secs < DAY {
-        let hours = secs / 3600;
+        let hours = secs / HOUR;
-        let mins = (secs % 3600) / 60;
+        let mins = (secs % HOUR) / 60;
         if mins == 0 {
             format!("{}h", hours)
         } else {
             format!("{}h {:02}m", hours, mins)
         }
+    } else {
+        let mut days = secs / DAY;
+        // Round the leftover hours to the nearest hour, carrying into days.
+        let mut hours = (secs % DAY + HOUR / 2) / HOUR;
+        if hours == 24 {
+            days += 1;
+            hours = 0;
+        }
+        if hours == 0 {
+            format!("{}d", days)
+        } else {
+            format!("{}d {}h", days, hours)
+        }
     }
 }
 
@@ -350,10 +370,14 @@ fn print_box_table(
                 let shown = truncate_cell(cell, widths[i]);
                 print!(" {:<width$} ", shown, width = widths[i]);
                 if bar_max.is_some() && i == ncols - 1 {
-                    // Render bar chart in the extra column.
+                    // Render bar chart in the extra column. `filled` is
+                    // clamped rather than trusted: a caller that passes a
+                    // `bar_max` below the largest row would otherwise make
+                    // `bar_width - filled` underflow and panic.
                     let val: i64 = cell.parse().unwrap_or(0);
                     let max = bar_max.unwrap_or(1).max(1);
-                    let filled = ((val as f64 / max as f64) * bar_width as f64).round() as usize;
+                    let ratio = (val as f64 / max as f64).clamp(0.0, 1.0);
+                    let filled = ((ratio * bar_width as f64).round() as usize).min(bar_width);
                     let empty = bar_width - filled;
                     print!("│ {}{} ", "█".repeat(filled), "░".repeat(empty));
                 }
@@ -607,7 +631,7 @@ pub fn print_terminal_report(data: &ReportData) {
     // --- Top Artists ---
     if !data.top_artists.is_empty() {
         println!("\n  Top Artists");
-        let max_plays = data.top_artists.first().map(|a| a.plays).unwrap_or(1);
+        let max_plays = data.top_artists.iter().map(|a| a.plays).max().unwrap_or(1);
         let rows: Vec<Vec<String>> = data
             .top_artists
             .iter()
@@ -632,7 +656,7 @@ pub fn print_terminal_report(data: &ReportData) {
     // --- Top Albums ---
     if !data.top_albums.is_empty() {
         println!("\n  Top Albums");
-        let max_plays = data.top_albums.first().map(|a| a.plays).unwrap_or(1);
+        let max_plays = data.top_albums.iter().map(|a| a.plays).max().unwrap_or(1);
         let rows: Vec<Vec<String>> = data
             .top_albums
             .iter()
@@ -658,7 +682,9 @@ pub fn print_terminal_report(data: &ReportData) {
     // --- Top Genres ---
     if !data.top_genres.is_empty() {
         println!("\n  Top Genres");
-        let max_plays = data.top_genres.first().map(|g| g.plays).unwrap_or(1);
+        // Not `first()`: `top_genres` ranks multi-word genres above broad
+        // single-word ones, so the top row is not necessarily the busiest.
+        let max_plays = data.top_genres.iter().map(|g| g.plays).max().unwrap_or(1);
         let rows: Vec<Vec<String>> = data
             .top_genres
             .iter()
@@ -683,7 +709,7 @@ pub fn print_terminal_report(data: &ReportData) {
     // --- Top Tracks ---
     if !data.top_tracks.is_empty() {
         println!("\n  Top Tracks");
-        let max_plays = data.top_tracks.first().map(|t| t.plays).unwrap_or(1);
+        let max_plays = data.top_tracks.iter().map(|t| t.plays).max().unwrap_or(1);
         let rows: Vec<Vec<String>> = data
             .top_tracks
             .iter()
@@ -813,7 +839,7 @@ pub fn render_html_report(conn: &Connection, limit: i64, all_time_limit: i64) ->
     h.open("<div class=\"hero\">");
     h.line("<h1>Listening Report</h1>");
     h.linef(format_args!(
-        "<div class=\"sub\">Generated by scrbblr on {}</div>",
+        "<div class=\"sub\">Generated by <a href=\"https://github.com/arturmeski/scrbblr\">scrbblr</a> on {}</div>",
         html_escape(&generated_at)
     ));
     h.close("</div>");
@@ -956,7 +982,14 @@ pub fn render_html_report(conn: &Connection, limit: i64, all_time_limit: i64) ->
         // We intentionally round the cover count up to a full desktop row so
         // the grid doesn't end with a partially filled row when the user picks
         // a limit like 20 and desktop layout has 6 columns.
-        let cover_limit = album_cover_grid_limit(limit);
+        //
+        // Use this period's own limit, not the base one: all-time sections
+        // render a longer top-albums table, and a grid sized from `limit`
+        // would show fewer covers than the table directly beneath it. It also
+        // has to match what `albums_needed_for_report` pre-enriched, which is
+        // likewise computed per period.
+        let period_limit = if is_all_time { all_time_limit } else { limit };
+        let cover_limit = album_cover_grid_limit(period_limit);
         let cover_albums = db::top_albums(conn, &data.period.name, cover_limit)
             .unwrap_or_else(|_| data.top_albums.clone());
         if !cover_albums.is_empty() {
@@ -1053,25 +1086,14 @@ pub fn render_html_report(conn: &Connection, limit: i64, all_time_limit: i64) ->
                     .flatten()
                     .and_then(|m| m.cover_url);
                 let cover = resolve_cover(cover_url, &mut cover_files);
-                // Prepend a subtle source dot to the album name if we know
+                let source_cell = source_dot_cell(a.dominant_source.as_deref(), &ordered_sources);
-                // the dominant source for this album.
-                let album_cell = if let Some(src) = a.dominant_source.as_deref() {
-                    if let Some((bg, _)) = source_colours(src, &ordered_sources) {
-                        format!(
-                            "<span style=\"display:inline-block;width:9px;height:9px;\
-                             border-radius:50%;background:{};margin-right:5px;\
-                             vertical-align:middle\"></span>{}",
-                            bg,
-                            html_escape(&a.album)
-                        )
-                    } else {
-                        html_escape(&a.album)
-                    }
-                } else {
-                    html_escape(&a.album)
-                };
                 BarRow {
-                    cells: vec![(i + 1).to_string(), html_escape(&a.artist), album_cell],
+                    cells: vec![
+                        (i + 1).to_string(),
+                        html_escape(&a.artist),
+                        html_escape(&a.album),
+                        source_cell,
+                    ],
                     value: a.plays,
                     suffix: format_duration(a.listen_time_secs),
                     cover,
@@ -1082,7 +1104,7 @@ pub fn render_html_report(conn: &Connection, limit: i64, all_time_limit: i64) ->
         write_bar_table(
             &mut h,
             "Top Albums",
-            &["#", "Artist", "Album", "Plays", ""],
+            &["#", "Artist", "Album", "Src", "Plays", ""],
             &album_rows,
         );
 
@@ -1146,23 +1168,14 @@ pub fn render_html_report(conn: &Connection, limit: i64, all_time_limit: i64) ->
                     .flatten()
                     .and_then(|m| m.cover_url);
                 let cover = resolve_cover(cover_url, &mut cover_files);
-                let title_cell = if let Some(src) = t.dominant_source.as_deref() {
+                let source_cell = source_dot_cell(t.dominant_source.as_deref(), &ordered_sources);
-                    if let Some((bg, _)) = source_colours(src, &ordered_sources) {
-                        format!(
-                            "<span style=\"display:inline-block;width:9px;height:9px;\
-                             border-radius:50%;background:{};margin-right:5px;\
-                             vertical-align:middle\"></span>{}",
-                            bg,
-                            html_escape(&t.title)
-                        )
-                    } else {
-                        html_escape(&t.title)
-                    }
-                } else {
-                    html_escape(&t.title)
-                };
                 BarRow {
-                    cells: vec![(i + 1).to_string(), html_escape(&t.artist), title_cell],
+                    cells: vec![
+                        (i + 1).to_string(),
+                        html_escape(&t.artist),
+                        html_escape(&t.title),
+                        source_cell,
+                    ],
                     value: t.plays,
                     suffix: format_duration(t.listen_time_secs),
                     cover,
@@ -1173,7 +1186,7 @@ pub fn render_html_report(conn: &Connection, limit: i64, all_time_limit: i64) ->
         write_bar_table(
             &mut h,
             "Top Tracks",
-            &["#", "Artist", "Title", "Plays", ""],
+            &["#", "Artist", "Title", "Src", "Plays", ""],
             &track_rows,
         );
 
@@ -1243,6 +1256,12 @@ h1, h2, h3 { margin: 0; }
   border-radius: 14px; box-shadow: 0 14px 40px rgba(0,0,0,.25);
 }
 .sub { margin-top: 6px; color: var(--muted); font-size: 14px; }
+.sub a {
+  color: var(--bar-fill);
+  text-decoration-color: #a87937;
+  text-underline-offset: 2px;
+}
+.sub a:hover { color: #ffd38e; }
 .jump-nav {
   position: sticky; top: 0; z-index: 20;
   margin-top: 12px; padding: 8px;
@@ -1288,7 +1307,7 @@ h1, h2, h3 { margin: 0; }
 .kpi .v { margin-top: 4px; font-size: 22px; font-weight: 700; }
 section { margin-top: 18px; }
 .section-title { font-size: 16px; margin-bottom: 8px; }
-.card { background: var(--panel2); border: 1px solid var(--line); border-radius: 10px; overflow: hidden; }
+.card { background: var(--panel2); border: 1px solid var(--line); border-radius: 10px; overflow-x: auto; overflow-y: hidden; }
 table { width: 100%; border-collapse: collapse; }
 th, td { text-align: left; padding: 8px 12px; border-bottom: 1px solid var(--line); }
 th { font-size: 11px; color: var(--muted); letter-spacing: .02em; text-transform: uppercase; }
@@ -1301,6 +1320,15 @@ tr:last-child td { border-bottom: none; }
 .bar-track { flex: 1; }
 .bar { height: 18px; border-radius: 4px; background: var(--bar-fill); min-width: 2px; }
 .bar-label { font-size: 12px; color: var(--muted); white-space: nowrap; }
+.source-col { width: 38px; min-width: 38px; text-align: center; }
+.source-dot {
+  display: inline-block;
+  width: 10px;
+  height: 10px;
+  border-radius: 50%;
+  vertical-align: middle;
+}
+.source-dot-fallback { background: #7a6a55; }
 .grid { display: grid; grid-template-columns: repeat(6, minmax(0, 1fr)); gap: 10px; }
 .album { position: relative; background: var(--panel2); border: 1px solid var(--line); border-radius: 10px; overflow: hidden; }
 .cover { width: 100%; aspect-ratio: 1/1; object-fit: cover; background: #0d1116; }
@@ -1308,8 +1336,9 @@ tr:last-child td { border-bottom: none; }
        color: #a99984;
        background: linear-gradient(135deg, #17130d, #2c2116); font-size: 12px; }
 .pin-dot { position: absolute; top: 5px; right: 5px; width: 22px; height: 22px;
-           border-radius: 50%; background: rgba(0,0,0,0.45); border: none;
+           border-radius: 50%; background: rgba(0,0,0,0.45); border: none; padding: 0;
-           cursor: pointer; font-size: 12px; line-height: 22px; text-align: center;
+           cursor: pointer; font-size: 12px; line-height: 1;
+           display: flex; align-items: center; justify-content: center;
            opacity: 0; transition: opacity 0.15s; z-index: 1; }
 .album:hover .pin-dot { opacity: 1; }
 .pin-dot:hover { background: rgba(0,0,0,0.75); }
@@ -1338,6 +1367,11 @@ tr:last-child td { border-bottom: none; }
   .wrap { padding: 16px 12px 36px; }
   .grid { grid-template-columns: repeat(2, minmax(0, 1fr)); }
   .jump-nav { margin-top: 10px; padding: 7px; }
+  th, td { padding: 7px 8px; }
+  .source-col { width: 28px; min-width: 28px; }
+  .bar-cell { width: 32%; }
+  .bar-wrap { gap: 4px; }
+  .bar-label { font-size: 11px; }
   /* On mobile show rank + duration only; drop the raw play-count and the bar graphic.
      The .bar-label (duration text) remains visible inside .bar-cell. */
   .play-count { display: none; }
@@ -1358,6 +1392,29 @@ const SOURCE_PALETTE: &[(&str, &str)] = &[
     ("rgba(220,130,130,0.22)", "rgba(220,130,130,0.60)"), // red
 ];
 
+/// Render the "Src" table cell for a row: a coloured dot carrying the source
+/// name as its tooltip, or `-` when the row predates the `source` column.
+///
+/// Sources beyond the palette fall back to a neutral dot so the column never
+/// silently loses the tooltip. The returned string is HTML and must be placed
+/// in a `BarRow` with `raw_cells: true`.
+fn source_dot_cell(source: Option<&str>, ordered_sources: &[String]) -> String {
+    let Some(src) = source else {
+        return "-".to_string();
+    };
+    match source_colours(src, ordered_sources) {
+        Some((bg, _)) => format!(
+            "<span class=\"source-dot\" style=\"background:{}\" title=\"{}\"></span>",
+            bg,
+            html_attr_escape(src)
+        ),
+        None => format!(
+            "<span class=\"source-dot source-dot-fallback\" title=\"{}\"></span>",
+            html_attr_escape(src)
+        ),
+    }
+}
+
 /// Look up the card background and border colours for a source name, given
 /// the ordered list of all-time sources (most played first).
 fn source_colours<'a>(source: &str, ordered_sources: &[String]) -> Option<(&'a str, &'a str)> {
@@ -1466,6 +1523,11 @@ impl HtmlWriter {
 /// for use in HTML (`covers/<filename>`), and track the source file for
 /// copying into the output directory. Returns `None` if the path is missing
 /// or the file doesn't exist on disk.
+///
+/// The existence check gates the returned path as well as the copy list: a
+/// cache row whose image has since been deleted must fall through to the
+/// "No cover" placeholder rather than emit an `<img>` pointing at a file that
+/// was never copied into the output directory.
 fn resolve_cover(
     abs_path: Option<String>,
     cover_files: &mut Vec<std::path::PathBuf>,
@@ -1473,9 +1535,10 @@ fn resolve_cover(
     let src = abs_path?;
     let src_path = std::path::Path::new(&src);
     let filename = src_path.file_name()?;
-    if src_path.exists() {
+    if !src_path.exists() {
-        cover_files.push(src_path.to_path_buf());
+        return None;
     }
+    cover_files.push(src_path.to_path_buf());
     Some(format!("covers/{}", filename.to_string_lossy()))
 }
 
@@ -1532,7 +1595,12 @@ fn write_bar_table(h: &mut HtmlWriter, title: &str, headers: &[&str], rows: &[Ba
     // it can be hidden together with the td.play-count cells on mobile.
     let play_count_idx = headers.len().saturating_sub(2);
     for (i, hdr) in headers.iter().enumerate() {
-        if i == play_count_idx {
+        if *hdr == "Src" {
+            h.linef(format_args!(
+                "<th class=\"source-col\">{}</th>",
+                html_escape(hdr)
+            ));
+        } else if i == play_count_idx {
             h.linef(format_args!(
                 "<th class=\"play-count\">{}</th>",
                 html_escape(hdr)
@@ -1558,8 +1626,17 @@ fn write_bar_table(h: &mut HtmlWriter, title: &str, headers: &[&str], rows: &[Ba
                 h.line("<td><span class=\"mini-ph\"></span></td>");
             }
         }
-        for cell in &row.cells {
+        for (i, cell) in row.cells.iter().enumerate() {
+            if headers.get(i).is_some_and(|h| *h == "Src") {
                 if row.raw_cells {
+                    h.linef(format_args!("<td class=\"source-col\">{}</td>", cell));
+                } else {
+                    h.linef(format_args!(
+                        "<td class=\"source-col\">{}</td>",
+                        html_escape(cell)
+                    ));
+                }
+            } else if row.raw_cells {
                 h.linef(format_args!("<td>{}</td>", cell));
             } else {
                 h.linef(format_args!("<td>{}</td>", html_escape(cell)));
@@ -1658,12 +1735,9 @@ fn html_attr_escape(s: &str) -> String {
 /// applying the same deprioritisation rule as `top_genres`: multi-word genres
 /// rank before single-word broad descriptors ("rock", "electronic", etc.).
 fn top_genre_labels(genre_csv: &str, max_labels: usize) -> Vec<String> {
-    let mut labels: Vec<String> = genre_csv
+    // Reuse the parser the genre ranking uses, so a label rejected there (a
-        .split(',')
+    // review-site domain, say) can never reappear on an album card.
-        .map(str::trim)
+    let mut labels = db::split_genre_list(genre_csv);
-        .filter(|s| !s.is_empty())
-        .map(|s| s.to_string())
-        .collect();
 
     // Stable sort so that multi-word genres always precede single-word ones,
     // preserving the original MusicBrainz order within each group.
@@ -1708,6 +1782,16 @@ mod tests {
         assert_eq!(format_duration(0), "0s");
     }
 
+    #[test]
+    fn test_format_duration_days() {
+        assert_eq!(format_duration(86400), "1d");
+        assert_eq!(format_duration(90000), "1d 1h");
+        // 1d 5h 40m rounds up to 1d 6h.
+        assert_eq!(format_duration(106800), "1d 6h");
+        // Leftover hours rounding up to 24 carries into the next day.
+        assert_eq!(format_duration(172799), "2d");
+    }
+
     #[test]
     fn test_format_play_count() {
         assert_eq!(format_play_count(0), "0 plays");
@@ -1722,6 +1806,57 @@ mod tests {
         assert_eq!(truncate_cell("abcdef", 3), "abc");
     }
 
+    #[test]
+    fn test_print_box_table_survives_row_exceeding_bar_max() {
+        // `top_genres` ranks deprioritised single-word genres last, so the
+        // first row is not necessarily the busiest. Passing an under-stated
+        // bar_max used to underflow `bar_width - filled` and panic mid-report.
+        let rows = vec![
+            vec![
+                "1".to_string(),
+                "alternative metal".to_string(),
+                "1".to_string(),
+            ],
+            vec!["2".to_string(), "pop".to_string(), "9".to_string()],
+        ];
+        // bar_max deliberately smaller than the largest row value.
+        print_box_table(&["#", "Genre", "Plays"], &rows, Some(1), &[1]);
+    }
+
+    #[test]
+    fn test_resolve_cover_skips_missing_file() {
+        let mut files: Vec<std::path::PathBuf> = Vec::new();
+
+        // No cover recorded at all.
+        assert_eq!(resolve_cover(None, &mut files), None);
+        assert!(files.is_empty());
+
+        // A cache row pointing at a file that is no longer on disk must not
+        // produce an <img> — nothing would be copied next to the HTML, so the
+        // page would show a broken image instead of the placeholder.
+        let missing = "/nonexistent/scrbblr/covers/gone.jpg".to_string();
+        assert_eq!(resolve_cover(Some(missing), &mut files), None);
+        assert!(
+            files.is_empty(),
+            "a missing file must not be queued to copy"
+        );
+    }
+
+    #[test]
+    fn test_resolve_cover_accepts_existing_file() {
+        let dir = std::env::temp_dir().join("scrbblr-test-covers");
+        std::fs::create_dir_all(&dir).unwrap();
+        let path = dir.join("present.jpg");
+        std::fs::write(&path, b"not really a jpeg").unwrap();
+
+        let mut files: Vec<std::path::PathBuf> = Vec::new();
+        let rel = resolve_cover(Some(path.to_string_lossy().to_string()), &mut files);
+        assert_eq!(rel.as_deref(), Some("covers/present.jpg"));
+        assert_eq!(files, vec![path.clone()]);
+
+        let _ = std::fs::remove_file(&path);
+    }
+
     #[test]
     fn test_album_cover_grid_limit_rounds_to_full_rows() {
         assert_eq!(album_cover_grid_limit(20), 24);
@@ -1732,9 +1867,10 @@ mod tests {
 
     #[test]
     fn test_top_genre_labels_caps_to_three() {
-        // "alternative rock" and "industrial" are multi-word → rank first.
+        // "alternative rock" is the only multi-word genre → it ranks first.
-        // "electronic" and "darkwave" are single-word → deprioritised.
+        // "electronic", "industrial" and "darkwave" are single-word and so are
-        // With limit 3 we get both multi-word genres + one single-word.
+        // deprioritised, keeping their original order among themselves.
+        // With limit 3 we get the multi-word genre + the first two of those.
         let labels = top_genre_labels("alternative rock, electronic, industrial, darkwave", 3);
         assert_eq!(
             labels,

src/watcher.rs

@@ -127,6 +127,9 @@ impl CurrentTrack {
 ///   track, accumulated across multiple play/pause cycles
 pub struct ScrobbleTracker<F: FnMut(NewScrobble)> {
     current_track: Option<CurrentTrack>,
+    /// Local timestamp captured when tracking for the current track starts.
+    /// Used for report period attribution instead of evaluation time.
+    track_started_at: Option<chrono::NaiveDateTime>,
     is_playing: bool,
     has_seen_status: bool,
     playing_since: Option<Instant>,
@@ -140,6 +143,7 @@ impl<F: FnMut(NewScrobble)> ScrobbleTracker<F> {
     pub fn new(scrobble_fn: F, source: String) -> Self {
         Self {
             current_track: None,
+            track_started_at: None,
             is_playing: false,
             has_seen_status: false,
             playing_since: None,
@@ -205,6 +209,7 @@ impl<F: FnMut(NewScrobble)> ScrobbleTracker<F> {
                     title,
                     duration_us,
                 });
+                self.track_started_at = Some(chrono::Local::now().naive_local());
                 self.accumulated_secs = 0.0;
 
                 // Do not blindly assume metadata implies Playing. If we've
@@ -284,8 +289,10 @@ impl<F: FnMut(NewScrobble)> ScrobbleTracker<F> {
 
             // Only scrobble if the user listened for at least the threshold duration.
             if self.accumulated_secs >= threshold {
-                let now = chrono::Local::now()
+                let attribution_time = self
-                    .naive_local()
+                    .track_started_at
+                    .take()
+                    .unwrap_or_else(|| chrono::Local::now().naive_local())
                     .format("%Y-%m-%dT%H:%M:%S")
                     .to_string();
 
@@ -298,11 +305,13 @@ impl<F: FnMut(NewScrobble)> ScrobbleTracker<F> {
                     title: track.title,
                     track_duration_secs: track_dur,
                     played_duration_secs: self.accumulated_secs.round() as i64,
-                    scrobbled_at: now,
+                    scrobbled_at: attribution_time,
                     source: self.source.clone(),
                 };
                 (self.scrobble_fn)(scrobble);
             }
+
+            self.track_started_at = None;
         }
     }
 }
@@ -446,6 +455,8 @@ pub fn create_db_tracker(
 #[cfg(test)]
 pub struct TestableTracker {
     current_track: Option<CurrentTrack>,
+    /// Simulated timestamp for when current track tracking started.
+    track_started_at_secs: Option<f64>,
     is_playing: bool,
     has_seen_status: bool,
     /// Simulated timestamp (in seconds) when the current Playing stretch began.
@@ -465,6 +476,7 @@ impl TestableTracker {
     pub fn new() -> Self {
         Self {
             current_track: None,
+            track_started_at_secs: None,
             is_playing: false,
             has_seen_status: false,
             playing_since_secs: None,
@@ -499,17 +511,20 @@ impl TestableTracker {
             let threshold = track.threshold_secs();
             if self.accumulated_secs >= threshold {
                 let track_dur = track.duration_secs();
+                let attribution_secs = self.track_started_at_secs.take().unwrap_or(self.clock_secs);
                 let scrobble = NewScrobble {
                     artist: track.artist,
                     album: track.album,
                     title: track.title,
                     track_duration_secs: track_dur,
                     played_duration_secs: self.accumulated_secs.round() as i64,
-                    scrobbled_at: format!("test-time-{}", self.clock_secs),
+                    scrobbled_at: format!("test-time-{}", attribution_secs),
                     source: self.source.clone(),
                 };
                 self.scrobbled.push(scrobble);
             }
+
+            self.track_started_at_secs = None;
         }
     }
 
@@ -541,6 +556,7 @@ impl TestableTracker {
                     title,
                     duration_us,
                 });
+                self.track_started_at_secs = Some(self.clock_secs);
                 self.accumulated_secs = 0.0;
                 if self.has_seen_status {
                     self.playing_since_secs = if self.is_playing {
@@ -1014,6 +1030,32 @@ mod tests {
         assert_eq!(tracker.scrobbled.len(), 0);
     }
 
+    #[test]
+    fn test_scrobble_timestamp_uses_track_start_time() {
+        // Attribute the scrobble to when tracking started, not when the
+        // outgoing track was evaluated.
+        let mut tracker = TestableTracker::new();
+
+        tracker.advance_time(1_000.0);
+        tracker.handle_event(Event::Metadata {
+            artist: "Burial".into(),
+            album: "Untrue".into(),
+            title: "Untrue".into(),
+            duration_us: Some(60_000_000), // threshold = 30s
+        });
+
+        tracker.advance_time(35.0);
+        tracker.handle_event(Event::Metadata {
+            artist: "Deftones".into(),
+            album: "White Pony".into(),
+            title: "Digital Bath".into(),
+            duration_us: Some(291_000_000),
+        });
+
+        assert_eq!(tracker.scrobbled.len(), 1);
+        assert_eq!(tracker.scrobbled[0].scrobbled_at, "test-time-1000");
+    }
+
     #[test]
     fn test_stop_below_threshold_no_scrobble() {
         // Play a track for 3 seconds then stop — should NOT be scrobbled,