mirror of https://github.com/Nezreka/SoulSync.git
Closes #587. Three coordinated fixes per codex's diagnosis. AcoustID verification gate left intact — these fixes target the upstream scanner false-positive surface plus a separate retag-path gap. Bug 1 — scanner used recordings[0] as authoritative `core/repair_jobs/acoustid_scanner.py:_scan_file` only checked the top fingerprint match's metadata. AcoustID often returns multiple recordings per fingerprint (sample collisions, multi-MB-record cases) and the wrong-credited recording can outrank the right- credited one. Foxxify case 2 (Nana / Nana): top match credited the wrong artist while a lower-ranked candidate matched the user's expected metadata exactly. Lifted the verifier's all-candidates check to a shared pure helper `core/matching/acoustid_candidates.py:find_matching_recording`. Both verifier and scanner can now ask "given these candidates, does ANY of them match expected (title, artist)?" with the same contract. Scanner suppresses the finding when any candidate matches. Bug 2 — no duration check guards against fingerprint hash collisions Foxxify case 3: 17-minute mashup edit fingerprinted to a 5-minute late-70s Japanese hiphop track (different songs, fingerprint hash collision on a sampled section). Scanner had no signal to detect this and would have recommended retagging the 17-min file as the 5-min track. `duration_mismatches_strongly` in the same helper module flags drifts beyond max(60s, 35%). Scanner now skips findings when the candidate's duration disagrees strongly with the file's expected duration. Loaded duration via the existing tracks SQL (added `t.duration` to the SELECT). Returns False when either side is unknown — no behavior change for older rows without duration data. Bug 3 — scanner retag bypassed multi-value ARTISTS tag setting `core/repair_worker.py:_fix_wrong_song` called `write_tags_to_file` with single-string artist updates. The writer only wrote TPE1 (single string) and never read the user's `metadata_enhancement.tags.write_multi_artist` config. Multi-value ARTISTS tags got stripped on every retag, contradicting the post-download enrichment pipeline's behavior. Per codex's pick (option B over routing through enhance_file_metadata), extended `write_tags_to_file` with an optional `artists_list` parameter. Each format-specific writer respects the config flag the same way enrichment.py does: - ID3: TPE1 stays as joined display string + TXXX:Artists multi-value - Vorbis/Opus/FLAC: `artist` display string + `artists` multi-value key - MP4: \xa9ART as list when on, single string when off Scanner retag derives the per-artist list by splitting AcoustID's credit through the existing `split_artist_credit` helper (same separators the matching layer already uses). Backward compatible: callers that don't pass `artists_list` get the exact same single-string write as before. No regression for the write_artist_image button or any other tag_writer caller. 15 tests on the candidate helper + duration guard. 13 tests on the tag_writer multi-value path (write/skip/single/ no-list cases for FLAC + the config-gate helper). 4 new scanner regression tests pinning lower-ranked candidate suppression, no-suppression when no candidate matches, duration mismatch skip, no-skip when duration matches. Existing scanner tests updated for the new 11-column SQL select (added duration column to fake schema + test row tuples). Full suite: 3097 passed. Ruff clean.pull/598/head
Broque Thomas
1 month ago
parent
1fc4c4313b
commit
9cc09118bf
@ -0,0 +1,143 @@
|
||||
"""Find a matching AcoustID candidate for an expected (title, artist).
|
||||
|
||||
AcoustID returns multiple recordings per fingerprint — same audio can
|
||||
correspond to multiple MusicBrainz recordings (different releases,
|
||||
different metadata-quality entries, sample / cover-version collisions).
|
||||
The "top" recording AcoustID returns isn't always the one whose
|
||||
metadata matches the user's expected track.
|
||||
|
||||
Both the post-download verifier (`core/acoustid_verification.py`) and
|
||||
the AcoustID library scanner (`core/repair_jobs/acoustid_scanner.py`)
|
||||
need to ask: "given these candidates, does ANY of them match
|
||||
(expected_title, expected_artist) by title+artist similarity?" The
|
||||
verifier had its own inline loop; the scanner only checked the top
|
||||
match → false positives whenever the wrong-credited recording out-
|
||||
ranked the right-credited one.
|
||||
|
||||
This module is the single shared boundary for that question.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any, Callable, Dict, Iterable, Optional, Tuple
|
||||
|
||||
from utils.logging_config import get_logger
|
||||
|
||||
logger = get_logger("matching.acoustid_candidates")
|
||||
|
||||
|
||||
def find_matching_recording(
|
||||
recordings: Iterable[Dict[str, Any]],
|
||||
expected_title: str,
|
||||
expected_artist: str,
|
||||
*,
|
||||
title_threshold: float = 0.70,
|
||||
artist_threshold: float = 0.60,
|
||||
similarity: Optional[Callable[[str, str], float]] = None,
|
||||
artist_similarity: Optional[Callable[[str, str], float]] = None,
|
||||
skip_predicate: Optional[Callable[[Dict[str, Any]], bool]] = None,
|
||||
) -> Tuple[Optional[Dict[str, Any]], float, float]:
|
||||
"""Return the first AcoustID candidate whose metadata passes both
|
||||
title + artist similarity thresholds.
|
||||
|
||||
Args:
|
||||
recordings: AcoustID recording dicts. Each must carry ``title``
|
||||
and ``artist`` strings; entries without both are skipped.
|
||||
expected_title: The track title the caller expected.
|
||||
expected_artist: The artist the caller expected.
|
||||
title_threshold: Minimum title similarity to accept (default 0.70).
|
||||
artist_threshold: Minimum artist similarity to accept (default 0.60).
|
||||
similarity: ``(a, b) -> float`` for title comparison. Defaults
|
||||
to a lowercase exact-equals stub when not supplied — callers
|
||||
should pass their stricter normaliser (verifier passes its
|
||||
parenthetical-stripping ``_similarity``; scanner passes
|
||||
its own).
|
||||
artist_similarity: ``(expected, actual) -> float`` for artist
|
||||
comparison. Lets callers supply alias-aware comparison
|
||||
(verifier wraps ``_alias_aware_artist_sim``; scanner wraps
|
||||
``artist_names_match``). Defaults to ``similarity`` if
|
||||
unset.
|
||||
skip_predicate: Optional ``(recording_dict) -> bool``. When
|
||||
truthy, the candidate is skipped (used by the verifier to
|
||||
drop wrong-version recordings — instrumental vs vocal etc).
|
||||
|
||||
Returns:
|
||||
``(recording, title_sim, artist_sim)`` for the first matching
|
||||
candidate, or ``(None, best_title_sim, best_artist_sim)`` when
|
||||
none match. The non-None ``best_*`` values let callers report
|
||||
the closest near-miss when they need to log why nothing matched.
|
||||
|
||||
Iteration order matches the input order (typically AcoustID's own
|
||||
fingerprint-confidence ranking). Returns on first match — does NOT
|
||||
score every candidate looking for the highest sim.
|
||||
"""
|
||||
if not expected_title or not expected_artist:
|
||||
return None, 0.0, 0.0
|
||||
|
||||
sim = similarity or _default_similarity
|
||||
asim = artist_similarity or sim
|
||||
|
||||
best_title_sim = 0.0
|
||||
best_artist_sim = 0.0
|
||||
|
||||
for rec in recordings or ():
|
||||
if not isinstance(rec, dict):
|
||||
continue
|
||||
rec_title = (rec.get('title') or '').strip()
|
||||
rec_artist = (rec.get('artist') or '').strip()
|
||||
if not rec_title or not rec_artist:
|
||||
continue
|
||||
if skip_predicate and skip_predicate(rec):
|
||||
continue
|
||||
|
||||
title_sim = sim(expected_title, rec_title)
|
||||
if title_sim > best_title_sim:
|
||||
best_title_sim = title_sim
|
||||
|
||||
artist_sim = asim(expected_artist, rec_artist)
|
||||
if artist_sim > best_artist_sim:
|
||||
best_artist_sim = artist_sim
|
||||
|
||||
if title_sim >= title_threshold and artist_sim >= artist_threshold:
|
||||
return rec, title_sim, artist_sim
|
||||
|
||||
return None, best_title_sim, best_artist_sim
|
||||
|
||||
|
||||
def _default_similarity(a: str, b: str) -> float:
|
||||
if not a or not b:
|
||||
return 0.0
|
||||
return 1.0 if a.lower().strip() == b.lower().strip() else 0.0
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────
|
||||
# Duration guard — codex item (5).
|
||||
# ────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def duration_mismatches_strongly(
|
||||
expected_seconds: Optional[float],
|
||||
candidate_seconds: Optional[float],
|
||||
*,
|
||||
abs_tolerance_s: float = 60.0,
|
||||
rel_tolerance: float = 0.35,
|
||||
) -> bool:
|
||||
"""Return True when the candidate's duration is too far from expected
|
||||
to confidently treat it as the same recording.
|
||||
|
||||
Catches fingerprint hash collisions (the reporter's 17-minute
|
||||
mashup → 5-minute Japanese hiphop track case). When EITHER duration
|
||||
is unknown / non-positive, returns False — no behavior change.
|
||||
|
||||
Threshold: drift greater than max(``abs_tolerance_s``,
|
||||
``rel_tolerance * expected``). The relative term scales with track
|
||||
length so a 20% mismatch on a 3-minute track and a 20% mismatch on
|
||||
a 30-minute mix are both treated as suspicious.
|
||||
"""
|
||||
if not expected_seconds or expected_seconds <= 0:
|
||||
return False
|
||||
if not candidate_seconds or candidate_seconds <= 0:
|
||||
return False
|
||||
drift = abs(float(candidate_seconds) - float(expected_seconds))
|
||||
threshold = max(abs_tolerance_s, rel_tolerance * float(expected_seconds))
|
||||
return drift > threshold
|
||||
@ -0,0 +1,201 @@
|
||||
"""Tests for the shared AcoustID candidate-matching helper.
|
||||
|
||||
Issue #587 / Foxxify report — scanner used to treat ``recordings[0]``
|
||||
as authoritative, so when AcoustID returned multiple candidates and
|
||||
the top one was the wrong-credited recording (different MB entry
|
||||
under the same fingerprint), the scanner created a false-positive
|
||||
"Wrong download" finding even though a lower-ranked candidate matched
|
||||
the expected metadata exactly.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from difflib import SequenceMatcher
|
||||
|
||||
import pytest
|
||||
|
||||
from core.matching.acoustid_candidates import (
|
||||
duration_mismatches_strongly,
|
||||
find_matching_recording,
|
||||
)
|
||||
|
||||
|
||||
def _ratio_sim(a: str, b: str) -> float:
|
||||
"""Reasonable test similarity that handles non-trivial differences."""
|
||||
if not a or not b:
|
||||
return 0.0
|
||||
return SequenceMatcher(None, a.lower().strip(), b.lower().strip()).ratio()
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
# find_matching_recording
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
def test_top_recording_matches_returns_immediately():
|
||||
recordings = [
|
||||
{'title': 'Nana', 'artist': 'Geoxor'},
|
||||
{'title': 'Nana', 'artist': 'Edward Vesala Trio'},
|
||||
]
|
||||
result, t_sim, a_sim = find_matching_recording(
|
||||
recordings, 'Nana', 'Geoxor', similarity=_ratio_sim,
|
||||
)
|
||||
assert result == {'title': 'Nana', 'artist': 'Geoxor'}
|
||||
assert t_sim == 1.0
|
||||
assert a_sim == 1.0
|
||||
|
||||
|
||||
def test_falls_through_to_lower_ranked_match_for_foxxify_nana_case():
|
||||
"""Reporter case 2: top AcoustID candidate is 'Nana' by 'Edward
|
||||
Vesala Trio' (97% fingerprint), but the LOWER-ranked candidate
|
||||
is the expected 'Nana' by 'Geoxor'. Pre-fix scanner saw only [0]
|
||||
and flagged. Post-fix returns the matching candidate."""
|
||||
recordings = [
|
||||
{'title': 'Nana', 'artist': 'Edward Vesala Trio'}, # AcoustID's top match
|
||||
{'title': 'Nana', 'artist': 'Geoxor'}, # the actual right one
|
||||
]
|
||||
result, _, _ = find_matching_recording(
|
||||
recordings, 'Nana', 'Geoxor', similarity=_ratio_sim,
|
||||
)
|
||||
assert result == {'title': 'Nana', 'artist': 'Geoxor'}
|
||||
|
||||
|
||||
def test_no_match_returns_none_with_best_seen_sims():
|
||||
"""When no candidate passes thresholds, return the best-seen sims
|
||||
so callers can log the closest near-miss in the finding."""
|
||||
recordings = [
|
||||
{'title': 'Different Song', 'artist': 'Different Artist'},
|
||||
{'title': 'Sort Of Close', 'artist': 'Different Artist'},
|
||||
]
|
||||
result, t_sim, a_sim = find_matching_recording(
|
||||
recordings, 'Different', 'AnotherArtist',
|
||||
similarity=_ratio_sim,
|
||||
title_threshold=0.95,
|
||||
artist_threshold=0.95,
|
||||
)
|
||||
assert result is None
|
||||
# Best seen — even though no candidate passed the threshold
|
||||
assert t_sim > 0.0
|
||||
assert a_sim > 0.0
|
||||
|
||||
|
||||
def test_skips_recordings_missing_title_or_artist():
|
||||
recordings = [
|
||||
{'title': None, 'artist': 'Geoxor'},
|
||||
{'title': 'Nana', 'artist': ''},
|
||||
{'title': 'Nana', 'artist': 'Geoxor'},
|
||||
]
|
||||
result, _, _ = find_matching_recording(
|
||||
recordings, 'Nana', 'Geoxor', similarity=_ratio_sim,
|
||||
)
|
||||
assert result == {'title': 'Nana', 'artist': 'Geoxor'}
|
||||
|
||||
|
||||
def test_skips_non_dict_entries():
|
||||
recordings = [None, 'string', {'title': 'Nana', 'artist': 'Geoxor'}]
|
||||
result, _, _ = find_matching_recording(
|
||||
recordings, 'Nana', 'Geoxor', similarity=_ratio_sim,
|
||||
)
|
||||
assert result == {'title': 'Nana', 'artist': 'Geoxor'}
|
||||
|
||||
|
||||
def test_empty_inputs_return_none():
|
||||
assert find_matching_recording([], 'X', 'Y')[0] is None
|
||||
assert find_matching_recording([{'title': 'X', 'artist': 'Y'}], '', 'Y')[0] is None
|
||||
assert find_matching_recording([{'title': 'X', 'artist': 'Y'}], 'X', '')[0] is None
|
||||
|
||||
|
||||
def test_separate_artist_similarity_function_is_honored():
|
||||
"""Verifier passes alias-aware comparison via artist_similarity.
|
||||
Make sure it's used instead of the generic similarity."""
|
||||
recordings = [{'title': 'Track', 'artist': '澤野弘之'}]
|
||||
|
||||
def alias_aware(expected, actual):
|
||||
# Pretend our alias chain bridges Hiroyuki Sawano ↔ 澤野弘之
|
||||
if expected == 'Hiroyuki Sawano' and actual == '澤野弘之':
|
||||
return 1.0
|
||||
return 0.0
|
||||
|
||||
result, _, a_sim = find_matching_recording(
|
||||
recordings, 'Track', 'Hiroyuki Sawano',
|
||||
similarity=_ratio_sim,
|
||||
artist_similarity=alias_aware,
|
||||
)
|
||||
assert result is not None
|
||||
assert a_sim == 1.0
|
||||
|
||||
|
||||
def test_skip_predicate_drops_unwanted_candidates():
|
||||
"""Verifier uses skip_predicate to drop wrong-version recordings
|
||||
(instrumental vs vocal, etc.)."""
|
||||
recordings = [
|
||||
{'title': 'Track (Instrumental)', 'artist': 'X'},
|
||||
{'title': 'Track', 'artist': 'X'},
|
||||
]
|
||||
result, _, _ = find_matching_recording(
|
||||
recordings, 'Track', 'X',
|
||||
similarity=_ratio_sim,
|
||||
skip_predicate=lambda r: 'instrumental' in (r.get('title') or '').lower(),
|
||||
)
|
||||
assert result == {'title': 'Track', 'artist': 'X'}
|
||||
|
||||
|
||||
def test_title_threshold_can_be_lowered_for_loose_matching():
|
||||
recordings = [{'title': 'Sort Of Close', 'artist': 'Right Artist'}]
|
||||
# With strict default threshold this fails
|
||||
result_strict, _, _ = find_matching_recording(
|
||||
recordings, 'Different', 'Right Artist', similarity=_ratio_sim,
|
||||
)
|
||||
assert result_strict is None
|
||||
# With a permissive threshold the artist match alone wouldn't help —
|
||||
# title sim must also pass.
|
||||
result_loose, _, _ = find_matching_recording(
|
||||
recordings, 'Different', 'Right Artist',
|
||||
similarity=_ratio_sim, title_threshold=0.0,
|
||||
)
|
||||
assert result_loose is not None
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
# duration_mismatches_strongly — guard against fingerprint collisions
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
def test_durations_within_tolerance_pass():
|
||||
# 3-minute track, 1-second drift — well within tolerance
|
||||
assert duration_mismatches_strongly(180, 181) is False
|
||||
# 3-minute vs 4-minute — within the 60s absolute tolerance
|
||||
assert duration_mismatches_strongly(180, 240) is False
|
||||
|
||||
|
||||
def test_drift_above_absolute_floor_flags():
|
||||
# 3-minute expected, 5-minute candidate (120s drift > 63s threshold)
|
||||
assert duration_mismatches_strongly(180, 300) is True
|
||||
|
||||
|
||||
def test_relative_tolerance_scales_with_long_tracks():
|
||||
# 30-minute expected vs 12-minute candidate (1080s vs 720s) —
|
||||
# 18-minute drift > 35% of 30min = 10.5min → mismatch
|
||||
assert duration_mismatches_strongly(1800, 720) is True
|
||||
# 30-minute expected vs 28-minute candidate — 2min drift = under
|
||||
# max(60s, 35%*30min) = max(60, 630) = 630s → still safe
|
||||
assert duration_mismatches_strongly(1800, 1680) is False
|
||||
|
||||
|
||||
def test_reporter_17min_mashup_vs_5min_track_flagged():
|
||||
"""Foxxify's 17min mashup edit vs 5min late-70s Japanese hiphop —
|
||||
fingerprint collision. Duration guard should mark this suspicious."""
|
||||
assert duration_mismatches_strongly(17 * 60, 5 * 60) is True
|
||||
|
||||
|
||||
def test_unknown_duration_returns_false_no_behavior_change():
|
||||
"""When either side is missing duration, don't change behavior."""
|
||||
assert duration_mismatches_strongly(None, 300) is False
|
||||
assert duration_mismatches_strongly(180, None) is False
|
||||
assert duration_mismatches_strongly(0, 300) is False
|
||||
assert duration_mismatches_strongly(180, 0) is False
|
||||
assert duration_mismatches_strongly(-5, 300) is False
|
||||
|
||||
|
||||
def test_string_or_int_durations_handled():
|
||||
# Defensive — coerce numeric types
|
||||
assert duration_mismatches_strongly(180.5, 181.0) is False
|
||||
assert duration_mismatches_strongly(int(180), int(300)) is True
|
||||
@ -0,0 +1,175 @@
|
||||
"""Tests for the multi-value artist write path in tag_writer.
|
||||
|
||||
Issue #587 — the AcoustID scanner's "Apply Match" retag was bypassing
|
||||
the user's `metadata_enhancement.tags.write_multi_artist` setting and
|
||||
writing single-string TPE1 only. The repair-path retag now passes an
|
||||
``artists_list`` to ``write_tags_to_file`` and the writer respects the
|
||||
config flag the same way the post-download enrichment pipeline does.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from mutagen.flac import FLAC, StreamInfo
|
||||
|
||||
from core.tag_writer import (
|
||||
_multi_artist_write_enabled,
|
||||
_resolve_artists_list_for_write,
|
||||
write_tags_to_file,
|
||||
)
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
# _resolve_artists_list_for_write — derives the multi-value list
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
def test_resolves_artists_list_field():
|
||||
assert _resolve_artists_list_for_write({'artists_list': ['A', 'B']}) == ['A', 'B']
|
||||
|
||||
|
||||
def test_resolves_artists_field_alias():
|
||||
assert _resolve_artists_list_for_write({'artists': ['A', 'B']}) == ['A', 'B']
|
||||
|
||||
|
||||
def test_resolves_underscore_artists_list_field():
|
||||
# _artists_list — the post-process pipeline's internal field name
|
||||
assert _resolve_artists_list_for_write({'_artists_list': ['A', 'B']}) == ['A', 'B']
|
||||
|
||||
|
||||
def test_returns_none_when_no_list_supplied():
|
||||
assert _resolve_artists_list_for_write({'artist_name': 'Solo'}) is None
|
||||
|
||||
|
||||
def test_returns_none_for_non_list_input():
|
||||
assert _resolve_artists_list_for_write({'artists_list': 'A, B'}) is None
|
||||
assert _resolve_artists_list_for_write({'artists_list': {'A': 1}}) is None
|
||||
|
||||
|
||||
def test_strips_empty_and_non_string_entries():
|
||||
out = _resolve_artists_list_for_write({'artists_list': ['A', '', None, ' ', 'B']})
|
||||
assert out == ['A', 'B']
|
||||
|
||||
|
||||
def test_returns_none_when_list_only_has_empty_entries():
|
||||
assert _resolve_artists_list_for_write({'artists_list': ['', None, ' ']}) is None
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
# _multi_artist_write_enabled — config gate
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
def test_multi_artist_write_reads_config():
|
||||
with patch('config.settings.config_manager.get', return_value=True):
|
||||
assert _multi_artist_write_enabled() is True
|
||||
with patch('config.settings.config_manager.get', return_value=False):
|
||||
assert _multi_artist_write_enabled() is False
|
||||
|
||||
|
||||
def test_multi_artist_write_swallows_config_error():
|
||||
with patch('config.settings.config_manager.get', side_effect=RuntimeError('boom')):
|
||||
assert _multi_artist_write_enabled() is False
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
# Vorbis end-to-end — write multi-value to a real FLAC file
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
def _make_minimal_flac(path):
|
||||
"""Create a tiny but valid FLAC with mutagen's lowest-overhead
|
||||
stream info so we can write tags and read them back."""
|
||||
# Empty audio body — just enough to satisfy mutagen's parser.
|
||||
# 44-byte FLAC header + STREAMINFO block.
|
||||
minimal = (
|
||||
b'fLaC'
|
||||
+ b'\x80\x00\x00\x22' # last STREAMINFO block, length 34
|
||||
+ b'\x00\x10\x00\x10' # min/max block size
|
||||
+ b'\x00\x00\x00\x00\x00\x00' # min/max frame size
|
||||
+ b'\x0a\xc4\x42\xf0\x00\x00\x00\x00' # sample rate / channels / etc
|
||||
+ b'\x00' * 16 # MD5
|
||||
)
|
||||
with open(path, 'wb') as f:
|
||||
f.write(minimal)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def flac_path():
|
||||
fd, path = tempfile.mkstemp(suffix='.flac')
|
||||
os.close(fd)
|
||||
_make_minimal_flac(path)
|
||||
yield path
|
||||
try:
|
||||
os.remove(path)
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
|
||||
def test_multi_value_artists_key_written_when_setting_on(flac_path):
|
||||
with patch('config.settings.config_manager.get', return_value=True):
|
||||
result = write_tags_to_file(flac_path, {
|
||||
'title': 'Track',
|
||||
'artist_name': 'Artist A, Artist B',
|
||||
'artists_list': ['Artist A', 'Artist B'],
|
||||
}, embed_cover=False)
|
||||
|
||||
assert result['success'] is True
|
||||
assert 'artists_multi' in result['written_fields']
|
||||
|
||||
audio = FLAC(flac_path)
|
||||
# Display string preserved as `artist`
|
||||
assert audio.get('artist') == ['Artist A, Artist B']
|
||||
# Multi-value list written to `artists` key (Picard convention)
|
||||
assert audio.get('artists') == ['Artist A', 'Artist B']
|
||||
|
||||
|
||||
def test_multi_value_artists_key_skipped_when_setting_off(flac_path):
|
||||
with patch('config.settings.config_manager.get', return_value=False):
|
||||
result = write_tags_to_file(flac_path, {
|
||||
'title': 'Track',
|
||||
'artist_name': 'Artist A, Artist B',
|
||||
'artists_list': ['Artist A', 'Artist B'],
|
||||
}, embed_cover=False)
|
||||
|
||||
assert result['success'] is True
|
||||
assert 'artists_multi' not in result['written_fields']
|
||||
|
||||
audio = FLAC(flac_path)
|
||||
assert audio.get('artist') == ['Artist A, Artist B']
|
||||
# No multi-value key when setting is off — backward compat
|
||||
assert audio.get('artists') is None
|
||||
|
||||
|
||||
def test_single_artist_does_not_write_multi_value_key(flac_path):
|
||||
"""When list has only one entry (or no list at all), don't
|
||||
write the multi-value key even if the setting is on. The point
|
||||
is to differentiate true multi-artist from single-artist tracks."""
|
||||
with patch('config.settings.config_manager.get', return_value=True):
|
||||
result = write_tags_to_file(flac_path, {
|
||||
'title': 'Track',
|
||||
'artist_name': 'Solo Artist',
|
||||
'artists_list': ['Solo Artist'],
|
||||
}, embed_cover=False)
|
||||
|
||||
assert result['success'] is True
|
||||
assert 'artists_multi' not in result['written_fields']
|
||||
audio = FLAC(flac_path)
|
||||
assert audio.get('artists') is None
|
||||
|
||||
|
||||
def test_no_artists_list_legacy_callers_unchanged(flac_path):
|
||||
"""Backward compat — callers that don't supply artists_list get
|
||||
the same single-string write as before. No regression for the
|
||||
write_artist_image button or any other tag_writer caller."""
|
||||
with patch('config.settings.config_manager.get', return_value=True):
|
||||
result = write_tags_to_file(flac_path, {
|
||||
'title': 'Track',
|
||||
'artist_name': 'Solo Artist',
|
||||
}, embed_cover=False)
|
||||
|
||||
assert result['success'] is True
|
||||
assert 'artists_multi' not in result['written_fields']
|
||||
audio = FLAC(flac_path)
|
||||
assert audio.get('artists') is None
|
||||
Loading…
Reference in new issue