Skip to content

Add opt-in pref mergeContributorsByName for album-artist contributor matching#1579

Open
catcatcatcatcatcatcatcatcatcat wants to merge 2 commits into
LMS-Community:public/9.0from
catcatcatcatcatcatcatcatcatcat:fix/merge-contributors-by-name
Open

Add opt-in pref mergeContributorsByName for album-artist contributor matching#1579
catcatcatcatcatcatcatcatcatcat wants to merge 2 commits into
LMS-Community:public/9.0from
catcatcatcatcatcatcatcatcatcat:fix/merge-contributors-by-name

Conversation

@catcatcatcatcatcatcatcatcatcat
Copy link
Copy Markdown

Add opt-in pref mergeContributorsByName for album-artist contributor matching

Fixes #1578.

Summary

Adds a server preference, mergeContributorsByName, that changes the contributor lookup in Slim::Schema::Contributor::add from "MBID first, name fallback only if existing row has NULL MBID" to "name first, MBID fallback if no name match." Default is OFF; existing behavior is preserved.

Motivation

Same display name, different MBID → two contributor rows under the same name in the browse UI. The most common trigger is band/collaboration credits on MusicBrainz that use the solo artist's name string in ALBUMARTIST (e.g. "Stephen Malkmus" credited to "Stephen Malkmus & The Jicks" on MusicBrainz; "Mirah" credited to a multi-artist collaboration). See the linked issue for a full reproduction, source-code analysis, and empirical confirmation via a per-track tag experiment.

Change

Slim/Schema/Contributor.pm: in sub add, gate the existing MBID-first lookup on !$prefs->get('mergeContributorsByName'). When the pref is on, look up by name first, tie-breaking by preferring rows with non-null musicbrainz_id (richer data) then by id ascending (oldest first) for determinism. Fall back to MBID lookup only if no name match exists.

Slim/Utils/Prefs.pm (or wherever scanner-related prefs are defaulted, depending on layout): default mergeContributorsByName => 0.

Settings UI: add a checkbox under My Music → MP3 (or a more general "Contributor scanning" subsection) with the help text:

When enabled, contributors are matched by name first, even if MusicBrainz IDs differ. Use this if MusicBrainz credits cause the same artist to appear as separate browse entries (e.g. a band-name credit alongside solo releases under the same display name). Leave off if you have legitimately distinct artists who share a display name and rely on MBIDs to keep them apart.

Tradeoffs and why default-OFF is correct

The current MBID-first behavior is the right default for libraries that contain legitimately distinct artists with the same display name (e.g. John Williams the composer vs. John Williams the guitarist) — those users want MBIDs to keep their rows separate, and a global "match by name" would regress them. Making the new behavior opt-in preserves both populations.

Alternatives considered

  • Per-name allowlist. More surgical (you can merge "Stephen Malkmus" while still distinguishing the John Williamses) but requires a settings-page UI for managing the list. Worth doing as a follow-up; the global toggle is the smallest patch that fixes the immediate complaint.
  • Always prefer name. Cleaner code but a regression for the ambiguous-name population.
  • Reverse: blocklist of MBIDs to ignore. Bad UX (exposes UUIDs to users).

Test plan

Manual verification on a library exhibiting the bug (Stephen Malkmus and Mirah examples from the issue):

  1. With pref OFF (default): clear+rescan, verify contributors table still contains both rows (existing behavior preserved).
  2. With pref ON: clear+rescan, verify contributors table now has a single row per display name; the orphaned rows are pruned by Contributor::rescan once they have no remaining contributor_track references.
  3. Mixed-MBID library with legitimately distinct same-name artists (synthetic test): with pref OFF, verify they stay separate. With pref ON, verify they collapse (expected; user opted in).
  4. Rescan idempotency: scan twice in a row with pref ON, confirm no row count drift.

Patch

See Contributor.pm.diff alongside this PR description.

Note on the existing no-MBID branch (SELECT … LIMIT 1 with no ORDER BY)

While debugging the underlying issue I also noticed that the existing no-MBID name lookup is non-deterministic across rescans because it uses LIMIT 1 without ORDER BY. This makes the documented "strip MBID frames to merge" workaround unreliable — in my own library it picked the canonical contributor for one duplicate and the duplicate row for another. I have not changed that path in this PR (out of scope, conservative diff), but it would be a worthwhile follow-up to add ORDER BY id LIMIT 1 there as well for reproducibility. Happy to do that in a separate PR if the maintainers agree.

…matching

When two tracks share the same ALBUMARTIST display string but carry
different MusicBrainz Album Artist IDs, LMS creates duplicate contributor
rows under the same visible name. This adds a server preference
(mergeContributorsByName, default OFF) that makes Contributor::add look
up by name first and fall back to MBID only if no name match exists,
preventing the split. Default-off preserves existing behaviour for
libraries that legitimately use MBIDs to distinguish same-name artists.

- Slim/Schema/Contributor.pm: new lookup branch in sub add
- Slim/Utils/Prefs.pm: default declaration + rescan trigger
- HTML/EN/settings/server/behavior.html: checkbox in My Music section
- strings.txt: EN strings for the new setting
@catcatcatcatcatcatcatcatcatcat catcatcatcatcatcatcatcatcatcat marked this pull request as draft May 24, 2026 04:01
- Slim/Web/Settings/Server/Behavior.pm: add mergeContributorsByName to
  the sub prefs whitelist so the setting page can read and save it
- Changelog9.html: note the new preference under v9.0.4 New Features

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@catcatcatcatcatcatcatcatcatcat catcatcatcatcatcatcatcatcatcat marked this pull request as ready for review May 24, 2026 04:24
@catcatcatcatcatcatcatcatcatcat
Copy link
Copy Markdown
Author

@darrell-k — tagging you as you've done the recent contributor/MusicBrainz work (#1325, #1331) and are likely the right person to review this. Happy to adjust the pref name, settings placement, or anything else based on your feedback.

@michaelherger
Copy link
Copy Markdown
Member

How is this different from "ignore MB tags" - which can easily be fixed by removing the tags?

@michaelherger
Copy link
Copy Markdown
Member

This definitely steps into territory @darrell-k has been working on and discussing recently. I'm tempted to decline this PR for the simple reason that it's just adding another pref to deal with some music files' incorrect tagging. Fix the tags and we don't need to maintain another code branch. Wait for @darrell-k's latest work. Join the community where I believe this topic has been discussed extensively recently in a larger and more comprehensive context.

Oh, and certainly rebase on top of 9.2 to get started. Respect the project's expectations (DCO), declare the use of Claude or whatever other tool did the work, tell it to keep comments short and sweet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants