still getting extra artists. need to test on my deployment

src/file_tags.rs

@@ -22,6 +22,7 @@ fn tag_type_for_file(ft: FileType) -> TagType {
 }
 
 /// Write updated metadata back to the music file's embedded tags.
+/// Skips the write if all tags already match (avoids expensive FLAC rewrites).
 pub fn write_tags(
     file_path: &str,
     details: &RecordingDetails,
@@ -46,6 +47,39 @@ pub fn write_tags(
         .cloned()
         .unwrap_or_else(|| lofty::tag::Tag::new(tag_type));
 
+    // Check if all tags already match — skip the expensive write if so
+    let existing_title = tag.title().map(|s| s.to_string());
+    let existing_artist = tag.artist().map(|s| s.to_string());
+    let existing_album = tag.album().map(|s| s.to_string());
+    let existing_year = tag.year();
+    let existing_genre = tag.genre().map(|s| s.to_string());
+
+    let want_album = release.map(|r| r.title.clone());
+    let want_year = year.map(|y| y as u32);
+    let want_genre = genre.map(|g| g.to_string());
+
+    let title_ok = existing_title.as_deref() == Some(&details.title);
+    let artist_ok = existing_artist.as_deref() == Some(&details.artist);
+    let album_ok = match (&existing_album, &want_album) {
+        (Some(e), Some(w)) => e == w,
+        (None, None) => true,
+        _ => false,
+    };
+    let year_ok = existing_year == want_year;
+    let genre_ok = match (&existing_genre, &want_genre) {
+        (Some(e), Some(w)) => e == w,
+        (None, None) => true,
+        _ => false,
+    };
+
+    if title_ok && artist_ok && album_ok && year_ok && genre_ok {
+        tracing::debug!(
+            path = file_path,
+            "file tags already correct, skipping write"
+        );
+        return Ok(());
+    }
+
     // Set metadata
     tag.set_title(details.title.clone());
     tag.set_artist(details.artist.clone());

src/tagger.rs

@@ -1,5 +1,5 @@
-use sea_orm::{ActiveValue::NotSet, ActiveValue::Set, DatabaseConnection};
-use shanty_data::MetadataFetcher as MetadataProvider;
+use sea_orm::{ActiveModelTrait, ActiveValue::NotSet, ActiveValue::Set, DatabaseConnection};
+use shanty_data::{MetadataFetcher as MetadataProvider, RecordingDetails, ReleaseRef};
 use shanty_db::entities::track;
 use shanty_db::queries;
 
@@ -42,79 +42,74 @@ pub async fn tag_track(
     track: &track::Model,
     config: &TagConfig,
 ) -> TagResult<bool> {
-    // If the track already has an MBID, skip searching and go straight to lookup
-    let (details, best_release) = if let Some(ref mbid) = track.musicbrainz_id {
+    // Resolve recording details — try MBID lookup first, fall back to search
+    let resolved = if let Some(ref mbid) = track.musicbrainz_id {
         tracing::info!(id = track.id, mbid = %mbid, "looking up recording by MBID");
 
-        if config.dry_run {
-            tracing::info!(id = track.id, mbid = %mbid, "DRY RUN: would enrich from MBID");
-            return Ok(true);
-        }
-
-        let details = provider.get_recording(mbid).await?;
-        let best_release = crate::matcher::pick_best_release(track, &details.releases);
-        (details, best_release)
-    } else {
-        // No MBID — search by artist + title
-        let (artist, title) = match matcher::build_query(track) {
-            Some(q) => q,
-            None => {
-                tracing::debug!(id = track.id, path = %track.file_path, "no query possible, skipping");
-                return Ok(false);
+        match provider.get_recording(mbid).await {
+            Ok(details) => {
+                let best_release = matcher::pick_best_release(track, &details.releases);
+                Some((details, best_release))
             }
-        };
-
-        tracing::info!(id = track.id, artist = %artist, title = %title, "searching MusicBrainz");
-
-        let candidates = provider.search_recording(&artist, &title).await?;
-
-        if candidates.is_empty() {
-            tracing::debug!(id = track.id, "no results from MusicBrainz");
-            return Ok(false);
-        }
-
-        let best = match matcher::select_best_match(track, candidates, config.confidence) {
-            Some(m) => m,
-            None => {
-                tracing::debug!(
-                    id = track.id,
-                    "no match above confidence threshold {}",
-                    config.confidence
+            Err(e) => {
+                tracing::warn!(
+                    id = track.id, mbid = %mbid, error = %e,
+                    "MBID lookup failed, falling back to search"
                 );
-                return Ok(false);
+                None
             }
-        };
-
-        log_match(track, &best);
-
-        if config.dry_run {
-            return Ok(true);
         }
-
-        let details = provider.get_recording(&best.recording.mbid).await?;
-        let best_release = best.best_release;
-        (details, best_release)
+    } else {
+        None
     };
 
-    // Use existing artist_id if already set (e.g., from download pipeline).
-    // Only upsert from MB when the track has no artist association yet.
-    let artist_id = if track.artist_id.is_some() {
-        track.artist_id
-    } else {
-        match &details.artist_mbid {
-            Some(mbid) => Some(
-                queries::artists::upsert(conn, &details.artist, Some(mbid))
-                    .await?
-                    .id,
-            ),
-            None => Some(
-                queries::artists::upsert(conn, &details.artist, None)
-                    .await?
-                    .id,
-            ),
+    let (details, best_release) = match resolved {
+        Some(r) => r,
+        None => {
+            // Search by artist + title
+            match search_and_match(track, provider, config).await? {
+                Some(r) => r,
+                None => return Ok(false),
+            }
         }
     };
 
+    if config.dry_run {
+        return Ok(true);
+    }
+
+    // If the resolved MBID differs from the track's original MBID, update the wanted_item
+    // so the cleanup step doesn't delete files whose MBID changed during tagging
+    if let Some(ref old_mbid) = track.musicbrainz_id
+        && old_mbid != &details.mbid
+        && let Ok(Some(wanted)) = queries::wanted::find_by_mbid(conn, old_mbid).await
+    {
+        let mut active: shanty_db::entities::wanted_item::ActiveModel = wanted.into();
+        active.musicbrainz_id = Set(Some(details.mbid.clone()));
+        let _ = active.update(conn).await;
+        tracing::info!(
+            old_mbid = %old_mbid,
+            new_mbid = %details.mbid,
+            "updated wanted_item MBID after tagger fallback"
+        );
+    }
+
+    // Always resolve artist_id from MB data — this is the authoritative source for the
+    // primary artist. The indexer or download worker may have set artist_id to a collaborator
+    // artist (e.g., "Bass Drum of Death, Not Documented"), so we always correct it here.
+    let artist_id = match &details.artist_mbid {
+        Some(mbid) => Some(
+            queries::artists::upsert(conn, &details.artist, Some(mbid))
+                .await?
+                .id,
+        ),
+        None => Some(
+            queries::artists::upsert(conn, &details.artist, None)
+                .await?
+                .id,
+        ),
+    };
+
     // Upsert album from best release
     let (album_id, album_name) = if let Some(ref release) = best_release {
         let album = queries::albums::upsert(
@@ -139,20 +134,27 @@ pub async fn tag_track(
 
     let genre = details.genres.first().cloned();
 
-    // Update track metadata
+    // Update track metadata — preserve artist name when artist_id was already set
+    let artist_name_for_track = if track.artist_id.is_some() {
+        track
+            .artist
+            .clone()
+            .or_else(|| Some(details.artist.clone()))
+    } else {
+        Some(details.artist.clone())
+    };
     let active = track::ActiveModel {
         id: Set(track.id),
         file_path: Set(track.file_path.clone()),
         title: Set(Some(details.title.clone())),
-        artist: Set(Some(details.artist.clone())),
+        artist: Set(artist_name_for_track.clone()),
         album: Set(album_name),
-        album_artist: Set(Some(details.artist.clone())),
+        album_artist: Set(artist_name_for_track),
         musicbrainz_id: Set(Some(details.mbid.clone())),
         artist_id: Set(artist_id),
         album_id: Set(album_id),
         year: Set(year),
         genre: Set(genre.clone()),
-        // Preserve existing values for fields we don't update
         track_number: NotSet,
         disc_number: NotSet,
         duration: NotSet,
@@ -183,6 +185,49 @@ pub async fn tag_track(
     Ok(true)
 }
 
+/// Search MusicBrainz by artist+title and return the best match.
+/// Returns None if no query is possible or no match exceeds the confidence threshold.
+async fn search_and_match(
+    track: &track::Model,
+    provider: &impl MetadataProvider,
+    config: &TagConfig,
+) -> TagResult<Option<(RecordingDetails, Option<ReleaseRef>)>> {
+    let (artist, title) = match matcher::build_query(track) {
+        Some(q) => q,
+        None => {
+            tracing::debug!(id = track.id, path = %track.file_path, "no query possible, skipping");
+            return Ok(None);
+        }
+    };
+
+    tracing::info!(id = track.id, artist = %artist, title = %title, "searching MusicBrainz");
+
+    let candidates = provider.search_recording(&artist, &title).await?;
+
+    if candidates.is_empty() {
+        tracing::debug!(id = track.id, "no results from MusicBrainz");
+        return Ok(None);
+    }
+
+    let best = match matcher::select_best_match(track, candidates, config.confidence) {
+        Some(m) => m,
+        None => {
+            tracing::debug!(
+                id = track.id,
+                "no match above confidence threshold {}",
+                config.confidence
+            );
+            return Ok(None);
+        }
+    };
+
+    log_match(track, &best);
+
+    let details = provider.get_recording(&best.recording.mbid).await?;
+    let best_release = best.best_release;
+    Ok(Some((details, best_release)))
+}
+
 fn log_match(track: &track::Model, best: &ScoredMatch) {
     tracing::info!(
         id = track.id,