Fix skipped_lock creating phantom empty genre/dedup runs; add Run now to playlist list

genre_review_service.run() and dedup_review_service._run_scan() both
unconditionally created a new run row even when the underlying job never
actually executed (skipped_lock, or any other non-success status). For
genres this was directly user-visible: the review page always shows the
most recent run, so a skipped click created an empty run that displaced
the real previous preview, making it look like every pending change had
vanished. Both now return None (persisting nothing) when the job didn't
succeed, and both routers surface a "didn't run, something else was
using the pipeline" notice instead of silently redirecting. Cleaned up
the one phantom empty genre_runs row already sitting in production,
restoring the real 20-change preview.

Also added a "Run now" button to each row on the Playlists list page
(previously only on a playlist's own detail page), for the common case
of adding new tracks and wanting to sync immediately.

app/routers/dedup.py

@@ -57,7 +57,9 @@ async def scan(user: dict = Depends(require_auth)):
     # Runs both the file-naming and acoustic passes -- see
     # dedup_review_service.scan_all(). The candidates table shows which
     # pass caught each one instead of needing two separate buttons.
-    await dedup_review_service.scan_all(triggered_by="manual")
+    tag_run, fuzzy_run = await dedup_review_service.scan_all(triggered_by="manual")
+    if tag_run is None or fuzzy_run is None:
+        return RedirectResponse(url="/dedup?skipped=1", status_code=303)
     return RedirectResponse(url="/dedup", status_code=303)
 
 

app/routers/genres.py

@@ -32,13 +32,17 @@ async def scan(user: dict = Depends(require_auth)):
     # Always force=True: matches the weekly scheduled run's own behavior
     # (see genre_review_service.run's docstring), so a preview accurately
     # shows what "Fix genres now" would actually do.
-    await genre_review_service.run(apply=False, force=True, triggered_by="manual")
+    result = await genre_review_service.run(apply=False, force=True, triggered_by="manual")
+    if result is None:
+        return RedirectResponse(url="/genres?skipped=1", status_code=303)
     return RedirectResponse(url="/genres", status_code=303)
 
 
 @router.post("/apply")
 async def apply(user: dict = Depends(require_auth)):
-    await genre_review_service.run(apply=True, force=True, triggered_by="manual")
+    result = await genre_review_service.run(apply=True, force=True, triggered_by="manual")
+    if result is None:
+        return RedirectResponse(url="/genres?skipped=1", status_code=303)
     return RedirectResponse(url="/genres", status_code=303)
 
 

app/services/dedup_review_service.py

@@ -70,11 +70,20 @@ def _parse_json_lines(output: str) -> list[dict]:
     return candidates
 
 
-async def _run_scan(job_key: str, script_name: str, triggered_by: str) -> DedupRun:
+async def _run_scan(job_key: str, script_name: str, triggered_by: str) -> DedupRun | None:
+    """Returns None (persisting nothing) if the job never actually ran --
+    e.g. skipped_lock because something else was using the pipeline lock
+    at that moment. Same lesson as genre_review_service.run(): recording a
+    "run happened" row for a run that didn't is just misleading history,
+    even though (unlike genres) it isn't user-visible here today since the
+    pending-candidates list isn't scoped to "latest run only"."""
     script = str(settings.pipeline_dir / "lib" / script_name)
     job_run, output = await pipeline_runner.run_job_capture(
         job_key, [script, "--json"], triggered_by=triggered_by, timeout=_JOB_TIMEOUT_SECONDS
     )
+    if job_run.status != "success":
+        return None
+
     candidates = _parse_json_lines(output)
 
     db = SessionLocal()
@@ -128,7 +137,7 @@ async def _run_scan(job_key: str, script_name: str, triggered_by: str) -> DedupR
         db.close()
 
 
-async def scan(triggered_by: str = "manual") -> DedupRun:
+async def scan(triggered_by: str = "manual") -> DedupRun | None:
     """Dry-run dedup-library.sh --json, persist every candidate deletion
     into a fresh dedup_runs/dedup_candidates pair. Never deletes anything
     -- the scheduled maintenance:dedup job also only ever calls this (no
@@ -137,7 +146,7 @@ async def scan(triggered_by: str = "manual") -> DedupRun:
     return await _run_scan("dedup:scan", _SCRIPT, triggered_by)
 
 
-async def scan_fuzzy(triggered_by: str = "manual") -> DedupRun:
+async def scan_fuzzy(triggered_by: str = "manual") -> DedupRun | None:
     """Dry-run find-fuzzy-dupes.py --json. Tag-based passes (scan() above)
     only catch duplicates whose artist/title tags overlap; this compares
     Chromaprint audio fingerprints instead, so it also catches the same
@@ -146,10 +155,14 @@ async def scan_fuzzy(triggered_by: str = "manual") -> DedupRun:
     return await _run_scan("dedup:scan_fuzzy", _FUZZY_SCRIPT, triggered_by)
 
 
-async def scan_all(triggered_by: str = "manual") -> tuple[DedupRun, DedupRun]:
+async def scan_all(triggered_by: str = "manual") -> tuple[DedupRun | None, DedupRun | None]:
     """Run both passes back to back for a single "Scan now" click -- each
     call acquires and releases the shared pipeline lock on its own, so
-    running them sequentially here is safe, just one after the other."""
+    running them sequentially here is safe, just one after the other.
+    Either element is None if that particular pass got skipped_lock/failed
+    (see _run_scan) -- e.g. the fuzzy pass can't start until the tag-based
+    pass above it has released the lock, so it's not unusual for one to
+    succeed and the other to be skipped by something else entirely."""
     tag_run = await scan(triggered_by=triggered_by)
     fuzzy_run = await scan_fuzzy(triggered_by=triggered_by)
     return tag_run, fuzzy_run

app/services/genre_review_service.py

@@ -21,7 +21,7 @@ def _parse_json_lines(output: str) -> list[dict]:
     return candidates
 
 
-async def run(apply: bool, force: bool = False, triggered_by: str = "schedule") -> GenreRun:
+async def run(apply: bool, force: bool = False, triggered_by: str = "schedule") -> GenreRun | None:
     """Run spotify-genre.py --json (dry-run or --apply), persisting the
     diff into a fresh genre_runs/genre_candidates pair either way.
 
@@ -29,7 +29,14 @@ async def run(apply: bool, force: bool = False, triggered_by: str = "schedule")
     (low-risk/reversible, GENRE_LOCK-protected) -- this wraps that same
     scheduled run so its diff becomes reviewable after the fact, rather
     than being a separate preview-only mode. scheduler_service's
-    maintenance:spotify_genre job calls this with apply=True."""
+    maintenance:spotify_genre job calls this with apply=True.
+
+    Returns None (and persists nothing) if the job never actually ran --
+    e.g. skipped_lock because something else was using the pipeline at
+    that moment. Without this check, a skipped/failed attempt still
+    created an empty genre_runs row, which then became "latest run" on
+    the review page (it's always the most recent by id) and hid the real
+    previous run's candidates behind a phantom empty one."""
     args = ["--json"]
     if apply:
         args.append("--apply")
@@ -40,6 +47,9 @@ async def run(apply: bool, force: bool = False, triggered_by: str = "schedule")
     job_run, output = await pipeline_runner.run_job_capture(
         "genre:run", [script] + args, triggered_by=triggered_by
     )
+    if job_run.status != "success":
+        return None
+
     candidates = _parse_json_lines(output)
 
     db = SessionLocal()

app/templates/dedup/index.html

@@ -13,6 +13,10 @@
   </div>
 </div>
 
+{% if request.query_params.get('skipped') %}
+<div class="notice warning">One or both passes didn't run — something else was using the pipeline at that moment (check <a href="/settings/jobs">Jobs</a> for what). Try again in a moment.</div>
+{% endif %}
+
 <h2>Pending candidates</h2>
 <p class="muted" style="margin-top:-0.5rem;">Paths are relative to the library root. Hover a name for the full path.</p>
 <form id="bulk-delete-form" method="post" action="/dedup/confirm"></form>

app/templates/genres/index.html

@@ -8,6 +8,10 @@
   </div>
 </div>
 
+{% if request.query_params.get('skipped') %}
+<div class="notice warning">Didn't run — something else was using the pipeline at that moment (check <a href="/settings/jobs">Jobs</a> for what). Nothing changed; try again in a moment.</div>
+{% endif %}
+
 <div class="panel" style="max-width:560px;">
   <p class="muted" style="margin-top:0;">Preview shows what would change, including overwriting existing genres. Fix writes those changes for real.</p>
   <div class="btn-row">

app/templates/playlists/index.html

@@ -21,6 +21,9 @@
         <td class="muted mono">{{ sync_times.get(p.id) or "unscheduled" }}</td>
         <td class="muted">{{ "yes" if p.no_m3u else "no" }}</td>
         <td class="actions-cell">
+          <form method="post" action="/settings/jobs/playlist:{{ p.name }}/run" class="inline">
+            <button type="submit" class="btn btn-sm" title="Sync this playlist now instead of waiting for its scheduled time">Run now</button>
+          </form>
           <form method="post" action="/playlists/{{ p.id }}/delete" class="inline"
                 onsubmit="return confirm('Remove {{ p.name }}? This deletes its rendered config but not any downloaded music.')">
             <button type="submit" class="btn btn-sm btn-danger">Remove</button>