test fixes

src/library.rs

@@ -156,82 +156,53 @@ pub async fn add_artist(
         }
     };
 
-    tracing::info!(name = %resolved_name, mbid = %artist_mbid, "fetching discography (release groups)");
+    tracing::info!(name = %resolved_name, mbid = %artist_mbid, "loading discography");
 
-    let all_release_groups = provider
+    // Use the unified discography source — same MBIDs the detail page displays.
-        .get_artist_release_groups(&artist_mbid)
+    // This reads from artist_rg_tracks caches (populated by enrich_artist),
-        .await
+    // ensuring wanted_items always have MBIDs that match the detail page.
-        .map_err(|e| WatchError::Other(format!("failed to fetch release groups: {e}")))?;
+    let disc = load_or_build_discography(conn, &artist_mbid, provider).await;
 
-    // Only include release groups where this artist is the primary credit,
+    if disc.is_empty() {
-    // filtered by allowed secondary types (same as the artist detail page)
+        tracing::warn!(
-    let release_groups: Vec<_> = all_release_groups
+            name = %resolved_name,
-        .into_iter()
+            mbid = %artist_mbid,
-        .filter(|rg| !rg.featured)
+            "no discography data available — visit the artist page first to populate caches"
-        .filter(|rg| {
+        );
-            if rg.secondary_types.is_empty() {
+        return Ok(AddSummary::default());
+    }
+
+    // Deduplicate by MBID and expand into wanted items
+    let mut summary = AddSummary::default();
+    let mut seen_mbids: std::collections::HashSet<String> = std::collections::HashSet::new();
+
+    // Filter by allowed secondary types
+    let filtered: Vec<_> = disc
+        .iter()
+        .filter(|r| {
+            if r.rg_type.is_empty() || r.rg_type == "Album" {
                 true
             } else {
-                rg.secondary_types
+                allowed_secondary_types.iter().any(|st| st == &r.rg_type)
-                    .iter()
-                    .all(|st| allowed_secondary_types.contains(st))
             }
         })
         .collect();
 
-    tracing::info!(count = release_groups.len(), "found release groups");
+    tracing::info!(
+        total = disc.len(),
+        filtered = filtered.len(),
+        "discography loaded"
+    );
 
-    let mut summary = AddSummary::default();
+    for rec in &filtered {
-    let mut seen_mbids: std::collections::HashSet<String> = std::collections::HashSet::new();
+        if !seen_mbids.insert(rec.mbid.clone()) {
-    let mut disc_recordings: Vec<DiscRecording> = Vec::new();
+            continue; // Already processed this recording
-
-    for rg in &release_groups {
-        // Resolve a concrete release MBID from the release group
-        let release_mbid = if let Some(ref rid) = rg.first_release_mbid {
-            rid.clone()
-        } else {
-            // Browse releases for this release group to find a concrete release
-            match provider.resolve_release_from_group(&rg.mbid).await {
-                Ok(rid) => rid,
-                Err(e) => {
-                    tracing::warn!(rg = %rg.title, rg_id = %rg.mbid, error = %e, "failed to resolve release from group");
-                    summary.errors += 1;
-                    continue;
-                }
-            }
-        };
-
-        tracing::info!(title = %rg.title, release_mbid = %release_mbid, "fetching tracks for release group");
-
-        let rg_type = rg.primary_type.clone().unwrap_or_default();
-        let rg_date = rg.first_release_date.clone();
-
-        let tracks = match provider.get_release_tracks(&release_mbid).await {
-            Ok(t) => t,
-            Err(e) => {
-                tracing::warn!(rg = %rg.title, error = %e, "failed to fetch tracks");
-                summary.errors += 1;
-                continue;
-            }
-        };
-
-        for track in &tracks {
-            // Build discography cache entry for every recording
-            disc_recordings.push(DiscRecording {
-                mbid: track.recording_mbid.clone(),
-                title: track.title.clone(),
-                rg_type: rg_type.clone(),
-                rg_date: rg_date.clone(),
-            });
-
-            if !seen_mbids.insert(track.recording_mbid.clone()) {
-                continue; // Already processed this recording in another release group
         }
         match add_track_inner(
             conn,
             &resolved_name,
-                &track.title,
+            &rec.title,
-                Some(&track.recording_mbid),
+            Some(&rec.mbid),
             Some(&artist_mbid),
             user_id,
         )
@@ -240,18 +211,11 @@ pub async fn add_artist(
             Ok(true) => summary.tracks_added += 1,
             Ok(false) => summary.tracks_already_owned += 1,
             Err(e) => {
-                    tracing::warn!(track = %track.title, error = %e, "failed to add track");
+                tracing::warn!(track = %rec.title, error = %e, "failed to add track");
                 summary.errors += 1;
             }
         }
     }
-    }
-
-    // Populate the discography cache so subsequent top-song watches resolve instantly
-    if let Ok(json) = serde_json::to_string(&disc_recordings) {
-        let cache_key = format!("artist_known_recordings:{artist_mbid}");
-        let _ = queries::cache::set(conn, &cache_key, "computed", &json, 7 * 86400).await;
-    }
 
     tracing::info!(%summary, "artist watch complete");
     Ok(summary)
@@ -392,7 +356,7 @@ pub async fn add_track(
 async fn finish_add_track(
     conn: &DatabaseConnection,
     title: &str,
-    artist_name: &str,
+    _artist_name: &str,
     recording_mbid: Option<String>,
     artist: artist::Model,
     user_id: Option<i32>,
@@ -449,7 +413,7 @@ async fn finish_add_track(
         });
     }
 
-    let is_owned = matching::track_is_owned(conn, artist_name, title).await?;
+    let is_owned = matching::track_is_owned(conn, recording_mbid.as_deref()).await?;
 
     let item = queries::wanted::add(
         conn,
@@ -496,8 +460,8 @@ async fn load_and_resolve_discography(
 }
 
 /// Load the discography cache, or build it from the detail page's cached release group
-/// tracks (`artist_rg_tracks:*`). This ensures the MBIDs match what the detail page displays.
+/// tracks (`artist_rg_tracks:*`). Only uses caches populated by enrich_artist() — never
-/// Falls back to MB API only if no detail page caches exist.
+/// fetches from MB API independently, to ensure MBIDs always match the detail page.
 async fn load_or_build_discography(
     conn: &DatabaseConnection,
     artist_mbid: &str,
@@ -505,17 +469,17 @@ async fn load_or_build_discography(
 ) -> Vec<DiscRecording> {
     let cache_key = format!("artist_known_recordings:{artist_mbid}");
 
-    // Try loading from cache (handle both new Vec format and old HashMap format)
+    // Try the pre-built known_recordings cache first
     if let Ok(Some(json)) = queries::cache::get(conn, &cache_key).await {
         if let Ok(recordings) = serde_json::from_str::<Vec<DiscRecording>>(&json) {
             return recordings;
         }
-        // Old format — fall through to rebuild
         tracing::debug!(artist_mbid, "rebuilding discography cache (old format)");
     }
 
-    // Build from the detail page's cached release group tracks.
+    // Build from the detail page's cached release group tracks (artist_rg_tracks:*).
-    // These are the source of truth for which recording MBIDs the detail page will display.
+    // These are populated by enrich_artist() and are the source of truth.
+    // We only use cached data — never fetch from MB API here to avoid MBID divergence.
     let mut recordings = Vec::new();
     if let Ok(release_groups) = provider.get_artist_release_groups(artist_mbid).await {
         for rg in &release_groups {
@@ -525,7 +489,6 @@ async fn load_or_build_discography(
             let rg_type = rg.primary_type.clone().unwrap_or_default();
             let rg_date = rg.first_release_date.clone();
 
-            // Prefer the detail page's cached tracks for this release group
             let rg_cache_key = format!("artist_rg_tracks:{}", rg.mbid);
             if let Ok(Some(json)) = queries::cache::get(conn, &rg_cache_key).await
                 && let Ok(cached) = serde_json::from_str::<serde_json::Value>(&json)
@@ -544,26 +507,15 @@ async fn load_or_build_discography(
                         });
                     }
                 }
-                continue; // Got tracks from detail page cache, skip API
-            }
-
-            // No detail page cache — fetch from API
-            let release_id = rg.first_release_mbid.as_deref().unwrap_or(&rg.mbid);
-            if let Ok(tracks) = provider.get_release_tracks(release_id).await {
-                for t in &tracks {
-                    recordings.push(DiscRecording {
-                        mbid: t.recording_mbid.clone(),
-                        title: t.title.clone(),
-                        rg_type: rg_type.clone(),
-                        rg_date: rg_date.clone(),
-                    });
-                }
             }
+            // No cache for this RG = skip it (enrich_artist hasn't fetched it yet)
         }
     }
 
-    // Cache for 7 days
+    // Cache the result if we found anything
-    if let Ok(json) = serde_json::to_string(&recordings) {
+    if !recordings.is_empty()
+        && let Ok(json) = serde_json::to_string(&recordings)
+    {
         let _ = queries::cache::set(conn, &cache_key, "computed", &json, 7 * 86400).await;
     }
 
@@ -601,7 +553,7 @@ async fn add_track_inner(
         return Ok(false);
     }
 
-    let is_owned = matching::track_is_owned(conn, artist_name, title).await?;
+    let is_owned = matching::track_is_owned(conn, recording_mbid).await?;
 
     let item = queries::wanted::add(
         conn,

src/matching.rs

@@ -87,34 +87,19 @@ pub async fn album_is_owned(
     Ok(false)
 }
 
-/// Check if a specific track is "owned" — a track with matching artist + title exists.
+/// Check if a specific track is "owned" — a track with the same recording MBID exists.
+/// Uses MBID-only matching to avoid false positives from fuzzy name matching
+/// (e.g., "Kill Me" incorrectly matching "How I Get Myself Killed").
 pub async fn track_is_owned(
     conn: &DatabaseConnection,
-    artist_name: &str,
+    recording_mbid: Option<&str>,
-    title: &str,
 ) -> WatchResult<bool> {
-    let norm_artist = normalize(artist_name);
+    if let Some(mbid) = recording_mbid {
-    let norm_title = normalize(title);
+        let all_tracks = queries::tracks::get_by_mbid(conn, mbid).await?;
-
+        if !all_tracks.is_empty() {
-    // Fuzzy: search tracks matching the title
-    let tracks = queries::tracks::search(conn, title).await?;
-    for track in &tracks {
-        let title_match = track
-            .title
-            .as_deref()
-            .map(|t| strsim::jaro_winkler(&norm_title, &normalize(t)) > 0.85)
-            .unwrap_or(false);
-        let artist_match = track
-            .artist
-            .as_deref()
-            .or(track.album_artist.as_deref())
-            .map(|a| strsim::jaro_winkler(&norm_artist, &normalize(a)) > 0.85)
-            .unwrap_or(false);
-        if title_match && artist_match {
             return Ok(true);
         }
     }
-
     Ok(false)
 }
 

tests/integration.rs

@@ -14,7 +14,7 @@ async fn test_db() -> Database {
 }
 
 /// Insert a fake track into the DB to simulate an indexed library.
-async fn insert_track(db: &Database, artist: &str, title: &str, album: &str) {
+async fn insert_track(db: &Database, artist: &str, title: &str, album: &str, mbid: Option<&str>) {
     let now = Utc::now().naive_utc();
     let artist_rec = queries::artists::upsert(db.conn(), artist, None)
         .await
@@ -28,6 +28,7 @@ async fn insert_track(db: &Database, artist: &str, title: &str, album: &str) {
         artist: Set(Some(artist.to_string())),
         album: Set(Some(album.to_string())),
         album_artist: Set(Some(artist.to_string())),
+        musicbrainz_id: Set(mbid.map(String::from)),
         file_size: Set(1_000_000),
         artist_id: Set(Some(artist_rec.id)),
         album_id: Set(Some(album_rec.id)),
@@ -38,6 +39,26 @@ async fn insert_track(db: &Database, artist: &str, title: &str, album: &str) {
     queries::tracks::upsert(db.conn(), active).await.unwrap();
 }
 
+/// Populate the artist_rg_tracks cache so add_artist/add_track can resolve recordings.
+async fn populate_discography_cache(db: &Database, _artist_mbid: &str) {
+    let rg_tracks = serde_json::json!({
+        "release_mbid": "release-123",
+        "tracks": [
+            {"recording_mbid": "rec-1", "title": "Track One"},
+            {"recording_mbid": "rec-2", "title": "Track Two"},
+        ]
+    });
+    queries::cache::set(
+        db.conn(),
+        &format!("artist_rg_tracks:rg-123"),
+        "musicbrainz",
+        &rg_tracks.to_string(),
+        86400,
+    )
+    .await
+    .unwrap();
+}
+
 /// Mock provider that returns a tracklist for known releases.
 struct MockProvider;
 
@@ -60,7 +81,19 @@ impl MetadataProvider for MockProvider {
             score: 100,
         }])
     }
-    async fn get_recording(&self, _mbid: &str) -> DataResult<RecordingDetails> {
+    async fn get_recording(&self, mbid: &str) -> DataResult<RecordingDetails> {
+        // Return details for known test MBIDs
+        if mbid == "rec-time" {
+            return Ok(RecordingDetails {
+                mbid: "rec-time".into(),
+                title: "Time".into(),
+                artist: "Pink Floyd".into(),
+                artist_mbid: None,
+                genres: vec![],
+                releases: vec![],
+                duration_ms: None,
+            });
+        }
         Err(shanty_data::DataError::Other("not found".into()))
     }
     async fn search_artist(
@@ -160,13 +193,31 @@ async fn test_add_track_auto_owned() {
     let db = test_db().await;
     let provider = MockProvider;
 
-    insert_track(&db, "Pink Floyd", "Time", "DSOTM").await;
+    // Create artist with MBID so the fast path can resolve
+    queries::artists::upsert(db.conn(), "Pink Floyd", Some("pf-mbid"))
+        .await
+        .unwrap();
+    // Populate discography cache so fast path finds "Time" → "rec-time"
+    let known = serde_json::json!([
+        {"mbid": "rec-time", "title": "Time", "rg_type": "Album", "rg_date": "1973"}
+    ]);
+    queries::cache::set(
+        db.conn(),
+        "artist_known_recordings:pf-mbid",
+        "computed",
+        &known.to_string(),
+        86400,
+    )
+    .await
+    .unwrap();
+
+    insert_track(&db, "Pink Floyd", "Time", "DSOTM", Some("rec-time")).await;
 
     let entry = add_track(
         db.conn(),
         Some("Pink Floyd"),
         Some("Time"),
-        None,
+        Some("rec-time"),
         &provider,
         None,
     )
@@ -204,6 +255,9 @@ async fn test_add_artist_expands_to_tracks() {
     let db = test_db().await;
     let provider = MockProvider;
 
+    // Pre-populate the discography cache (normally done by enrich_artist)
+    populate_discography_cache(&db, "artist-456").await;
+
     let summary = add_artist(db.conn(), Some("Test Artist"), None, &provider, &[], None)
         .await
         .unwrap();
@@ -278,7 +332,24 @@ async fn test_library_summary() {
     let db = test_db().await;
     let provider = MockProvider;
 
-    insert_track(&db, "Pink Floyd", "Time", "DSOTM").await;
+    // Create artist with MBID and populate discography cache
+    queries::artists::upsert(db.conn(), "Pink Floyd", Some("pf-mbid"))
+        .await
+        .unwrap();
+    let known = serde_json::json!([
+        {"mbid": "rec-time", "title": "Time", "rg_type": "Album", "rg_date": "1973"}
+    ]);
+    queries::cache::set(
+        db.conn(),
+        "artist_known_recordings:pf-mbid",
+        "computed",
+        &known.to_string(),
+        86400,
+    )
+    .await
+    .unwrap();
+
+    insert_track(&db, "Pink Floyd", "Time", "DSOTM", Some("rec-time")).await;
 
     add_track(
         db.conn(),
@@ -294,7 +365,7 @@ async fn test_library_summary() {
         db.conn(),
         Some("Pink Floyd"),
         Some("Time"),
-        None,
+        Some("rec-time"),
         &provider,
         None,
     )