downloads: stop AcoustID scan duplicating history rows / leaving verified tracks 'unverified' (#934)

the AcoustID scanner matched library_history rows by EXACT file_path, but that path is
frozen at import time while the file moves afterward (media-server import / reorganize) —
so tracks.file_path (what the scan reads) no longer equals it. two failures resulted, both
introduced in 37ea6604: verified status never reached the history row (verified tracks kept
showing 'unverified'), and a fresh acoustid_scan row was INSERTed every run (5551 rows for
3675 songs).

- new pure, tested matcher (core/downloads/history_match.py): exact path → filename guarded
  by title; prefers a real download row over a synthetic scan row.
- _persist_status now HEALS the matched row's path + status (so future scans match cleanly),
  DELETES synthetic acoustid_scan duplicates by exact path (collision-free, never a real row),
  and inserts only when the file genuinely has no row.
- a full AcoustID job now self-cleans existing duplicates — no destructive bulk migration.

8 matcher + 4 real-DB heal/dedup/insert tests; existing scanner tests updated to the new
seam (heal vs insert). 1076 acoustid/verification/download tests green.

core/downloads/history_match.py

@@ -0,0 +1,62 @@
+"""Match a file back to its download-history row when its path has drifted (#934).
+
+``library_history.file_path`` is frozen at import time, but the file moves afterward
+(media-server import, library reorganize) and ``tracks.file_path`` — what the AcoustID
+scanner reads — no longer equals it. Matching on the exact path alone then fails twice:
+the verification status never reaches the history row (verified tracks read "unverified"),
+and a fresh ``acoustid_scan`` row gets inserted every run (thousands of duplicates).
+
+This module picks the canonical history row by exact path first, then by FILENAME guarded
+by a title check — so a shared filename ("01 - Intro.flac") can never heal the wrong song.
+Pure (no DB) so the matching rules are unit-testable; the caller does the SQL.
+"""
+
+from __future__ import annotations
+
+import os
+from typing import Iterable, Optional, Sequence, Tuple
+
+
+def _norm_title(value) -> str:
+    """Alphanumeric-only lowercase form, so "Song (Remaster)" vs "song remaster"
+    style drift between the download tag and the media-server tag still agrees."""
+    return ''.join(ch for ch in str(value or '').lower() if ch.isalnum())
+
+
+def like_filename_filter(basename: str) -> str:
+    """A ``LIKE ... ESCAPE '\\'`` pattern that coarsely matches rows whose path ends
+    in ``basename``. Escapes the LIKE metacharacters (``%`` ``_`` ``\\``) — filenames
+    routinely contain underscores. Callers MUST still confirm with an exact basename
+    compare (``pick_history_row`` does), since ``'%name'`` also matches ``'xname'``."""
+    esc = basename.replace('\\', '\\\\').replace('%', '\\%').replace('_', '\\_')
+    return '%' + esc
+
+
+def pick_history_row(candidates: Sequence[Tuple], *, current_paths: Iterable[str],
+                     basename: str, title: str) -> Optional[int]:
+    """Return the id of the history row to update for this file, or None.
+
+    ``candidates``: ``(id, file_path, title, download_source)`` rows the DB pre-filtered
+    (exact path or filename LIKE). A row matches when its path equals the current path OR
+    its filename matches AND its title agrees — the title guard prevents a shared filename
+    ("01 - Intro.flac") from healing a different song's row. Among matches a REAL download
+    row is preferred over a synthetic ``acoustid_scan`` row, so the scanner heals the
+    genuine record and the caller can delete the synthetic duplicate. None when nothing
+    matches safely (caller then inserts a fresh row — the "file SoulSync never downloaded"
+    intent)."""
+    paths = {p for p in current_paths if p}
+    want = _norm_title(title)
+    matches: list = []  # (id, is_exact, is_real)
+    for cid, cpath, ctitle, csource in candidates:
+        is_real = csource != 'acoustid_scan'
+        if cpath and cpath in paths:
+            matches.append((cid, True, is_real))
+        elif (basename and cpath and os.path.basename(cpath) == basename
+              and (not want or not _norm_title(ctitle) or _norm_title(ctitle) == want)):
+            matches.append((cid, False, is_real))
+    if not matches:
+        return None
+    # Prefer a REAL download row over a synthetic acoustid_scan row; within that, prefer an
+    # exact-path match over a filename match. Stable, so ties keep DB order (first/oldest id).
+    matches.sort(key=lambda m: (m[2], m[1]), reverse=True)
+    return matches[0][0]

core/repair_jobs/acoustid_scanner.py

@@ -389,14 +389,43 @@ class AcoustIDScannerJob(RepairJob):
             cur.execute(
                 "UPDATE tracks SET verification_status = ? WHERE id = ?",
                 (status, track_id))
-            matched = 0
-            for p in {p for p in (fpath, db_path) if p}:
-                cur.execute(
-                    "UPDATE library_history SET verification_status = ? WHERE file_path = ?",
-                    (status, p))
-                matched += max(getattr(cur, 'rowcount', 0) or 0, 0)
-            if status == 'unverified' and matched == 0:
             exp = expected or {}
+            # Find the canonical history row for this file. The stored path is frozen at
+            # import time while the file has since moved (media-server import / reorganize),
+            # so an exact-path match alone misses it — then the status never lands and a
+            # duplicate row gets inserted every scan (#934). Match exact path first, then
+            # filename guarded by title, and HEAL the row's path so future scans match cleanly.
+            from core.downloads.history_match import pick_history_row, like_filename_filter
+            current = fpath or db_path
+            basename = os.path.basename(current) if current else ''
+            clauses, params = [], []
+            for p in {p for p in (fpath, db_path) if p}:
+                clauses.append("file_path = ?")
+                params.append(p)
+            if basename:
+                clauses.append("file_path LIKE ? ESCAPE '\\'")
+                params.append(like_filename_filter(basename))
+            row_id = None
+            if clauses:
+                cur.execute(
+                    "SELECT id, file_path, title, download_source FROM library_history WHERE "
+                    + " OR ".join(clauses),
+                    params)
+                row_id = pick_history_row(
+                    cur.fetchall(),
+                    current_paths=(fpath, db_path),
+                    basename=basename, title=exp.get('title') or '')
+            if row_id is not None:
+                cur.execute(
+                    "UPDATE library_history SET verification_status = ?, file_path = ? WHERE id = ?",
+                    (status, current, row_id))
+                # Drop synthetic scan-created duplicates for this exact file (the #934
+                # leftovers). Exact path → collision-free; never touches a real download row.
+                cur.execute(
+                    "DELETE FROM library_history WHERE id != ? AND download_source = 'acoustid_scan' "
+                    "AND file_path = ?",
+                    (row_id, current))
+            elif status == 'unverified':
                 cur.execute(
                     """INSERT INTO library_history
                        (event_type, title, artist_name, album_name, file_path,

tests/test_acoustid_history_heal.py

@@ -0,0 +1,95 @@
+"""#934: the AcoustID scanner heals the download-history row when the file moved,
+instead of leaving it stuck 'unverified' and inserting duplicate scan rows.
+
+Seeds the exact bug shape (a real download row at the OLD import path + a synthetic
+'acoustid_scan' duplicate at the NEW library path) and drives the real _persist_status,
+asserting it collapses to one correct, verified row.
+"""
+
+from __future__ import annotations
+
+import types
+
+import pytest
+
+from core.repair_jobs.acoustid_scanner import AcoustIDScannerJob
+from database.music_database import MusicDatabase
+
+
+@pytest.fixture()
+def db(tmp_path):
+    return MusicDatabase(str(tmp_path / 'm.db'))
+
+
+def _scanner():
+    # _persist_status uses only `context`, never instance state — bypass __init__.
+    return AcoustIDScannerJob.__new__(AcoustIDScannerJob)
+
+
+def _rows(db):
+    with db._get_connection() as conn:
+        return [dict(r) for r in conn.execute(
+            "SELECT file_path, download_source, verification_status FROM library_history")]
+
+
+OLD = '/downloads/transfer/Artist/01 - Song.flac'   # frozen import path in history
+NEW = '/music/Artist/Album/01 - Song.flac'          # where the file lives now (tracks path)
+
+
+def test_verify_heals_drifted_row_and_drops_synthetic_dup(db):
+    # the bug's leftover state: a stuck real row + a synthetic scan dup for the same song.
+    db.add_library_history_entry('download', 'Song', file_path=OLD,
+                                 download_source='soulseek', verification_status='unverified')
+    db.add_library_history_entry('download', 'Song', file_path=NEW,
+                                 download_source='acoustid_scan', verification_status='unverified')
+
+    _scanner()._persist_status(
+        types.SimpleNamespace(db=db), track_id='t1', fpath=NEW, db_path=NEW,
+        status='verified', write_tag=False, expected={'title': 'Song'})
+
+    rows = _rows(db)
+    assert len(rows) == 1                              # synthetic dup deleted, no new insert
+    assert rows[0]['download_source'] == 'soulseek'    # the REAL row survived
+    assert rows[0]['verification_status'] == 'verified'
+    assert rows[0]['file_path'] == NEW                 # path healed to current location
+
+
+def test_idempotent_no_growth_on_rescan(db):
+    db.add_library_history_entry('download', 'Song', file_path=OLD,
+                                 download_source='soulseek', verification_status='unverified')
+    ctx = types.SimpleNamespace(db=db)
+    for _ in range(3):
+        _scanner()._persist_status(ctx, track_id='t1', fpath=NEW, db_path=NEW,
+                                   status='verified', write_tag=False, expected={'title': 'Song'})
+    rows = _rows(db)
+    assert len(rows) == 1 and rows[0]['verification_status'] == 'verified'
+
+
+def test_unknown_file_inserts_one_row_then_dedups(db):
+    # a file SoulSync never downloaded → first scan inserts one review-queue row...
+    ctx = types.SimpleNamespace(db=db)
+    _scanner()._persist_status(ctx, track_id='t1', fpath=NEW, db_path=NEW,
+                               status='unverified', write_tag=False,
+                               expected={'title': 'Song', 'artist': 'Artist'})
+    assert len(_rows(db)) == 1
+    # ...and a rescan matches it (no duplicate).
+    _scanner()._persist_status(ctx, track_id='t1', fpath=NEW, db_path=NEW,
+                               status='unverified', write_tag=False,
+                               expected={'title': 'Song', 'artist': 'Artist'})
+    rows = _rows(db)
+    assert len(rows) == 1 and rows[0]['download_source'] == 'acoustid_scan'
+
+
+def test_does_not_heal_wrong_song_with_same_filename(db):
+    # different song, same filename, different title → must stay untouched (no false heal).
+    db.add_library_history_entry('download', 'A Different Song', file_path='/other/01 - Song.flac',
+                                 download_source='soulseek', verification_status='verified')
+    _scanner()._persist_status(
+        types.SimpleNamespace(db=db), track_id='t1', fpath=NEW, db_path=NEW,
+        status='unverified', write_tag=False, expected={'title': 'Song'})
+    rows = _rows(db)
+    # the unrelated row is untouched, and the unknown file got its own new row.
+    paths = {r['file_path'] for r in rows}
+    assert '/other/01 - Song.flac' in paths and NEW in paths
+    other = next(r for r in rows if r['file_path'] == '/other/01 - Song.flac')
+    assert other['verification_status'] == 'verified'   # not corrupted

tests/test_acoustid_scanner.py

@@ -4,8 +4,9 @@ from core.repair_jobs.acoustid_scanner import AcoustIDScannerJob
 
 
 class _FakeCursor:
-    def __init__(self, rows):
+    def __init__(self, rows, lib_rows=None):
         self._rows = rows
+        self._lib_rows = lib_rows or []
         self.executed = []
 
     def execute(self, query, params=None):
@@ -13,6 +14,10 @@ class _FakeCursor:
         return self
 
     def fetchall(self):
+        # The #934 history-match SELECT gets its own (id, file_path, title, source)
+        # rows; the tracks scan query gets the track rows.
+        if self.executed and 'FROM library_history' in self.executed[-1][0]:
+            return self._lib_rows
         return self._rows
 
     def fetchone(self):
@@ -20,8 +25,8 @@ class _FakeCursor:
 
 
 class _FakeConnection:
-    def __init__(self, rows):
-        self._cursor = _FakeCursor(rows)
+    def __init__(self, rows, lib_rows=None):
+        self._cursor = _FakeCursor(rows, lib_rows)
 
     def cursor(self):
         return self._cursor
@@ -107,12 +112,13 @@ def test_scan_handles_mixed_track_id_types(monkeypatch):
 # and finds the primary artist at 100%, suppressing the false flag.
 
 
-def _make_finding_capturing_context(track_row, captured):
+def _make_finding_capturing_context(track_row, captured, lib_rows=None):
     """Context that captures any create_finding calls into the
     `captured` list. Tests assert against this list to verify whether
     the scanner created a finding (false positive) or correctly
-    skipped (multi-value match resolved)."""
-    conn = _FakeConnection([track_row])
+    skipped (multi-value match resolved). ``lib_rows`` seeds the
+    library_history match SELECT (#934)."""
+    conn = _FakeConnection([track_row], lib_rows)
     config_manager = SimpleNamespace(
         get=lambda key, default=None: default,
         set=lambda *args, **kwargs: None,
@@ -887,9 +893,10 @@ def test_human_verified_files_are_never_scanned(monkeypatch):
 # ---------------------------------------------------------------------------
 
 
-def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_artist):
+def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_artist, lib_rows=None):
     """Drive one _scan_file call and return (status_updates, tag_writes) where
-    status_updates is the list of (query, params) UPDATEs the scanner ran."""
+    status_updates is the list of (query, params) UPDATEs the scanner ran.
+    ``lib_rows`` seeds the library_history match SELECT (#934)."""
     import core.repair_jobs.acoustid_scanner as scanner_mod
     monkeypatch.setattr(scanner_mod, "_resolve_expected_artist_aliases",
                         lambda name: [], raising=False)
@@ -905,7 +912,7 @@ def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_arti
     context = _make_finding_capturing_context(
         track_row=("9", "Call Your Name", expected_artist,
                    "/music/cyn.flac", 1, "Album", None, None),
-        captured=captured)
+        captured=captured, lib_rows=lib_rows)
     fake = SimpleNamespace(fingerprint_and_lookup=lambda f: {
         'best_score': 0.97,
         'recordings': [{'title': 'Call Your Name', 'artist': aid_artist}]})
@@ -920,29 +927,32 @@ def _run_persistence_scan(monkeypatch, *, file_status, aid_artist, expected_arti
 
 
 def test_scan_pass_backfills_verified_status(monkeypatch):
-    # Untagged file + clean fingerprint PASS → the scan backfills 'verified'
-    # into the tag, the tracks row AND library_history (review-queue feed).
+    # Clean fingerprint PASS → the scan backfills 'verified' into the tag, the tracks
+    # row AND the file's library_history row. The history row's path drifted since
+    # download (file moved), so the scan heals it by id (#934) rather than missing it.
     updates, tag_writes, captured = _run_persistence_scan(
         monkeypatch, file_status=None,
-        aid_artist='Sawano Hiroyuki', expected_artist='Sawano Hiroyuki')
+        aid_artist='Sawano Hiroyuki', expected_artist='Sawano Hiroyuki',
+        lib_rows=[(1, '/downloads/old/cyn.flac', 'Call Your Name', 'soulseek')])
     assert captured == []
     assert tag_writes == [('/music/cyn.flac', 'verified')]
     assert any('tracks' in q and p == ('verified', '9') for q, p in updates)
-    assert any('library_history' in q and p == ('verified', '/music/cyn.flac')
+    # healed by id, status set + path refreshed to the file's current location.
+    assert any('library_history' in q and p == ('verified', '/music/cyn.flac', 1)
                for q, p in updates)
 
 
 def test_scan_skip_marks_untagged_file_unverified(monkeypatch):
     # Title matches but the artist is ambiguous (cover/collab band?) → SKIP.
-    # An untagged file gets 'unverified' so it surfaces in the Downloads-page
-    # review queue instead of silently passing or being deleted.
+    # An untagged file SoulSync never downloaded (no history row) gets a fresh
+    # 'unverified' row INSERTed so it surfaces in the Downloads-page review queue.
     updates, tag_writes, captured = _run_persistence_scan(
         monkeypatch, file_status=None,
-        aid_artist='Mantilla', expected_artist='Metallica')
+        aid_artist='Mantilla', expected_artist='Metallica')  # no lib_rows → no existing row
     assert captured == []
     assert tag_writes == [('/music/cyn.flac', 'unverified')]
     assert any('tracks' in q and p == ('unverified', '9') for q, p in updates)
-    assert any('library_history' in q and p == ('unverified', '/music/cyn.flac')
+    assert any('INSERT INTO library_history' in q and p[-1] == 'unverified'
                for q, p in updates)
 
 

tests/test_history_match.py

@@ -0,0 +1,63 @@
+"""Pure matcher that re-links a moved file to its download-history row (#934)."""
+
+from core.downloads.history_match import pick_history_row, like_filename_filter
+
+
+# (id, file_path, title, download_source)
+def _row(i, path, title='', source='soulseek'):
+    return (i, path, title, source)
+
+
+def test_exact_current_path_wins():
+    cands = [_row(1, '/old/song.flac', 'Song'), _row(2, '/lib/song.flac', 'Song')]
+    assert pick_history_row(cands, current_paths=('/lib/song.flac', None),
+                            basename='song.flac', title='Song') == 2
+
+
+def test_falls_back_to_filename_when_path_drifted():
+    # history has the OLD import path; scanner only knows the NEW library path.
+    cands = [_row(7, '/downloads/transfer/Artist/01 - Song.flac', 'Song')]
+    assert pick_history_row(cands, current_paths=('/music/Artist/Album/01 - Song.flac', None),
+                            basename='01 - Song.flac', title='Song') == 7
+
+
+def test_title_guard_blocks_shared_filename_collision():
+    # two different songs both named "01 - Intro.flac" — must NOT heal the wrong one.
+    cands = [_row(1, '/a/01 - Intro.flac', 'Album A Intro'),
+             _row(2, '/b/01 - Intro.flac', 'Album B Intro')]
+    got = pick_history_row(cands, current_paths=('/c/01 - Intro.flac', None),
+                           basename='01 - Intro.flac', title='Album B Intro')
+    assert got == 2
+
+
+def test_title_drift_still_matches_after_normalization():
+    cands = [_row(5, '/old/track.flac', 'Song (Remastered)')]
+    assert pick_history_row(cands, current_paths=('/new/track.flac', None),
+                            basename='track.flac', title='song remastered') == 5
+
+
+def test_prefers_real_download_row_over_synthetic_scan_row():
+    # the #934 collapse: a real download row (drifted path) + a synthetic scan dup at the
+    # exact current path. The REAL row must win, so the synthetic one can be deleted.
+    cands = [_row(10, '/old/song.flac', 'Song', source='soulseek'),
+             _row(11, '/lib/song.flac', 'Song', source='acoustid_scan')]
+    assert pick_history_row(cands, current_paths=('/lib/song.flac', None),
+                            basename='song.flac', title='Song') == 10
+
+
+def test_no_basename_match_returns_none():
+    cands = [_row(1, '/x/other.flac', 'Other')]
+    assert pick_history_row(cands, current_paths=('/x/wanted.flac', None),
+                            basename='wanted.flac', title='Wanted') is None
+
+
+def test_filename_only_substring_is_not_a_match():
+    # '/x/mysong.flac' must NOT satisfy basename 'song.flac'
+    cands = [_row(1, '/x/mysong.flac', 'My Song')]
+    assert pick_history_row(cands, current_paths=('/x/song.flac', None),
+                            basename='song.flac', title='Song') is None
+
+
+def test_like_filter_escapes_metacharacters():
+    # underscores/percents in filenames must not become LIKE wildcards
+    assert like_filename_filter('a_b%c.flac') == r'%a\_b\%c.flac'