Discogs: count rows with empty type_ as real tracks too

Reported by kettui on PR #374 review: the inline filter that backed
`set_album_api_track_count` only counted rows where `type_ == 'track'`,
but `discogs_client.get_album_tracks` itself accepts both `'track'`
AND empty `type_` as real songs (line 660: `type_ in ('track', '')`).
Releases where Discogs returns some real tracks with an empty `type_`
field would be undercounted, which would silently disagree with the
repair job's fallback `_get_expected_total` path (which calls into
`get_album_tracks_for_source` and therefore uses the client's count).

Extracted the filter into `count_discogs_real_tracks(tracklist)` —
single source of truth for the rule, testable in isolation, and the
worker call site is now a one-liner that names what it's doing. Also
defensive about the input shape: `type_ == None`, missing field, and
empty/None tracklist all handled cleanly.

10 tests pin the behavior:
- empty/missing/None type_ all count as a real track (the kettui case)
- 'heading', 'index', 'sub_track' excluded
- unknown future type strings excluded conservatively
- realistic multi-disc tracklist with mixed shapes counts correctly
- empty/None input returns 0 without raising

Credit: kettui — the PR #374 review comment that flagged this.

core/discogs_worker.py

@@ -23,6 +23,29 @@ from core.worker_utils import interruptible_sleep, set_album_api_track_count
 logger = get_logger("discogs_worker")
 
 
+def count_discogs_real_tracks(tracklist) -> int:
+    """Count actual songs in a Discogs tracklist response.
+
+    Discogs tracklists interleave real tracks with section headings
+    (``type_=='heading'``), index markers (``type_=='index'``),
+    and sub-tracks (``type_=='sub_track'``) that aren't themselves
+    songs. We count anything that's explicitly typed as ``'track'`` OR
+    has an empty/missing ``type_`` field — matching exactly what
+    :meth:`core.discogs_client.DiscogsClient.get_album_tracks` itself
+    treats as a real track (`type_ in ('track', '')`). Counting any
+    narrower set silently disagrees with the repair job's fallback
+    `_get_expected_total` path, which calls `get_album_tracks_for_source`
+    under the hood and therefore uses the client's count.
+
+    Reported by kettui on PR #374 — original filter only counted
+    ``type_=='track'`` and undercounted releases where the discogs
+    response left ``type_`` empty for some real tracks.
+    """
+    if not tracklist:
+        return 0
+    return sum(1 for t in tracklist if (t.get('type_') or '') in ('track', ''))
+
+
 class DiscogsWorker:
     """Background worker for enriching library artists and albums with Discogs metadata."""
 
@@ -455,15 +478,13 @@ class DiscogsWorker:
                 """, (image_url, album_id))
 
             # Cache the authoritative expected track count for the Album
-            # Completeness repair job. Discogs tracklists mix actual tracks
+            # Completeness repair job. See `count_discogs_real_tracks`
-            # with section headings and index entries — only type_=='track'
+            # for why we accept both `type_ == 'track'` and empty `type_`
-            # rows are real songs, so filter before counting. (Helper can't
+            # (kettui's PR #374 review — narrower filter undercounted).
-            # do this filter for us; it's Discogs-specific shape.)
-            tracklist = data.get('tracklist') or []
             set_album_api_track_count(
                 cursor,
                 album_id,
-                sum(1 for t in tracklist if t.get('type_') == 'track'),
+                count_discogs_real_tracks(data.get('tracklist')),
             )
 
             conn.commit()

tests/test_discogs_track_count.py

@@ -0,0 +1,129 @@
+"""Tests for `discogs_worker.count_discogs_real_tracks` — the filter
+that decides which entries in a Discogs tracklist count as real songs
+when caching the authoritative track count for the Album Completeness
+repair job.
+
+Reported by kettui on PR #374: the original inline filter only kept
+``type_ == 'track'`` rows, but `discogs_client.get_album_tracks` itself
+keeps both ``type_ == 'track'`` AND rows with an empty/missing
+``type_``. The narrower filter would undercount releases whose Discogs
+response left ``type_`` blank for some real tracks — and the repair
+job's fallback path (`_get_expected_total`) would silently disagree
+with the cached count.
+"""
+
+from core.discogs_worker import count_discogs_real_tracks
+
+
+# ---------------------------------------------------------------------------
+# The kettui case: empty type_ counts as a real track
+# ---------------------------------------------------------------------------
+
+def test_empty_type_counts_as_track():
+    tracklist = [
+        {'title': 'Track 1', 'type_': 'track'},
+        {'title': 'Track 2', 'type_': ''},     # <-- the bug
+        {'title': 'Track 3', 'type_': 'track'},
+    ]
+    assert count_discogs_real_tracks(tracklist) == 3
+
+
+def test_missing_type_field_counts_as_track():
+    """Discogs sometimes omits the field entirely rather than sending
+    an empty string. Both shapes mean 'real track'."""
+    tracklist = [
+        {'title': 'Track 1'},                  # no type_ key at all
+        {'title': 'Track 2', 'type_': 'track'},
+    ]
+    assert count_discogs_real_tracks(tracklist) == 2
+
+
+def test_none_type_counts_as_track():
+    """And the field may be present-but-None on some clients/versions."""
+    tracklist = [
+        {'title': 'Track 1', 'type_': None},
+        {'title': 'Track 2', 'type_': 'track'},
+    ]
+    assert count_discogs_real_tracks(tracklist) == 2
+
+
+# ---------------------------------------------------------------------------
+# Non-track rows are excluded (Discogs's structural markers)
+# ---------------------------------------------------------------------------
+
+def test_headings_excluded():
+    tracklist = [
+        {'title': 'Disc 1', 'type_': 'heading'},
+        {'title': 'Track 1', 'type_': 'track'},
+        {'title': 'Track 2', 'type_': 'track'},
+        {'title': 'Disc 2', 'type_': 'heading'},
+        {'title': 'Track 3', 'type_': 'track'},
+    ]
+    assert count_discogs_real_tracks(tracklist) == 3
+
+
+def test_indices_excluded():
+    tracklist = [
+        {'title': 'Side A', 'type_': 'index'},
+        {'title': 'Track 1', 'type_': 'track'},
+        {'title': 'Side B', 'type_': 'index'},
+        {'title': 'Track 2', 'type_': 'track'},
+    ]
+    assert count_discogs_real_tracks(tracklist) == 2
+
+
+def test_sub_tracks_excluded():
+    """Sub-tracks (parts of a medley) shouldn't double-count against
+    the parent track."""
+    tracklist = [
+        {'title': 'Medley', 'type_': 'track'},
+        {'title': 'Part A', 'type_': 'sub_track'},
+        {'title': 'Part B', 'type_': 'sub_track'},
+        {'title': 'Encore', 'type_': 'track'},
+    ]
+    assert count_discogs_real_tracks(tracklist) == 2
+
+
+def test_unknown_type_excluded():
+    """Conservative — if Discogs adds a new structural marker we
+    haven't seen, don't count it as a track until we explicitly add
+    it to the allowlist."""
+    tracklist = [
+        {'title': 'Track 1', 'type_': 'track'},
+        {'title': 'Some Future Marker', 'type_': 'experimental_thing'},
+    ]
+    assert count_discogs_real_tracks(tracklist) == 1
+
+
+# ---------------------------------------------------------------------------
+# Defensive — bad input shouldn't raise
+# ---------------------------------------------------------------------------
+
+def test_empty_tracklist_returns_zero():
+    assert count_discogs_real_tracks([]) == 0
+
+
+def test_none_tracklist_returns_zero():
+    assert count_discogs_real_tracks(None) == 0
+
+
+# ---------------------------------------------------------------------------
+# Realistic mixed tracklist (the kettui case in context)
+# ---------------------------------------------------------------------------
+
+def test_realistic_multi_disc_tracklist():
+    """A 2-disc release with headings, real tracks, AND a few rows
+    with empty type_ — should count all real tracks but no markers."""
+    tracklist = [
+        {'title': 'Disc 1', 'type_': 'heading'},
+        {'title': 'Track 1', 'type_': 'track'},
+        {'title': 'Track 2', 'type_': 'track'},
+        {'title': 'Track 3', 'type_': ''},       # the kettui case
+        {'title': 'Track 4', 'type_': 'track'},
+        {'title': 'Disc 2', 'type_': 'heading'},
+        {'title': 'Track 5', 'type_': 'track'},
+        {'title': 'Track 6'},                    # missing type_
+        {'title': 'Bonus Index', 'type_': 'index'},
+        {'title': 'Track 7', 'type_': 'track'},
+    ]
+    assert count_discogs_real_tracks(tracklist) == 7