Skip to content

Fix artist name splitting with semicolons and Vorbis multi-field handling#3390

Draft
OzGav wants to merge 4 commits intodevfrom
fix-tag-parsing-edge-case
Draft

Fix artist name splitting with semicolons and Vorbis multi-field handling#3390
OzGav wants to merge 4 commits intodevfrom
fix-tag-parsing-edge-case

Conversation

@OzGav
Copy link
Contributor

@OzGav OzGav commented Mar 14, 2026

  1. music_assistant/helpers/tags.py:
  • Fixed artists and album_artists properties to not split on semicolons when values are already in a list (multiple ARTIST fields from Vorbis)
  • Added MB ID count check for singular ARTIST/ALBUMARTIST tags - if count is 1, don't split even on semicolons
  • Added warning in _parse_vorbis_artist_tags() for non-standard ARTISTS/ALBUMARTISTS tags in Vorbis files
  • Added proper comments with spec references
  1. tests/core/test_tags.py:
  • Added test_vorbis_multiple_artist_fields_semicolon_in_name - mock test for multiple ARTIST fields with semicolon
  • Added test_flac_multiple_artist_fields_semicolon_e2e - end-to-end test with real FLAC file
  • Added test_id3_artist_tag_semicolon_single_mbid - test for single ARTIST with semicolon and 1 MB ID
  • Added test_id3_artist_tag_semicolon_multiple_mbids - test for ARTIST with semicolon and multiple MB IDs
  • Added test_id3_albumartist_tag_semicolon_single_mbid - same for album artist
  1. tests/fixtures/ArtistWithSemicolon.flac:
  • New test fixture with multiple ARTIST fields, one containing ave;new

@kepstin

This comment was marked as outdated.

@OzGav OzGav added this to the 2.8.0 milestone Mar 15, 2026
if tag := self.tags.get("artists"):
artists = split_items(tag)
# Runtime check: mutagen returns list[str] for Vorbis multi-field
if isinstance(tag, list) and len(tag) > 1: # type: ignore[unreachable]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These unreachable mypy warnings actually indicate an incorrect type declaration for self.tags. This should be dict[str, str | list[str]] which will also remove the need for these 'type: ignores'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tricky thing is that only the Vorbis parser returns lists (for artists/albumartists) - all other parsers return strings. Changing the type to handle this results in 40+ changes elsewhere. We can't normalize to semicolon-joined strings because the whole point is that Vorbis multi-field tags are already properly separated without delimiter ambiguity - joining and re-splitting would break artist names containing semicolons. Happy to do the 40+ changes if you think it's worth it

@MarvinSchenkel MarvinSchenkel marked this pull request as draft March 16, 2026 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants