v2.13.0 — visible failures, real retries, and the wrong-version fix

.github/workflows/tests.yml

@@ -0,0 +1,78 @@
+name: Tests
+
+# WHY THIS EXISTS
+#   The repo had ~20 test files and nothing that ran them. Every suite was green
+#   only when someone remembered to run it by hand, and two order-dependency bugs
+#   had already crept in unnoticed (test modules fighting over db.py's module-level
+#   DB_PATH, which is decided by whichever test imports it first).
+#
+#   pytest is the runner, not unittest: some suites use bare test functions that
+#   `python -m unittest` silently collects as zero tests and reports as success.
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+  workflow_dispatch:
+
+permissions:
+  contents: read
+
+jobs:
+  python:
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        service: [sync-api, sync-worker]
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.12"
+          cache: pip
+
+      - name: Install dependencies
+        working-directory: ${{ matrix.service }}
+        run: |
+          python -m pip install --upgrade pip
+          pip install -r requirements.txt
+          pip install pytest httpx
+          # sync-worker's Dockerfile installs tiddl separately from requirements.txt.
+          # Mirror that here: without it _refresh_tidal_token cannot resolve the Tidal
+          # client credentials and returns early, so its test fails on an absent
+          # dependency rather than on anything real.
+          if [ "${{ matrix.service }}" = "sync-worker" ]; then pip install tiddl; fi
+
+      - name: Run tests
+        working-directory: ${{ matrix.service }}
+        # Run the whole suite in ONE process on purpose: cross-module interference
+        # via shared module state is a real failure mode here, and running each
+        # file in isolation would hide exactly the bug this job exists to catch.
+        run: python -m pytest tests -q
+
+  web:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-node@v4
+        with:
+          node-version: "20"
+          cache: npm
+          cache-dependency-path: sync-web/package-lock.json
+
+      - name: Install dependencies
+        working-directory: sync-web
+        run: npm ci
+
+      - name: Typecheck
+        working-directory: sync-web
+        run: npx tsc --noEmit
+
+      - name: Build
+        working-directory: sync-web
+        # `next build` catches what tsc cannot -- most importantly a useSearchParams()
+        # call outside a Suspense boundary, which fails at the prerender step.
+        run: npm run build

CHANGELOG.md

@@ -1,5 +1,108 @@
 # Changelog
 
+## 2.13.0 — visible failures, real retries, and the wrong-version fix
+
+Three problems that were reported as feature requests turned out to be something
+other than they looked. Measuring them first is what made this release small.
+
+### The Missing Tracks page was never empty — it was erroring
+
+`/upload` asked for `per_page=500` from an endpoint capped at 200. The API replied
+with a 422 explaining exactly that, and the page rendered a clean empty table.
+
+The reason it survived so long is `api.ts`: it threw away the response body and
+raised `Error("API error: 422")`, so every caller's `catch { setRows([]) }` turned a
+loud, specific server error into "you have no missing tracks". The client now keeps
+the status and FastAPI's `detail`, retries only transport faults and 5xx (never a
+4xx, which is deterministic), and the page pages through all 270 errors instead of
+truncating at the cap.
+
+### "Fingerprint mismatch" involved no fingerprints
+
+The category matched on duration, not on the chromaprint WaxFlow computes and
+stores. 108 of the 109 tracks in it came from one path: the file-index title+artist
+matcher, which accepted any file whose title and artist agreed — so an extended
+mix, a radio edit or a live take all matched. Verify then rejected the file on
+duration and parked the track as an error.
+
+Measured across 265 such matches on the live library: 155 land within 5s of the
+Spotify duration and 110 exceed it, 95 of those by more than 15s. The two
+populations separate cleanly, so title+artist matches now require the durations to
+agree within 5s (configurable; fails open when either duration is unknown, as ~0.4%
+of indexed files have none). The category is renamed **Wrong Version**, which is
+what it always was.
+
+### Post-processing was working the whole time
+
+Of 400 tracks imported since July: 400 had BPM, 399 had cue points, 394 artwork,
+399 cloud-backed — and only 4 tracks in the entire library lacked cues. Nothing was
+broken; there was no way to see it. The dashboard now shows coverage for cue
+points, beat grids, BPM, key, genre and tags, sampled hourly by the worker.
+
+It also honestly reports what it *cannot* measure: Lexicon's API exposes no artwork
+or cloud-upload field, so those are named as unavailable rather than guessed at.
+
+One real bug did surface here: **"Auto-Analyze After Sync" silently disabled cue
+generation, tag lookup and cloud upload too**, despite each having its own checkbox
+in Settings and the toggle's own description mentioning only BPM/key detection.
+Turning off analysis now turns off exactly analysis.
+
+### Bulk retry
+
+`POST /api/tracks/bulk-retry` takes explicit ids or a category name resolved
+server-side by the same classifier that renders the count — so "Retry All 47"
+acts on those 47, not on a set that drifted. The Errors page gains per-category
+"Retry All" alongside the existing "Ignore All".
+
+It also **clears `fallback_attempts`**, without which a retry was quietly inert:
+`already_attempted()` treats any prior row as "we tried this", so the track reset,
+walked back down the pipeline, and failed identically without ever re-contacting
+Soulseek. The existing single-track retry had the same defect and is fixed too.
+
+Bulk operations now write one summary activity row instead of one per track;
+ignoring a category used to insert thousands and bury the dashboard feed.
+
+### Soulseek in service health
+
+`is_logged_in()` existed and was never surfaced. Because sync-api cannot import
+worker code and has no slskd credentials, the worker probes every 120s and persists
+the verdict to `app_config` for the API to serve — the pattern `lexicon_health`
+already uses. It distinguishes logged-out (slskd answers, but searches return
+nothing) from unreachable, and never-configured from broken. A verdict older than
+15 minutes reports as unknown rather than repeating a stale "ok".
+
+The dashboard now renders whatever services the API reports rather than a hardcoded
+list of three.
+
+### Dashboard drill-down
+
+Clicking a month's green/red/grey segment opens that exact set of tracks. The
+`month` filter is a half-open range rather than `substr(spotify_added_at, 1, 7)`, so
+it can use an index — and `tracks` had no index at all beyond its implicit primary
+key one, so `spotify_added_at`, `pipeline_stage` and `lexicon_status` now have them.
+
+Segment widths come from flex-grow on raw counts; three independently rounded
+percentages could total 101% and overflow the bar.
+
+### Also
+
+- The dashboard health probe called `GET /v1/tracks` — Lexicon's **entire library**
+  — every 10 seconds. It now calls `/v1/playlists`, as `lexicon_health` already did.
+- The nav error badge polled every errored track in full for a single integer;
+  there is now a counts-only summary endpoint.
+- **CI runs the tests.** The repo had ~20 test files and no workflow that executed
+  them. It runs pytest (not `unittest`, which silently collects zero tests from the
+  bare-function suites and reports success), plus a web typecheck and a real
+  `next build`.
+- Fixed two order-dependency bugs the new CI immediately exposed: test modules were
+  fighting over `db.py`'s module-level `DB_PATH`, which is decided by whichever test
+  imports it first.
+- The Tidal token-refresh test had been failing for anyone running the suite outside
+  the container. `tiddl` is installed by `sync-worker/Dockerfile` but is absent from
+  `requirements.txt`, so the function under test bailed out on missing credentials
+  before reaching the code the test was asserting on. CI now mirrors the Dockerfile.
+
+
 ## 2.12.4 — fix: the updater used the wrong compose project name
 
 The updater mounts the project at `/project`, and Compose derives the project name

VERSION

@@ -1 +1 @@
-2.12.4
+2.13.0

sync-api/error_categories.py

@@ -0,0 +1,65 @@
+"""Error-bucket classification for the Errors page.
+
+WHY THIS IS ITS OWN MODULE
+    The Errors page groups failures into buckets, and "Retry All" acts on a
+    bucket. If the endpoint that RENDERS a bucket and the endpoint that RETRIES
+    it classify tracks even slightly differently, the button retries a different
+    set than the number beside it claims. Both now call this one function, so
+    they cannot drift.
+
+The rules are ordered: the first match wins. `verify_codec` is checked before the
+error text because lossy tracks used to fall through to `download_failed`.
+"""
+
+from __future__ import annotations
+
+# Display order on the Errors page.
+ERROR_CATEGORIES: tuple[str, ...] = (
+    "not_lossless",
+    "no_tidal_match",
+    "download_failed",
+    "lexicon_sync_failed",
+    "wrong_version",
+    "other",
+)
+
+# Renamed in 2.13.0. The old key said "fingerprint_mismatch", but no fingerprint
+# is ever compared -- the check is duration-based, and what it actually catches is
+# a DIFFERENT EDIT of the right song (radio edit vs extended mix, and so on).
+# Kept so an older frontend still resolves the bucket.
+CATEGORY_ALIASES: dict[str, str] = {"fingerprint_mismatch": "wrong_version"}
+
+
+def canonical_category(name: str) -> str:
+    """Map a possibly-legacy category key to its current name."""
+    return CATEGORY_ALIASES.get(name, name)
+
+
+def categorize_error(track: dict) -> str:
+    """Bucket one errored track. Returns a key from ERROR_CATEGORIES."""
+    err = (track.get("pipeline_error") or "").lower()
+    verify_status = track.get("verify_status")
+    verify_codec = (track.get("verify_codec") or "").lower()
+
+    if verify_codec in ("aac", "mp3"):
+        return "not_lossless"
+    if verify_status == "fail" and "not lossless" in err:
+        return "not_lossless"
+    if "not lossless" in err or "aac" in err or "mp3" in err:
+        return "not_lossless"
+    if (
+        "no tidal match" in err
+        or "no match" in err
+        or "not found on tidal" in err
+        or "permanently unavailable" in err
+    ):
+        return "no_tidal_match"
+    if "geo-restricted" in err or "unavailable on tidal" in err:
+        return "download_failed"
+    if "download failed" in err or "download error" in err:
+        return "download_failed"
+    if "lexicon" in err:
+        return "lexicon_sync_failed"
+    if "fingerprint" in err or "mismatched" in err or "wrong version" in err:
+        return "wrong_version"
+    return "other"

sync-api/init_db.py

@@ -583,6 +583,12 @@ def init():
             ON source_attempts(track_id, source);
         CREATE INDEX IF NOT EXISTS idx_wanted_track ON wanted(track_id);
         CREATE INDEX IF NOT EXISTS idx_import_queue_state ON import_queue(state);
+        -- tracks carried no index at all beyond its implicit PK one. These three
+        -- back the dashboard month drill-down and the filters every list view uses.
+        CREATE INDEX IF NOT EXISTS idx_tracks_spotify_added_at
+            ON tracks(spotify_added_at);
+        CREATE INDEX IF NOT EXISTS idx_tracks_pipeline_stage ON tracks(pipeline_stage);
+        CREATE INDEX IF NOT EXISTS idx_tracks_lexicon_status ON tracks(lexicon_status);
         """)
     with get_db() as conn:
         cols = {r[1] for r in conn.execute("PRAGMA table_info(tracks)").fetchall()}

sync-api/models.py

@@ -58,6 +58,17 @@ class TrackUpdate(BaseModel):
     file_path: Optional[str] = None
 
 
+class BulkRetryRequest(BaseModel):
+    """Retry many tracks by explicit id, or by error category resolved server-side.
+
+    `limit` is a safety rail: a category retry can cover thousands of tracks, and
+    the pipeline picks work up every 10 seconds.
+    """
+    track_ids: Optional[list[int]] = None
+    category: Optional[str] = None
+    limit: int = Field(default=500, ge=1, le=5000)
+
+
 class TrackListResponse(BaseModel):
     tracks: list[TrackOut]
     total: int

sync-api/routes/dashboard.py

@@ -12,6 +12,57 @@
 LEXICON_API = os.environ.get("LEXICON_API_URL", "http://localhost:48624")
 TIDARR_API = os.environ.get("TIDARR_URL", "http://localhost:8484")  # optional legacy fallback
 
+# The worker re-probes Soulseek every 120s. If the last verdict is older than this,
+# the worker itself is the thing that is unwell, and reporting its last known "ok"
+# would be reporting a lie.
+_SOULSEEK_STALE_AFTER_SECONDS = 900
+
+# Statuses that mean "deliberately off", not "broken".
+_SOULSEEK_INACTIVE = {"disabled", "not_configured"}
+
+
+def _soulseek_service(conn) -> ServiceHealth:
+    """Report the verdict last persisted by the worker's soulseek_health task."""
+    keys = ("soulseek_health", "soulseek_detail", "soulseek_checked_at", "soulseek_latency_ms")
+    rows = conn.execute(
+        f"SELECT key, value FROM app_config WHERE key IN ({','.join('?' * len(keys))})",
+        keys,
+    ).fetchall()
+    cfg = {r["key"]: r["value"] for r in rows}
+
+    status = (cfg.get("soulseek_health") or "").strip()
+    detail = cfg.get("soulseek_detail") or None
+    checked_at = cfg.get("soulseek_checked_at")
+
+    if not status:
+        return ServiceHealth(
+            name="soulseek", status="unknown",
+            error="no health check recorded yet — is the worker running?",
+        )
+
+    try:
+        latency = float(cfg["soulseek_latency_ms"]) if cfg.get("soulseek_latency_ms") else None
+    except (TypeError, ValueError):
+        latency = None
+
+    if checked_at and status not in _SOULSEEK_INACTIVE:
+        try:
+            from datetime import datetime, timezone
+            age = (datetime.now(timezone.utc) - datetime.fromisoformat(checked_at)).total_seconds()
+            if age > _SOULSEEK_STALE_AFTER_SECONDS:
+                return ServiceHealth(
+                    name="soulseek", status="unknown", latency_ms=latency,
+                    error=f"last checked {int(age // 60)} min ago — worker may be stalled",
+                )
+        except (TypeError, ValueError):
+            pass
+
+    if status == "ok":
+        return ServiceHealth(name="soulseek", status="ok", latency_ms=latency)
+    if status in _SOULSEEK_INACTIVE:
+        return ServiceHealth(name="soulseek", status="disabled", error=detail)
+    return ServiceHealth(name="soulseek", status="error", latency_ms=latency, error=detail)
+
 
 @router.get("/dashboard", response_model=DashboardResponse)
 async def get_dashboard():
@@ -79,11 +130,14 @@ async def get_dashboard():
         # Service health checks
         services = []
 
-        # Lexicon
+        # Lexicon. Probe /v1/playlists, NOT /v1/tracks: this endpoint is polled every
+        # 10s by the dashboard, and /v1/tracks returns the ENTIRE library (5,600+
+        # rows) on every single call. lexicon_health._check_lexicon_reachable already
+        # uses /v1/playlists for exactly this reason.
         try:
             t0 = time.monotonic()
             async with httpx.AsyncClient(timeout=5.0) as client:
-                resp = await client.get(f"{LEXICON_API}/v1/tracks")
+                resp = await client.get(f"{LEXICON_API}/v1/playlists")
             latency = round((time.monotonic() - t0) * 1000, 1)
             services.append(ServiceHealth(
                 name="lexicon",
@@ -109,6 +163,10 @@ async def get_dashboard():
         except Exception as e:
             services.append(ServiceHealth(name="tidal", status="error", error=str(e)))
 
+        # Soulseek. The API has neither the slskd credentials nor the worker's client,
+        # so it reports what the worker's soulseek_health probe last persisted.
+        services.append(_soulseek_service(conn))
+
         return DashboardResponse(
             spotify_total=spotify_total,
             lexicon_synced=lexicon_synced,