Duplicating Artist if Featured

[gcoppola19]

Description

Fixes #6166

(...)

Checking if an Artist is already present in featured, and not adding it from Discogs plug in

I didn't add chaneglog or documentation, and can do so, but wanted to make sure this was the intended usage and implementation.

• Documentation.
• Changelog. (Add an entry to docs/changelog.rst to the bottom of one of the lists near the top of the document.)
• [ x] Tests. (Very much encouraged but not strictly required.)

[sourcery-ai]

Hey there - I've reviewed your changes - here's some feedback:

• The duplicate-featured-artist check in get_track_info relies on a case-insensitive substring match, which could give false positives (e.g., one artist’s name being a substring of another); consider a more structured comparison (e.g., splitting artist credits into tokens or normalizing to a list of artists) before deciding whether to append.
• The addition of getters["media_types"] = lambda a: a.media_types introduces a new album field without any corresponding use in this PR and with only commented-out tests; either wire this field into actual usage with real tests or drop it from this change to keep the PR focused.
• The change to config_default.yaml appears to be only a whitespace tweak to the plugins/pluginpath lines; consider reverting this unrelated formatting change to keep the diff minimal and focused on the Discogs/artist logic.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The duplicate-featured-artist check in `get_track_info` relies on a case-insensitive substring match, which could give false positives (e.g., one artist’s name being a substring of another); consider a more structured comparison (e.g., splitting artist credits into tokens or normalizing to a list of artists) before deciding whether to append.
- The addition of `getters["media_types"] = lambda a: a.media_types` introduces a new album field without any corresponding use in this PR and with only commented-out tests; either wire this field into actual usage with real tests or drop it from this change to keep the PR focused.
- The change to `config_default.yaml` appears to be only a whitespace tweak to the `plugins`/`pluginpath` lines; consider reverting this unrelated formatting change to keep the diff minimal and focused on the Discogs/artist logic.

## Individual Comments

### Comment 1
<location> `test/test_media_field.py:1-10` </location>
<code_context>
+# import unittest
</code_context>

<issue_to_address>
**issue (testing):** The new `media_types`/`media` behavior is currently untested because this whole test file is commented out.

This file looks like it was meant to validate album media aggregation, but because everything is commented out it no longer exercises the new `Album.media_types` behavior.

Please either:
- Restore and update these tests (import the real `Album`/`Library`/`Item` classes and assert on the public `album.media_types`/`album.media` API), or
- Remove this dead file and add a new test module that explicitly covers `media_types`, including mixed-media albums and relevant edge cases (e.g., missing media values, normalization behavior).

As it stands, the `models.py` change ships without automated coverage, so regressions here won’t be caught by tests.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[jackwilsdon]

Is there something we can do other than a substring match? This is going to avoid appending the featured artist if the main artist contains a subset of the featured artist's name (e.g. ABCD feat. D).

[gcoppola19]

I updated the logic to avoid relying on a substring match, and the new approach normalizes both the main artist and featured artist into lists and compares them, ensuring we only skip appending a featured artist when it is already present. I also added a test case for the example you provided to test it. Would this be a better implementation?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.95%. Comparing base (2bd77b9) to head (0317ae1).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6201      +/-   ##
==========================================
+ Coverage   67.93%   67.95%   +0.02%     
==========================================
  Files         137      137              
  Lines       18677    18690      +13     
  Branches     3155     3157       +2     
==========================================
+ Hits        12688    12701      +13     
  Misses       5324     5324              
  Partials      665      665              

Files with missing lines	Coverage Δ
beetsplug/discogs.py	71.70% <100.00%> (+0.92%)	⬆️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Serene-Arc]

Hi, couple things:

• Why was a test file deleted?
• For anything regarding string parsing, I'd want a ton of tests for edge cases.