fix: split multi-artist tags from yt-dlp downloads into separate artists

crates/pages/src/ytdlp.rs

@@ -3,7 +3,7 @@ use dioxus::core::spawn_forever;
 use dioxus::prelude::*;
 use std::fs::{self, OpenOptions};
 use std::io::BufRead;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 /// App-lifetime job list: downloads keep running (and keep their live
 /// progress) when the user navigates away from the page (#327).
@@ -242,6 +242,7 @@ fn build_command(
     out: &str,
     fmt: AudioFormat,
     opts: &YtdlpOptions,
+    artists_manifest: Option<&Path>,
 ) -> std::process::Command {
     let binary = find_ytdlp();
     let mut cmd = std::process::Command::new(&binary);
@@ -294,6 +295,15 @@ fn build_command(
 
     if opts.embed_metadata {
         cmd.arg("--embed-metadata");
+        // yt-dlp embeds multiple artists as one comma-joined string, which the
+        // library scanner can't split safely (a comma may be part of a name).
+        // Record the structured artists list per finished file so we can
+        // rewrite the tag unambiguously afterwards (issue #314).
+        if let Some(manifest) = artists_manifest {
+            cmd.arg("--print-to-file")
+                .arg("after_move:%(artists)j\t%(filepath)s")
+                .arg(manifest);
+        }
     }
     if opts.embed_thumbnail {
         cmd.arg("--embed-thumbnail");
@@ -390,6 +400,42 @@ fn build_command(
     cmd
 }
 
+/// One manifest line per downloaded file, printed by yt-dlp after the final
+/// move: `<artists as JSON array>\t<filepath>`. JSON escapes raw tabs, so
+/// splitting on the first tab is unambiguous. Returns `None` for files whose
+/// extractor exposes no structured artists list (the JSON side is `null`/`NA`).
+fn parse_artist_manifest_line(line: &str) -> Option<(Vec<String>, PathBuf)> {
+    let (json_part, path_part) = line.split_once('\t')?;
+    let artists: Vec<String> = serde_json::from_str(json_part).ok()?;
+    if path_part.trim().is_empty() {
+        return None;
+    }
+    Some((artists, PathBuf::from(path_part)))
+}
+
+/// Rewrite the artist tag of each downloaded file from the structured artists
+/// list recorded in the manifest (issue #314). yt-dlp embeds multiple artists
+/// as one comma-joined string; rejoining the structured list with `;` lets the
+/// library scanner split it without guessing, so a single artist whose name
+/// contains a comma (e.g. "Tyler, The Creator") is left untouched.
+/// Best-effort: a failed rewrite only logs, the file keeps yt-dlp's own tag.
+fn apply_artist_manifest(manifest: &Path) {
+    let Ok(content) = fs::read_to_string(manifest) else {
+        return;
+    };
+    for line in content.lines() {
+        let Some((artists, file)) = parse_artist_manifest_line(line) else {
+            continue;
+        };
+        if artists.len() < 2 {
+            continue;
+        }
+        if let Err(e) = reader::set_artist_tag(&file, &artists) {
+            tracing::warn!(file = %file.display(), error = %e, "multi-artist tag rewrite failed");
+        }
+    }
+}
+
 enum LineInfo {
     Progress {
         pct: f64,
@@ -538,9 +584,13 @@ pub fn YtdlpPage(config: Signal<AppConfig>) -> Element {
             }
 
             let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::<LineInfo>();
+            let blocking_job_id = job_id.clone();
 
             tokio::task::spawn_blocking(move || {
-                let mut cmd = build_command(&url, &out, fmt, &opts);
+                let artists_manifest = opts.embed_metadata.then(|| {
+                    std::env::temp_dir().join(format!("kopuz-ytdlp-artists-{blocking_job_id}.tsv"))
+                });
+                let mut cmd = build_command(&url, &out, fmt, &opts, artists_manifest.as_deref());
 
                 let mut child = match cmd.spawn() {
                     Ok(c) => c,
@@ -591,6 +641,9 @@ pub fn YtdlpPage(config: Signal<AppConfig>) -> Element {
                 match child.wait() {
                     Ok(s) if s.success() => {
                         tracing::info!(target: "ytdlp", "yt-dlp: download finished");
+                        if let Some(manifest) = &artists_manifest {
+                            apply_artist_manifest(manifest);
+                        }
                         let _ = tx.send(LineInfo::Done);
                     }
                     Ok(s) => {
@@ -605,6 +658,9 @@ pub fn YtdlpPage(config: Signal<AppConfig>) -> Element {
                         let _ = tx.send(LineInfo::Error(e.to_string()));
                     }
                 }
+                if let Some(manifest) = &artists_manifest {
+                    let _ = fs::remove_file(manifest);
+                }
             });
 
             while let Some(info) = rx.recv().await {
@@ -1138,3 +1194,66 @@ fn JobRow(props: JobRowProps) -> Element {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn manifest_line_parses_artists_and_path() {
+        let line = "[\"Kero Kero Bonito\", \"Douglas Lobban\"]\t/music/Album/song.opus";
+        let (artists, path) = parse_artist_manifest_line(line).expect("should parse");
+        assert_eq!(artists, vec!["Kero Kero Bonito", "Douglas Lobban"]);
+        assert_eq!(path, PathBuf::from("/music/Album/song.opus"));
+    }
+
+    #[test]
+    fn manifest_line_without_structured_artists_is_skipped() {
+        // Extractors without an artists list print null/NA on the JSON side.
+        assert!(parse_artist_manifest_line("null\t/music/x.opus").is_none());
+        assert!(parse_artist_manifest_line("NA\t/music/x.opus").is_none());
+        assert!(parse_artist_manifest_line("no tab here").is_none());
+        assert!(parse_artist_manifest_line("[\"A\", \"B\"]\t").is_none());
+    }
+
+    #[test]
+    fn apply_manifest_rewrites_multi_artist_tag() {
+        use lofty::file::TaggedFileExt;
+        use lofty::probe::Probe;
+        use lofty::tag::Accessor;
+
+        let fixture =
+            Path::new(env!("CARGO_MANIFEST_DIR")).join("../reader/tests/fixtures/comma.opus");
+        let dir = std::env::temp_dir().join(format!("kopuz-ytdlp-test-{}", std::process::id()));
+        fs::create_dir_all(&dir).unwrap();
+        let media = dir.join("comma.opus");
+        fs::copy(&fixture, &media).unwrap();
+
+        let manifest = dir.join("artists.tsv");
+        fs::write(
+            &manifest,
+            format!(
+                "[\"Kero Kero Bonito\", \"Douglas Lobban\", \"Sarah Perry\"]\t{}\n\
+                 [\"Tyler, The Creator\"]\t{}\n",
+                media.display(),
+                media.display()
+            ),
+        )
+        .unwrap();
+
+        apply_artist_manifest(&manifest);
+
+        let tagged = Probe::open(&media).unwrap().read().unwrap();
+        let artist = tagged
+            .primary_tag()
+            .and_then(|t| t.artist().map(|a| a.to_string()));
+        // Multi-artist line rewrites with ';'; the single-artist (comma-in-name)
+        // line must NOT rewrite, so the ';'-joined value survives it.
+        assert_eq!(
+            artist.as_deref(),
+            Some("Kero Kero Bonito;Douglas Lobban;Sarah Perry")
+        );
+
+        let _ = fs::remove_dir_all(&dir);
+    }
+}

crates/reader/src/lib.rs

@@ -9,7 +9,7 @@ pub mod scanner;
 pub mod utils;
 
 #[cfg(not(target_arch = "wasm32"))]
-pub use metadata::{read, read_cover, write_tags};
+pub use metadata::{read, read_cover, set_artist_tag, write_tags};
 pub use models::{
     Album, ArtistImageRef, CoverChange, FavoritesStore, Library, PlaylistFolder, PlaylistStore,
     Track, TrackEdits, TrackId,

crates/reader/src/metadata.rs

@@ -90,6 +90,11 @@ pub fn extract_metadata(
         })
         .unwrap_or_else(|| vec![artist.clone()]);
 
+    // The singular field carries the primary (first) credited artist; keeping
+    // the joined "A;B;C" string here would leak into the album-artist fallback
+    // and show up as a phantom artist in the library (issue #314).
+    let artist = artists.first().cloned().unwrap_or(artist);
+
     let album_title = tag.and_then(|t| t.album().map(|a| a.to_string()));
 
     let album_artist = tag
@@ -318,6 +323,41 @@ pub fn write_tags(track_path: &Path, edits: &TrackEdits) -> Result<(), String> {
         .map_err(|e| e.to_string())
 }
 
+/// Rewrite the artist tag of a downloaded file from a structured artist list,
+/// joining with `;` so the scanner splits it back into individual artists.
+/// Used by the yt-dlp download fix-up (issue #314): yt-dlp embeds multiple
+/// artists as one comma-joined string, which is ambiguous (a comma may be part
+/// of a real name), while the structured list is not.
+pub fn set_artist_tag(track_path: &Path, artists: &[String]) -> Result<(), String> {
+    use lofty::config::WriteOptions;
+    use lofty::file::AudioFile;
+
+    if artists.is_empty() {
+        return Ok(());
+    }
+
+    let mut tagged = Probe::open(track_path)
+        .map_err(|e| e.to_string())?
+        .read()
+        .map_err(|e| e.to_string())?;
+
+    if tagged.primary_tag().is_none() {
+        let tag_type = tagged.primary_tag_type();
+        tagged.insert_tag(Tag::new(tag_type));
+    }
+    let tag = tagged
+        .primary_tag_mut()
+        .ok_or_else(|| "no writable tag for this format".to_string())?;
+
+    tag.set_artist(artists.join(";"));
+    // Stale structured values would win over the artist field on scan.
+    tag.remove_key(ItemKey::TrackArtists);
+
+    tagged
+        .save_to_path(track_path, WriteOptions::default())
+        .map_err(|e| e.to_string())
+}
+
 /// Read the embedded front-cover picture (or best available) as raw bytes plus
 /// its MIME type, for previewing in the metadata editor. `None` if the file has
 /// no embedded artwork.

crates/reader/tests/artist_parsing.rs

@@ -0,0 +1,80 @@
+//! Regression tests for multi-artist tag parsing (issue #314).
+//!
+//! Downloaded files often carry a single joined `ARTIST` tag. The scanner
+//! must split `;`-separated values into `artists`, expose the primary
+//! artist via the singular `artist` field, and never guess on commas
+//! (names like "Tyler, The Creator" stay intact).
+
+use reader::{Library, Track, read};
+use std::path::{Path, PathBuf};
+
+fn fixture(name: &str) -> PathBuf {
+    Path::new(env!("CARGO_MANIFEST_DIR"))
+        .join("tests/fixtures")
+        .join(name)
+}
+
+fn read_fixture(name: &str) -> (Track, Library) {
+    let cache = std::env::temp_dir();
+    let mut lib = Library::default();
+    let track = read(&fixture(name), &cache, &mut lib)
+        .unwrap_or_else(|| panic!("failed to read fixture {name}"));
+    (track, lib)
+}
+
+fn album_artist(lib: &Library) -> String {
+    lib.albums
+        .first()
+        .map(|a| a.artist.clone())
+        .expect("album should be created")
+}
+
+#[test]
+fn single_artist_unchanged() {
+    let (track, lib) = read_fixture("single.opus");
+    assert_eq!(track.artist, "Solo Artist");
+    assert_eq!(track.artists, vec!["Solo Artist"]);
+    assert_eq!(album_artist(&lib), "Solo Artist");
+}
+
+#[test]
+fn semicolon_tag_splits_and_primary_artist_is_first() {
+    let (track, lib) = read_fixture("semicolon.opus");
+    assert_eq!(
+        track.artists,
+        vec!["Kero Kero Bonito", "Douglas Lobban", "Sarah Perry"]
+    );
+    // Singular artist must be the primary (first) artist, not the joined
+    // string, otherwise the artists page shows a phantom "A;B;C" entry.
+    assert_eq!(track.artist, "Kero Kero Bonito");
+    // Album artist falls back to track.artist when no ALBUMARTIST tag.
+    assert_eq!(album_artist(&lib), "Kero Kero Bonito");
+}
+
+#[test]
+fn comma_tag_is_not_guessed() {
+    // Comma-joined tags stay as-is on the read side: a comma may be part
+    // of a real name, so splitting is the writer's job (yt-dlp download fix).
+    let (track, lib) = read_fixture("comma.opus");
+    let joined = "Kero Kero Bonito, Douglas Lobban, Sarah Perry";
+    assert_eq!(track.artist, joined);
+    assert_eq!(track.artists, vec![joined]);
+    assert_eq!(album_artist(&lib), joined);
+}
+
+#[test]
+fn comma_in_artist_name_stays_intact() {
+    let (track, lib) = read_fixture("tyler.opus");
+    assert_eq!(track.artist, "Tyler, The Creator");
+    assert_eq!(track.artists, vec!["Tyler, The Creator"]);
+    assert_eq!(album_artist(&lib), "Tyler, The Creator");
+}
+
+#[test]
+fn explicit_album_artist_wins() {
+    let (track, lib) = read_fixture("albumartist.opus");
+    assert_eq!(track.artists, vec!["First One", "Second One"]);
+    assert_eq!(track.artist, "First One");
+    // ALBUMARTIST tag takes precedence over the track artist fallback.
+    assert_eq!(album_artist(&lib), "The Band");
+}