From 2444c93d482645106fa7b69c8d0e9e11af25079f Mon Sep 17 00:00:00 2001
From: Connor Johnstone <connor.johnstone@arcfield.com>
Date: Wed, 25 Mar 2026 16:25:35 -0400
Subject: [PATCH] fix for bad mbid

---
 src/tagger.rs | 120 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 72 insertions(+), 48 deletions(-)

diff --git a/src/tagger.rs b/src/tagger.rs
index ff7f055..2246a0f 100644
--- a/src/tagger.rs
+++ b/src/tagger.rs
@@ -1,5 +1,5 @@
 use sea_orm::{ActiveValue::NotSet, ActiveValue::Set, DatabaseConnection};
-use shanty_data::MetadataFetcher as MetadataProvider;
+use shanty_data::{MetadataFetcher as MetadataProvider, RecordingDetails, ReleaseRef};
 use shanty_db::entities::track;
 use shanty_db::queries;
 
@@ -42,60 +42,42 @@ 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
     };
 
+    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);
+    }
+
     // 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() {
@@ -152,7 +134,6 @@ pub async fn tag_track(
         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 +164,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,