#800: Write Tags must not overwrite a real file value with a placeholder

A mis-grouped library track sits under a 'Various Artists' / '[Unknown
Album]' record while the file itself is correctly tagged. Write Tags
reads the DB and stamped that junk over the file — destroying the
correct tags. (Rematching never helped: a match only stores a source-ID
pointer, it never changes the local name/title the writer reads.)

Guard: never replace a real file value with a placeholder. Added at both
seams so preview and write agree:
- build_tag_diff marks such a field protected/no-change (the preview no
  longer shows a wrong overwrite, and has_changes reflects reality),
- write_tags_to_file reads the file's current values and skips the
  placeholder-over-real fields, preserving the file.

Field-agnostic and direction-safe: the guard fires ONLY when the DB
value is a placeholder AND the file holds a real one. A legitimate value
still writes — including a genuine 'Various Artists' album artist on a
real compilation, where the file has no conflicting real value, so the
guard doesn't fire. Every write_tags_to_file caller writes DB->file as a
correction, so blocking placeholder-over-real is correct for all of them
(the download post-process uses a different path, embed_source_ids).

23 new tests (placeholder detection, the guard fn, build_tag_diff on the
screenshot-#2 scenario, end-to-end FLAC write preserving real values and
still overwriting with real ones). 113 existing repair/retag/tag tests
pass.

core/tag_writer.py

@@ -136,10 +136,49 @@ def read_file_tags(file_path: str) -> Dict[str, Any]:
     return result
 
 
+# Known placeholder / "we don't really know" metadata values. A match in the
+# DB never warrants overwriting a real value already in the file (issue #800:
+# a mis-grouped track sits under a "Various Artists" / "[Unknown Album]" record,
+# and Write Tags would otherwise stamp that junk over the file's correct tags).
+_PLACEHOLDER_META_VALUES = frozenset({
+    'various artists', 'various artist',
+    'unknown artist', 'unknown album', 'unknown',
+    '[unknown album]', '[unknown artist]', '[unknown]',
+    'untitled album',
+})
+
+
+def is_placeholder_meta(value: Any) -> bool:
+    """True for empty or known-placeholder metadata strings (case-insensitive)."""
+    if value is None:
+        return True
+    s = str(value).strip().lower()
+    return s == '' or s in _PLACEHOLDER_META_VALUES
+
+
+def guard_placeholder_overwrite(db_val: Any, file_val: Any) -> Any:
+    """#800 guard: never replace a real file value with a placeholder.
+
+    Returns ``None`` (skip the write → preserve the file's value) ONLY when the
+    DB value is a placeholder/empty AND the file already holds a real,
+    non-placeholder value. Otherwise returns ``db_val`` unchanged — so a
+    legitimate value still writes, including a genuine ``Various Artists`` album
+    artist on a real compilation (there the file has no conflicting real value,
+    so the guard doesn't fire).
+    """
+    if is_placeholder_meta(db_val) and not is_placeholder_meta(file_val):
+        return None
+    return db_val
+
+
 def build_tag_diff(file_tags: Dict[str, Any], db_data: Dict[str, Any]) -> List[Dict[str, Any]]:
     """
     Compare file tags against DB metadata. Returns a list of diffs:
-    [{ field, file_value, db_value, changed }]
+    [{ field, file_value, db_value, changed, protected }]
+
+    ``protected`` marks a field the #800 guard is holding back: the DB value is
+    a placeholder and the file's real value would be preserved, so it's shown
+    as no-change rather than a wrong overwrite.
     """
     fields = [
         ('title', 'title', 'Title'),
@@ -179,12 +218,22 @@ def build_tag_diff(file_tags: Dict[str, Any], db_data: Dict[str, Any]) -> List[D
         # Only mark as changed if DB has a value AND it differs from file
         # (writer skips fields where DB value is empty, so don't show them as diffs)
         changed = bool(db_str) and file_str != db_str
+
+        # #800 — if the change would replace a real file value with a
+        # placeholder (Various Artists / [Unknown Album] / …), hold it back:
+        # the writer preserves the file's value, so show it as no-change.
+        protected = False
+        if changed and guard_placeholder_overwrite(db_val, file_val) is None:
+            changed = False
+            protected = True
+
         diffs.append({
             'field': label,
             'file_key': file_key,
             'file_value': str(file_val) if file_val is not None else '',
             'db_value': str(db_val) if db_val is not None else '',
             'changed': changed,
+            'protected': protected,
         })
 
     # Cover art — special row
@@ -253,6 +302,23 @@ def write_tags_to_file(file_path: str, db_data: Dict[str, Any],
         artist = db_data.get('track_artist') or db_data.get('artist_name')  # Per-track artist for compilations/DJ mixes
         album = db_data.get('album_title')
         album_artist = db_data.get('artist_name')  # Album artist stays as the album-level artist
+
+        # #800 — never overwrite a real value already in the file with a
+        # placeholder DB value (Various Artists / [Unknown Album] / …). A
+        # mis-grouped track sits under such a record; writing it would destroy
+        # the file's correct tags. Reads the file's current values and skips
+        # only the placeholder-over-real fields (legit values, incl. a genuine
+        # compilation's Various Artists, still write — see guard docstring).
+        try:
+            _current = read_file_tags(file_path)
+        except Exception:
+            _current = {}
+        if not _current.get('error'):
+            title = guard_placeholder_overwrite(title, _current.get('title'))
+            artist = guard_placeholder_overwrite(artist, _current.get('artist'))
+            album = guard_placeholder_overwrite(album, _current.get('album'))
+            album_artist = guard_placeholder_overwrite(album_artist, _current.get('album_artist'))
+
         year = db_data.get('year')
         genres = db_data.get('genres')
         track_num = db_data.get('track_number')

tests/test_tag_writer_placeholder_guard.py

@@ -0,0 +1,143 @@
+"""#800 — Write Tags must never overwrite a real file value with a placeholder.
+
+A mis-grouped track sits under a 'Various Artists' / '[Unknown Album]' record,
+while the file itself is correctly tagged. Previously Write Tags stamped that
+junk over the good file (data loss). These pin the guard at both seams: the
+preview (build_tag_diff) and the actual write (write_tags_to_file).
+"""
+
+from __future__ import annotations
+
+import os
+import tempfile
+
+import pytest
+from mutagen.flac import FLAC
+
+from core.tag_writer import (
+    build_tag_diff,
+    guard_placeholder_overwrite,
+    is_placeholder_meta,
+    write_tags_to_file,
+)
+
+
+# ---------------------------------------------------------------------------
+# pure helpers
+# ---------------------------------------------------------------------------
+
+@pytest.mark.parametrize('val', [
+    None, '', '   ', 'Various Artists', 'various artists', 'VARIOUS ARTIST',
+    'Unknown Artist', 'Unknown Album', '[Unknown Album]', 'Unknown', 'Untitled Album',
+])
+def test_placeholder_values_detected(val):
+    assert is_placeholder_meta(val) is True
+
+
+@pytest.mark.parametrize('val', ['OneRepublic', 'Native (Deluxe)', 'Counting Stars', 'VA Movement'])
+def test_real_values_not_placeholder(val):
+    assert is_placeholder_meta(val) is False
+
+
+def test_guard_blocks_placeholder_over_real():
+    assert guard_placeholder_overwrite('Various Artists', 'OneRepublic') is None
+    assert guard_placeholder_overwrite('[Unknown Album]', 'Native (Deluxe)') is None
+
+
+def test_guard_allows_real_over_anything():
+    assert guard_placeholder_overwrite('OneRepublic', 'Old Wrong Name') == 'OneRepublic'
+    assert guard_placeholder_overwrite('OneRepublic', '') == 'OneRepublic'
+
+
+def test_guard_allows_placeholder_when_file_has_no_real_value():
+    # Legit compilation: file album-artist empty → 'Various Artists' still writes.
+    assert guard_placeholder_overwrite('Various Artists', '') == 'Various Artists'
+    assert guard_placeholder_overwrite('Various Artists', 'Various Artists') == 'Various Artists'
+
+
+# ---------------------------------------------------------------------------
+# build_tag_diff — the preview (screenshot #2 scenario)
+# ---------------------------------------------------------------------------
+
+def test_diff_protects_placeholder_over_real():
+    file_tags = {'title': 'Counting Stars', 'artist': 'OneRepublic',
+                 'album': 'Native (Deluxe)', 'album_artist': 'OneRepublic'}
+    db_data = {'title': 'Counting Stars', 'artist_name': 'Various Artists',
+               'album_title': '[Unknown Album]'}
+    diff = {d['field']: d for d in build_tag_diff(file_tags, db_data)}
+
+    assert diff['Title']['changed'] is False          # already equal
+    for f in ('Artist', 'Album', 'Album Artist'):
+        assert diff[f]['changed'] is False, f          # held back, not a wrong overwrite
+        assert diff[f]['protected'] is True, f
+    # Nothing real to write → no changes overall.
+    assert not any(d['changed'] for d in build_tag_diff(file_tags, db_data))
+
+
+def test_diff_real_db_value_still_changes():
+    file_tags = {'artist': 'Old Wrong'}
+    db_data = {'artist_name': 'OneRepublic'}
+    diff = {d['field']: d for d in build_tag_diff(file_tags, db_data)}
+    assert diff['Artist']['changed'] is True
+    assert diff['Artist']['protected'] is False
+
+
+def test_diff_compilation_va_writes_when_file_empty():
+    file_tags = {'album_artist': ''}  # no real value to protect
+    db_data = {'artist_name': 'Various Artists'}
+    diff = {d['field']: d for d in build_tag_diff(file_tags, db_data)}
+    assert diff['Album Artist']['changed'] is True
+    assert diff['Album Artist']['protected'] is False
+
+
+# ---------------------------------------------------------------------------
+# write_tags_to_file — end-to-end on a real FLAC
+# ---------------------------------------------------------------------------
+
+def _make_minimal_flac(path):
+    minimal = (
+        b'fLaC' + b'\x80\x00\x00\x22' + b'\x00\x10\x00\x10'
+        + b'\x00\x00\x00\x00\x00\x00' + b'\x0a\xc4\x42\xf0\x00\x00\x00\x00' + b'\x00' * 16
+    )
+    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_write_preserves_real_value_against_placeholder(flac_path):
+    # 1) lay down correct tags (the file is correctly tagged)
+    write_tags_to_file(flac_path, {
+        'title': 'Counting Stars', 'artist_name': 'OneRepublic',
+        'album_title': 'Native (Deluxe)',
+    }, embed_cover=False)
+
+    # 2) attempt to write the mis-grouped DB junk over it
+    result = write_tags_to_file(flac_path, {
+        'title': 'Counting Stars', 'artist_name': 'Various Artists',
+        'album_title': '[Unknown Album]',
+    }, embed_cover=False)
+    assert result['success'] is True
+
+    audio = FLAC(flac_path)
+    assert audio.get('artist') == ['OneRepublic']          # preserved, not clobbered
+    assert audio.get('album') == ['Native (Deluxe)']       # preserved
+    assert audio.get('albumartist') == ['OneRepublic']     # preserved
+
+
+def test_write_real_value_still_overwrites(flac_path):
+    write_tags_to_file(flac_path, {'artist_name': 'OneRepublic'}, embed_cover=False)
+    # A real (non-placeholder) new value must still overwrite normally.
+    result = write_tags_to_file(flac_path, {'artist_name': 'Coldplay'}, embed_cover=False)
+    assert result['success'] is True
+    assert FLAC(flac_path).get('artist') == ['Coldplay']