Fix embedart: retry pHash compare without colorspaces on empty IM 7 output

[e4779]

Summary

When magick compare -metric PHASH with the -define phash:colorspaces=sRGB,HCLp define (added in a873a19 for score determinism) emits empty stdout AND stderr on ImageMagick ≥7.1.1-44, IMBackend.compare() now retries the pipeline without the define instead of returning None. This restores a working compare_threshold gate for affected users instead of logging "Error while checking art similarity; skipping" on every track.

Fixes #6348.

Root cause

The phash:colorspaces=sRGB,HCLp define was added in a873a19 to make PHASH scores deterministic across IM builds for the same input images. Starting with ImageMagick 7.1.1-44, magick compare sometimes emits no output at all on either stdout or stderr when this define is passed (see ImageMagick/ImageMagick#5191). beets then attempts float(b""), which raises ValueError; the existing handler catches it, logs at debug level, and returns None. The caller in beetsplug/_utils/art.py:52 turns that into the user-facing warning "Error while checking art similarity; skipping", which fires on every track during beet import.

The existing workaround in PR #5650 (5c1817c78) handles only the parenthesized "0.84 (diff)" form — it does not handle the no-output-at-all case.

Fix

In IMBackend.compare():

1. Extract the convert | compare subprocess orchestration and the PHASH output parsing into two helpers (_run_compare_pipeline, _parse_compare_output) so the retry logic can be expressed cleanly.
2. _run_compare_pipeline returns a CompareResult(score, empty_output). The empty_output flag distinguishes the plugin embedart compare_threshold throws error #6348 case (compare exited cleanly with code 0 or 1, but both streams are empty) from a genuine parse error (non-empty but unparseable bytes) or a subprocess failure (non-zero return other than 1).
3. When the first run flags empty_output=True, compare() re-runs the same pipeline with a compare_cmd that omits the phash:colorspaces=sRGB,HCLp define. The score loses cross-build determinism but remains usable for the compare_threshold gate.
4. If the retry also yields None, compare() returns None as before (graceful-failure scenario).

The existing return-code-1 path still works: IM writes the score to stderr when the images differ (returncode = "images differ"); the parser reads stderr for non-zero exits.

Why not just drop the define?

The define gives deterministic PHASH scores for the same images across IM7 builds. Without it, scores vary depending on the default colorspaces / delegate paths of the specific build. Users relying on compare_threshold for consistent behavior between runs would prefer the deterministic path when it works. The retry-with-fallback preserves determinism for the vast majority of IM builds and only falls back to the non-deterministic path for the ones (7.1.1-44 and newer) that otherwise produce no score at all.

Reproduction

I first tried to reproduce this locally on Debian trixie (apt imagemagick = 7.1.1.43+dfsg1-1+deb13u11). On .43 the phash:colorspaces define still works:

$ magick a.png b.png -colorspace gray MIFF:- 2>/dev/null \
    | magick compare -define phash:colorspaces=sRGB,HCLp -metric PHASH - null:
0.910579

The empty-output case happens only on ≥7.1.1-44 builds. beetbox CI runs on ubuntu-latest (=noble), whose apt ships ImageMagick 6.9.12 — the bug is not reproducible there either. The regression tests in this PR are fully mocked through the existing @patch("beets.util.artresizer.subprocess") seam in ArtSimilarityTest, so they exercise the path regardless of which IM version (if any) is installed.

Tests

Two new tests added to ArtSimilarityTest (test/plugins/test_embedart.py):

• test_compare_empty_output_triggers_retry — first Popen pair yields empty streams, retry Popen pair yields a parseable score; asserts the similarity result reflects the retry score.
• test_compare_empty_output_retry_also_fails — both calls empty; asserts None (graceful failure).

Extended the existing _mock_popens helper with optional retry_status/retry_stdout/retry_stderr/retry_convert_status so tests can model the 4-Popen path (convert → compare → convert-retry → compare-retry). The existing 8 tests are unchanged.

Locally: 2588 passed, 142 skipped, 0 unexpected failures.

CI proposal (separate concern)

While working on this, I noticed the require_artresizer_compare decorator (test/plugins/test_embedart.py:33-64) has carried a maintainer TODO for years:

It would be better to investigate what exactly change in IM and handle that in ArtResizer.IMBackend.{can_compare,compare}. Skipping the tests as below is a quick fix to CI, but users may still see unexpected behaviour.

beetbox CI runs on IM 6.9.12 (apt on ubuntu-latest), so test_reject_different_art and test_accept_similar_art are currently skipped via that decorator. Adding an IM7-based job matrix (via a third-party image like dpokidov/imagemagick:7.1.2-12, since Ubuntu/Debian stable have no packaged IM7) would help catch this class of regression in the future. Happy to send that as a separate PR against .github/workflows/ci.yaml if useful — not doing it here to keep this PR focused on the fix.

Checklist

• Tests added (2 new regression tests in ArtSimilarityTest)
• Changelog entry added under "Bug fixes"
• poe test, poe lint, poe format, poe check-types all pass

[codecov]

Codecov Report

❌ Patch coverage is 89.47368% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.63%. Comparing base (4e3884a) to head (ff410f7).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
beets/util/artresizer.py	88.88%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #6861   +/-   ##
=======================================
  Coverage   75.63%   75.63%           
=======================================
  Files         163      163           
  Lines       21312    21333   +21     
  Branches     3360     3363    +3     
=======================================
+ Hits        16119    16136   +17     
- Misses       4401     4403    +2     
- Partials      792      794    +2     

Files with missing lines	Coverage Δ
beets/dbcore/db.py	94.42% <100.00%> (+<0.01%)	⬆️
beets/util/artresizer.py	62.36% <88.88%> (+1.57%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

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