Repair job card badge — show pending count, not last-scan count
Discord report: Duplicate Detector card said "372 findings" and Cover
Art Filler said "60 findings", but clicking the Findings tab's Pending
filter showed 0. User read it as "findings aren't being created" —
looked like a detector bug.
Actual cause: the badge sourced ``last_run.findings_created``
(historical "found in last scan") without considering current state.
After the user (or bulk-fix automation) resolved or dismissed those
findings, they no longer appeared on the Pending tab — but the badge
kept showing the last-scan number in red urgent styling.
Backend was correct end-to-end: detectors create pending rows,
bulk-fix moves them to resolved, Findings tab filters by status.
Only the badge display lied about current state.
Fix:
- ``RepairWorker._get_pending_count_by_job()`` — single SQL aggregation
returning ``{job_id: pending_count}`` for every job with pending
findings. O(1) lookup per job instead of N round trips.
- ``get_all_job_info()`` calls it once per request and adds
``pending_findings_count`` to each job's API response.
- ``enrichment.js`` job card now branches on the count:
- ``> 0`` → red ``"X pending"`` badge (urgent, action needed)
- ``= 0`` AND last scan found something → muted grey ``"X found in
last scan"`` (historical context, no action needed)
- New CSS class ``.repair-flow-badge.findings-historical`` for the
muted slate color so the two states are visually distinct.
User-visible result with the screenshotted state (372 dup / 60 cover-
art findings, all resolved):
- Before: red "372 findings" / "60 findings" — implied 432 things to
do, but Findings tab showed 0 pending
- After: grey "372 found in last scan" / "60 found in last scan" —
the badge text tells the user the count is historical, no surprise
when Pending is empty
Tests: 3 new tests in ``tests/test_create_finding_dedup_counter.py``
pin the per-job pending count helper:
- returns ``{job_id: count}`` based on status='pending' rows only;
resolved + dismissed rows excluded
- empty dict when no pending findings exist
- gracefully returns ``{}`` on DB error (badge falls back to
historical count via the existing JS ``or 0`` safety)
2188/2188 full suite green. Pure UI/state-display fix — no detector
logic, no backend behavior change.
This commit is contained in:
Broque Thomas
parent
7bd15438da
commit
4c11375930
5 changed files with 155 additions and 5 deletions
|
|
@ -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
|
||||
# ------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -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 == {}
|
||||
|
|
|
|||
|
|
@ -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">→</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">→</span>');
|
||||
flowParts.push(`<span class="repair-flow-badge findings findings-historical">${lastScanCount} found in last scan</span>`);
|
||||
}
|
||||
|
||||
// Build meta parts
|
||||
|
|
|
|||
|
|
@ -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' },
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Reference in a new issue