Merge pull request #514 from Nezreka/fix/repair-job-card-pending-count

Repair job card badge — show pending count, not last-scan count

core/repair_worker.py

@@ -257,8 +257,22 @@ class RepairWorker:
             self._config_manager.set(f'repair.jobs.{job_id}.settings', current)
 
     def get_all_job_info(self) -> List[dict]:
-        """Get info for all jobs (for API response)."""
+        """Get info for all jobs (for API response).
+
+        Includes ``pending_findings_count`` per job so the job-card
+        badge can show CURRENT pending state instead of the
+        ``last_run.findings_created`` historical scan count. Without
+        this, a scan that creates 372 findings + a subsequent bulk-
+        fix that resolves all of them leaves the badge displaying
+        "372 findings" while the Findings tab Pending filter shows 0
+        — confusing UX flagged on the Library Maintenance page.
+        """
         self._ensure_jobs_loaded()
+
+        # Single query → per-job pending count dict. O(1) lookup per
+        # job instead of N round trips.
+        pending_by_job = self._get_pending_count_by_job()
+
         jobs_info = []
         for job_id, job in self._jobs.items():
             config = self.get_job_config(job_id)
@@ -284,9 +298,30 @@ class RepairWorker:
                 'last_run': last_run,
                 'next_run': next_run,
                 'is_running': self._current_job_id == job_id,
+                'pending_findings_count': pending_by_job.get(job_id, 0),
             })
         return jobs_info
 
+    def _get_pending_count_by_job(self) -> dict:
+        """Return ``{job_id: pending_count}`` for every job that has
+        any pending findings. Single SQL aggregation."""
+        conn = None
+        try:
+            conn = self.db._get_connection()
+            cursor = conn.cursor()
+            cursor.execute("""
+                SELECT job_id, COUNT(*) FROM repair_findings
+                WHERE status = 'pending'
+                GROUP BY job_id
+            """)
+            return {row[0]: row[1] for row in cursor.fetchall()}
+        except Exception as e:
+            logger.debug("Error counting pending findings per job: %s", e)
+            return {}
+        finally:
+            if conn:
+                conn.close()
+
     # ------------------------------------------------------------------
     # Lifecycle
     # ------------------------------------------------------------------

tests/test_create_finding_dedup_counter.py

@@ -206,3 +206,96 @@ def test_caller_pattern_counts_only_real_inserts(repair_worker_with_temp_db):
 
     assert result.findings_created == 1
     assert result.findings_skipped_dedup == 4
+
+
+# ---------------------------------------------------------------------------
+# _get_pending_count_by_job — feeds the job-card "X pending" badge
+# ---------------------------------------------------------------------------
+
+
+def test_get_pending_count_by_job_returns_per_job_dict(repair_worker_with_temp_db):
+    """Pin: helper returns ``{job_id: pending_count}`` based on rows
+    with status='pending' only. Job-card badge uses this so it shows
+    CURRENT pending state instead of the historical
+    ``last_run.findings_created`` (which inflates after a bulk-fix
+    moves all findings to status='resolved')."""
+    worker, path = repair_worker_with_temp_db
+
+    conn = sqlite3.connect(path)
+    cur = conn.cursor()
+    # 3 pending duplicate_tracks for dup_detector
+    cur.executemany("""
+        INSERT INTO repair_findings (job_id, finding_type, severity, status,
+            entity_type, entity_id, file_path, title, description)
+        VALUES (?, 'duplicate_tracks', 'info', 'pending', 'track', ?, ?, 'T', 'D')
+    """, [
+        ('dup_detector', '1', '/a'),
+        ('dup_detector', '2', '/b'),
+        ('dup_detector', '3', '/c'),
+    ])
+    # 2 pending missing_cover_art for cover_art_filler
+    cur.executemany("""
+        INSERT INTO repair_findings (job_id, finding_type, severity, status,
+            entity_type, entity_id, file_path, title, description)
+        VALUES ('cover_art_filler', 'missing_cover_art', 'info', 'pending',
+                'album', ?, ?, 'T', 'D')
+    """, [('10', '/x'), ('11', '/y')])
+    # 1 RESOLVED finding — must NOT be counted
+    cur.execute("""
+        INSERT INTO repair_findings (job_id, finding_type, severity, status,
+            entity_type, entity_id, file_path, title, description)
+        VALUES ('dup_detector', 'duplicate_tracks', 'info', 'resolved',
+                'track', '99', '/z', 'T', 'D')
+    """)
+    # 1 DISMISSED finding — must NOT be counted
+    cur.execute("""
+        INSERT INTO repair_findings (job_id, finding_type, severity, status,
+            entity_type, entity_id, file_path, title, description)
+        VALUES ('cover_art_filler', 'missing_cover_art', 'info', 'dismissed',
+                'album', '88', '/w', 'T', 'D')
+    """)
+    conn.commit()
+    conn.close()
+
+    result = worker._get_pending_count_by_job()
+
+    assert result == {
+        'dup_detector': 3,
+        'cover_art_filler': 2,
+    }
+
+
+def test_get_pending_count_by_job_returns_empty_when_no_pending(repair_worker_with_temp_db):
+    """Pin: jobs with zero pending findings are absent from the dict
+    (caller handles missing key as 0)."""
+    worker, path = repair_worker_with_temp_db
+
+    conn = sqlite3.connect(path)
+    cur = conn.cursor()
+    cur.execute("""
+        INSERT INTO repair_findings (job_id, finding_type, severity, status,
+            entity_type, entity_id, file_path, title, description)
+        VALUES ('dup_detector', 'duplicate_tracks', 'info', 'resolved',
+                'track', '1', '/a', 'T', 'D')
+    """)
+    conn.commit()
+    conn.close()
+
+    result = worker._get_pending_count_by_job()
+
+    assert result == {}
+
+
+def test_get_pending_count_by_job_handles_db_error_gracefully(repair_worker_with_temp_db):
+    """Pin: any DB exception → returns ``{}``. Job-card badge falls
+    back to historical count via the ``or 0`` safety in JS."""
+    worker, _ = repair_worker_with_temp_db
+
+    # Replace _get_connection with one that raises.
+    def _raise():
+        raise sqlite3.OperationalError("simulated")
+    worker.db._get_connection = _raise
+
+    result = worker._get_pending_count_by_job()
+
+    assert result == {}

webui/static/enrichment.js

@@ -1555,11 +1555,25 @@ async function loadRepairJobs() {
                     flowParts.push('<span class="repair-flow-badge autofix">Auto-fix</span>');
                 }
             }
-            // Show pending findings count
-            const findingsCount = job.last_run ? (job.last_run.findings_created || 0) : 0;
-            if (findingsCount > 0) {
+            // Badge: prefer the CURRENT pending count from the API
+            // (matches what the Findings tab actually shows); fall
+            // back to the historical ``findings_created`` from the
+            // last run when pending = 0 but the last scan did find
+            // something — that way users still see a hint that a scan
+            // happened recently. Without this, a scan that finds 372
+            // duplicates and then has them all bulk-fixed shows
+            // "372 findings" on the badge while the Findings tab
+            // Pending filter is empty, which reads as a bug.
+            const pendingCount = job.pending_findings_count || 0;
+            const lastScanCount = job.last_run ? (job.last_run.findings_created || 0) : 0;
+            if (pendingCount > 0) {
                 flowParts.push('<span class="repair-flow-arrow">&rarr;</span>');
-                flowParts.push(`<span class="repair-flow-badge findings">${findingsCount} finding${findingsCount !== 1 ? 's' : ''}</span>`);
+                flowParts.push(`<span class="repair-flow-badge findings">${pendingCount} pending</span>`);
+            } else if (lastScanCount > 0) {
+                // Show historical count with a clear "found in last scan"
+                // qualifier so users don't expect them on the Pending tab.
+                flowParts.push('<span class="repair-flow-arrow">&rarr;</span>');
+                flowParts.push(`<span class="repair-flow-badge findings findings-historical">${lastScanCount} found in last scan</span>`);
             }
 
             // Build meta parts

webui/static/helper.js

@@ -3432,6 +3432,7 @@ const WHATS_NEW = {
     '2.4.2': [
         // --- post-2.4.1 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
         { date: 'Unreleased — 2.4.2 dev cycle' },
+        { title: 'Fix: Repair Job Card "X Findings" Badge Was Misleading After Bulk-Fix', desc: 'discord report: duplicate detector card said "372 findings" and cover art filler said "60 findings", but clicking the findings tab pending filter showed 0 — read like a bug ("findings aren\'t being created"). actual cause: job-card badge displayed `last_run.findings_created` (historical "found in last scan") which doesn\'t reflect current state when those findings have since been bulk-fixed and moved to status="resolved". fix: api response now includes `pending_findings_count` per job (current pending count from a single sql aggregation). badge now shows "X pending" when pending count > 0 (urgent red color), or "X found in last scan" with a muted grey color when pending = 0 but the last scan did surface something. user can tell at a glance whether something needs review vs whether it\'s a historical reminder. 3 new tests pin the per-job pending count helper.', page: 'stats' },
         { title: 'Fix: Downloads Stop After 2-3 Hours (slskd HTTP Timeout)', desc: 'github issue #499 (bafoed): big initial sync of spotify playlists worked for 2-3 hours then downloads silently stopped. 3 active tasks stuck in "searching" state, replaced every ~10 min by different ones, but slskd ui showed no actual searches happening. only fix was restarting the soulsync container — which would buy another 2-3 hours. root cause: `core/soulseek_client.py` constructed `aiohttp.ClientSession()` with no timeout at four sites. when slskd hung on a request (overloaded, transient network blip, internal stall), the http call blocked indefinitely — and the worker thread blocked with it. download executor only has `max_workers=3` for download workers. once 3 worker threads were wedged on hung calls, no further downloads could start. batch-level "stuck detection" (10-min) was correctly marking tasks `not_found` and trying to start replacements, but the executor pool was exhausted — replacements queued forever. fix: bounded `aiohttp.ClientTimeout` (total 120s, connect 15s, sock_read 60s) on every slskd `ClientSession` construction. legitimate metadata calls (search submission, status polls, download enqueue) finish in seconds — slskd doesn\'t stream files through these requests, so the timeout can\'t kill a real operation. when timeout fires, the request raises `asyncio.TimeoutError` which is now explicitly caught + logged + returns None to the caller (treats as a normal failure, same code path as a 5xx response). worker thread unblocks → executor pool stays healthy → downloads keep flowing. 3 new tests pin the timeout config + the `asyncio.TimeoutError` handler so future drift fails at the test boundary instead of at runtime against a wedged executor.', page: 'downloads' },
         { title: 'Fix: Library Reorganize Job Misclassified Album Tracks As Singles', desc: 'github issue #500 (bafoed): library reorganize repair job moved tracks like `Surf Curse/Surf Curse - Nothing Yet (2017)/01 - Christine F.flac` to single-template paths like `Surf Curse/Surf Curse - Christine F/Surf Curse - Christine F.flac`. root cause: the job used `is_album = (group_size > 1)` where `group_size` was the count of tracks for the same album currently sitting in the transfer folder being scanned — when only one track of an album was in transfer (rest already moved to library, or album tags varied across tracks like "Buds" vs "Buds (Bonus)"), each track became a 1-element group → all routed through single template. fix: rewrote the job to delegate to the per-album planner (`core.library_reorganize.preview_album_reorganize` / `reorganize_queue`) — the same planner the artist-detail "reorganize" modal uses. db-driven: the planner knows the album has multiple tracks regardless of how many sit in the transfer folder, so the album-vs-single classification is structurally correct. apply mode delegates to the existing reorganize queue → file move + post-processing + db update + sidecar handling all flow through one code path. only iterates albums for the ACTIVE media server (matches the artist-detail modal\'s scope) — multi-server users (plex + jellyfin etc) won\'t accidentally have the job touch the inactive server\'s files. albums missing a metadata source id get a single "needs enrichment first" finding instead of n per-track "no source" findings. dropped ~500 loc of tag-reading + transfer-walk + template logic that was duplicated against the per-album path. files in transfer with no db entry are now exclusively the orphan_file_detector\'s domain (clean separation). 12 tests pin the delegation contract.', page: 'library' },
         { title: 'Fix: Enrich Honors Manual Album Matches', desc: 'github issue #501 (tacobell444): if you manually matched an album to a specific source ID via the match-chip UI, then clicked "enrich" on that album, the worker would search by name and overwrite your manual match with whatever the search returned (or revert status to "not_found" if it found nothing). reorganize then read the now-wrong id and moved files to the wrong destination. fix: extracted a shared `core/enrichment/manual_match_honoring.py` helper. every per-source enrichment worker (spotify / itunes / deezer / tidal / qobuz) now reads its stored id column at the top of `_process_*_individual` — if present, it fetches via `client.get_album(stored_id)` directly and refreshes metadata without touching the id. fuzzy name search only runs as fallback for never-matched entities. discogs / audiodb / musicbrainz already had inline stored-id fast paths and are left alone. lastfm / genius are name-based and don\'t store ids. cin-shape lift: same fix in 5 workers gets exactly one helper, per-worker variability (column name, client method, response shape) plugs in via callbacks. 11 new helper tests pin: stored-id fast-path, no-id fallthrough, fetch-failure fallthrough, table/column whitelist, callback contract.', page: 'library' },

webui/static/style.css

@@ -50833,6 +50833,13 @@ tr.tag-diff-same {
     background: rgba(239, 68, 68, 0.10);
     color: #f87171;
 }
+/* Historical "found in last scan" — present but already resolved /
+   dismissed. Muted so it doesn't read as urgent action like the
+   current-pending badge. */
+.repair-flow-badge.findings-historical {
+    background: rgba(148, 163, 184, 0.10);
+    color: #94a3b8;
+}
 .repair-flow-arrow {
     color: rgba(255, 255, 255, 0.25);
     font-size: 12px;