refactor(server): extract the wishlist routes into routers/wanted.py (R3)

Free after _clean_str moved to lib (#841): wanted's deps are JSONResponse,
_clean_str (reqfields), app + meta_db (seam). 3 routes (list/add/remove),
0 setattr targets, 0 helpers to relocate.

Bodies verbatim; @app -> @router, meta_db -> appstate.meta_db. include_router at
the original site; 143-route table identical. No test retargeting.

server.py: 8,003 -> 7,974 (this branch is independent of the chart PR).

Verified: pyflakes clean; route table identical; pytest 2401 passed (52 in
test_wanted_api); eslint 0. Boot smoke: add wishlist entry -> list -> delete,
400 on missing artist+title (_clean_str path).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

CHANGELOG.md

@@ -26,7 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   next root-level module can't ship broken.
 
 ### Added
-- **`routers/` — extracting `server.py`'s route layer, cheapest-first (R3).** Each PR moves a cohesive route group into a `fastapi.APIRouter` under `lib/routers/`, mounted with `app.include_router(...)` at its original site (FastAPI matches in registration order; the full route table stays byte-identical). Bodies are verbatim — only the decorator receiver (`@app` → `@router`) and singleton reads (`meta_db` → `appstate.meta_db`, resolved at call time) change. So far: `audio_effects` (5), `artist_aliases` (5), `loops` (3 — saved A/B practice regions). The next cut is picked by a dependency-closure scan that ranks groups by how many `monkeypatch.setattr(server, …)` targets they'd drag along.
+- **`routers/` — extracting `server.py`'s route layer, cheapest-first (R3).** Each PR moves a cohesive route group into a `fastapi.APIRouter` under `lib/routers/`, mounted with `app.include_router(...)` at its original site (FastAPI matches in registration order; the full route table stays byte-identical). Bodies are verbatim — only the decorator receiver (`@app` → `@router`) and singleton reads (`meta_db` → `appstate.meta_db`, resolved at call time) change. So far: `audio_effects` (5), `artist_aliases` (5), `loops` (3), `playlists` (12 + covers), `ws_highway` (the 902-line highway chart WebSocket), `chart` (split/unsplit/work/fileinfo — unblocked by the DLC-path substrate). The DLC library-path resolution (`_get_dlc_dir`, pure `_resolve_dlc_path`) moved to `lib/dlc_paths.py`, reading paths through the seam; `config_dir`/`dlc_dir`/`dlc_dir_env` now ride the `appstate` seam (env-derived, so the pop-and-reimport fixtures reconfigure it for free), and the shared request-field sanitizer `_clean_str` moved to `lib/reqfields.py`. The next cut is picked by a dependency-closure scan that ranks groups by how many `monkeypatch.setattr(server, …)` targets they'd drag along.
 - **`routers/` — the first extracted route module (R3).** The five audio-effects mapping
   endpoints move out of `server.py` into `lib/routers/audio_effects.py` as a
   `fastapi.APIRouter`, mounted with `app.include_router(...)` **at the point in the file

docs/size-exemptions.md

@@ -55,8 +55,8 @@ without a *signed* exemption" is unenforceable.
 ## Planned, NOT exempt (owned by split plans — listed so nothing falls between states)
 
 core `static/app.js` (11,852) · `static/highway.js` (4,168, whole file) · `server.py`
-(9,302 — was 14,037; ratcheted by the R3 `MetadataDB` + `AudioEffectsMappingDB`
-extractions and the first three `routers/` modules) ·
+(7,880 — was 14,037; ratcheted by the R3 `MetadataDB` + `AudioEffectsMappingDB`
+extractions and seven `routers/` modules) ·
 `lib/metadata_db.py` (4,373 — new in R3; the `MetadataDB` class alone is 4,018 lines
 and is a monolith in its own right, to be split per-table once the router train
 lands) · `static/v3/songs.js` (4,134) · `static/capabilities/audio-session.js`

lib/appstate.py

@@ -62,10 +62,31 @@ copies a hardcoded file list — that regression is what moved this file here.
 meta_db = None
 audio_effect_mappings = None
 
-# Declared up front so `configure()` can reject a typo'd or stale keyword
-# instead of silently creating a new global that nothing ever reads. A seam
-# whose wiring can no-op undetected is worse than no seam.
-_SLOTS = frozenset({"meta_db", "audio_effect_mappings"})
+# Config paths. server.py derives these from the environment (fresh on every
+# import, so the ~49 pop-and-reimport fixtures keep working) and injects them
+# here. Routers read them as `appstate.config_dir` etc. — a module attribute at
+# call time. NOTE: config_dir/dlc_dir are env-derived, so a `setenv`+reimport
+# test reconfigures them for free; STATIC_DIR/SLOPPAK_CACHE_DIR are patched via
+# `setattr(server, …)` in a few tests, so those slots (when added) need their
+# tests retargeted to appstate in the same PR.
+config_dir = None
+dlc_dir = None          # the DLC_DIR env value as a Path (Path("") if unset)
+dlc_dir_env = None      # the raw DLC_DIR env string, "" if unset — distinguishes
+                        # "unset" from Path("")→"." (see dlc_paths._get_dlc_dir)
+# Cache/asset dirs. static_dir + sloppak_cache_dir are patched via
+# `setattr(server, …)` in a few tests, so a router reading them here needs those
+# setattr sites retargeted to `setattr(appstate, …)` in the same PR (ws_highway
+# retargets the 3 test_highway_ws_* SLOPPAK sites). config_dir-derived dirs are
+# reconfigured for free on a setenv+reimport.
+static_dir = None
+sloppak_cache_dir = None
+audio_cache_dir = None
+
+_SLOTS = frozenset({
+    "meta_db", "audio_effect_mappings",
+    "config_dir", "dlc_dir", "dlc_dir_env",
+    "static_dir", "sloppak_cache_dir", "audio_cache_dir",
+})
 
 
 def configure(**kwargs) -> None:

lib/dlc_paths.py

@@ -0,0 +1,99 @@
+"""DLC library path resolution — where the song files live, plus safe containment.
+
+Extracted from ``server.py`` (R3). ``_resolve_dlc_path`` is pure and moved
+verbatim. ``_get_dlc_dir`` reads the env-derived paths through the ``appstate``
+seam (``server.py`` configures ``dlc_dir``/``dlc_dir_env``/``config_dir`` at
+import, fresh on every re-import), so this module does no import-time IO and the
+pop-and-reimport fixtures keep working. ``server.py`` re-exports both names, so
+existing ``server._get_dlc_dir`` / ``server._resolve_dlc_path`` references
+(tests, other handlers) resolve unchanged.
+"""
+
+import json
+import os
+from pathlib import Path
+
+import appstate
+
+
+def _get_dlc_dir(cfg: dict | None = None) -> Path | None:
+    # Only consider DLC_DIR if the env var was non-empty. `Path("")` collapses
+    # to `.` and reports `.is_dir() == True`, which would silently shadow the
+    # config.json fallback. Checking the raw env string preserves
+    # `DLC_DIR=.` as a valid opt-in for cwd while keeping unset/empty out.
+    if appstate.dlc_dir_env and appstate.dlc_dir.is_dir():
+        return appstate.dlc_dir
+    if cfg is None:
+        config_file = appstate.config_dir / "config.json"
+        if config_file.exists():
+            try:
+                cfg = json.loads(config_file.read_text(encoding="utf-8"))
+            except Exception:
+                pass
+    if isinstance(cfg, dict):
+        raw = str(cfg.get("dlc_dir", "")).strip()
+        if raw:
+            p = Path(raw)
+            if p.is_dir():
+                return p
+    return None
+
+
+def _resolve_dlc_path(dlc: Path, filename: str) -> Path | None:
+    """Resolve `filename` under DLC_DIR and refuse anything that escapes.
+
+    `filename` arrives from `:path` route params and can contain `..`
+    segments. The Sloppak and archive paths happen to fail safely later
+    because their loaders raise on missing/invalid files, but loose-
+    folder format detection (`is_loose_song`) globs and parses XML on
+    disk first, which lets a crafted path trigger filesystem reads
+    outside DLC_DIR before any guard fires. Centralise the containment
+    check so every filename-bound handler validates before touching the
+    filesystem.
+
+    Containment here is LEXICAL (normalize `.`/`..` WITHOUT following
+    symlinks), not `safe_join`'s `.resolve()`-based check — because users
+    commonly mount their song library through a directory JUNCTION/symlink
+    (a library shared across app installs; the desktop app's own mounts).
+    `.resolve()` follows that junction to its real target, sees it sits
+    outside DLC_DIR, and wrongly rejects every song reached through it — the
+    scanner's `rglob` indexes those songs, but art/load then 403/404s (broken
+    covers, unplayable songs). Lexical normalization still rejects the only
+    escapes a `:path` filename can express — `..` traversal and absolute
+    paths — which the traversal tests pin. `safe_join` stays strict (it is
+    the zip-slip / plugin-asset guard, where following a symlink out IS the
+    defense); the loose-folder art handler keeps its own per-file symlink
+    re-check for defence-in-depth.
+
+    Returns the validated Path (not necessarily link-resolved), or None if
+    the filename is empty, contains a NUL, or escapes the DLC root.
+    """
+    if not filename:
+        return None
+    # Backslashes → forward slashes so a Windows-style `..\\x` traversal is
+    # rejected identically on POSIX (mirrors safe_join's normalisation).
+    safe = filename.replace("\\", "/")
+    if "\x00" in safe:
+        return None
+    # Reject drive-letter / absolute paths in BOTH conventions. A POSIX "/x" is
+    # caught by the containment check below (the `/` operator discards `root`),
+    # but a Windows drive-absolute "C:/x" is treated as a relative "C:" dir on
+    # POSIX and would otherwise slip in as `<root>/C:/x` — so the contract must
+    # hold cross-platform (a shared library is reached from either OS).
+    from pathlib import PurePosixPath, PureWindowsPath
+    if (PurePosixPath(safe).is_absolute()
+            or PureWindowsPath(safe).is_absolute()
+            or PureWindowsPath(safe).drive):
+        return None
+    try:
+        root = dlc.resolve()
+        # normpath collapses `.`/`..`/duplicate separators purely lexically —
+        # it never touches the filesystem, so an in-library junction component
+        # is preserved (allowed) while `..`/absolute segments still escape and
+        # get caught by the containment check below.
+        candidate = Path(os.path.normpath(root / safe))
+        if not candidate.is_relative_to(root):
+            return None
+    except (ValueError, OSError):
+        return None
+    return candidate

lib/reqfields.py

@@ -0,0 +1,13 @@
+"""Request-field coercion helpers shared by the raw-`dict` POST handlers.
+
+Extracted verbatim from ``server.py`` (R3). Pure — no IO, no globals — so it
+imports cleanly from both ``server`` and any ``routers/`` module.
+"""
+
+
+def _clean_str(value) -> str:
+    """Trim a request field to a string; non-strings (or missing) → ''.
+    Lets the raw-`dict` POST handlers treat wrong-typed JSON (an int/list/etc.
+    where a string was expected) as "empty" and answer 400, instead of raising
+    AttributeError/TypeError → 500 on a later .strip()/`in`."""
+    return value.strip() if isinstance(value, str) else ""

lib/routers/chart.py

@@ -0,0 +1,115 @@
+"""Chart-level endpoints — split/unsplit a chart from its work, resolve work
+membership, and the context-menu "Get info" file inspector.
+
+Extracted verbatim from ``server.py`` (R3); edits: ``@app`` -> ``@router``,
+``meta_db`` -> ``appstate.meta_db``. DLC path resolution comes from
+``dlc_paths``; sloppak/loose detection from the shared lib modules.
+"""
+
+from fastapi import APIRouter, HTTPException
+
+import appstate
+from dlc_paths import _get_dlc_dir, _resolve_dlc_path
+import sloppak as sloppak_mod
+import loosefolder as loosefolder_mod
+
+router = APIRouter()
+
+
+@router.post("/api/chart/{filename:path}/split")
+def api_split_chart(filename: str):
+    """'These aren't the same song' — split this chart out as its own singleton
+    work. Under /api/chart (NOT /api/song) so the DELETE /api/song/{path}
+    catch-all can't shadow it."""
+    key = appstate.meta_db._canonical_song_filename(filename)
+    appstate.meta_db.split_chart(key)
+    return {"ok": True, "filename": key}
+
+
+@router.post("/api/chart/{filename:path}/unsplit")
+def api_unsplit_chart(filename: str):
+    """Undo a split — rejoin the chart to its work."""
+    key = appstate.meta_db._canonical_song_filename(filename)
+    appstate.meta_db.unsplit_chart(key)
+    return {"ok": True, "filename": key}
+
+
+@router.get("/api/chart/{filename:path}/work")
+def api_get_chart_work(filename: str):
+    """Resolve a chart's work membership: {work_key, chart_count}. For openers
+    on rows that came from an ungrouped query (the tree view) — grouped grid
+    rows already carry both fields inline."""
+    return appstate.meta_db.chart_work(filename)
+
+
+@router.get("/api/chart/{filename:path}/fileinfo")
+def api_chart_fileinfo(filename: str):
+    """The context menu's "Get info": where the file lives + what the pack
+    contains. Under /api/chart — the GET /api/song/{path} catch-all would
+    swallow a /api/song/…/fileinfo suffix. Read-only; demo-mode blocks it
+    because it exposes filesystem paths."""
+    dlc = _get_dlc_dir()
+    if not dlc:
+        raise HTTPException(status_code=404, detail="not configured")
+    p = _resolve_dlc_path(dlc, filename)
+    if p is None:
+        raise HTTPException(status_code=403, detail="forbidden")
+    if not p.exists():
+        raise HTTPException(status_code=404, detail="not found")
+    # Restrict to actual charts — sloppak or loose song. Without this the route
+    # would stat ANY file the user happens to keep under DLC_DIR (e.g. notes),
+    # leaking its path/size; the app only recognises these two song formats.
+    is_pak = sloppak_mod.is_sloppak(p)
+    is_loose = loosefolder_mod.is_loose_song(p)
+    if not (is_pak or is_loose):
+        raise HTTPException(status_code=404, detail="not a chart")
+    st = p.stat()
+    info = {
+        "filename": filename,
+        "path": str(p),
+        "folder": str(p.parent),
+        "format": "sloppak" if is_pak else "loose",
+        # Directory-form songs report the tree's total (covers loose folders
+        # and dir-form paks); zip-form paks report the archive size. Symlinked
+        # entries are skipped so a link inside the folder can't pull in — or
+        # leak the size of — a file outside it.
+        "size": (st.st_size if p.is_file()
+                 else sum(f.stat().st_size for f in p.rglob("*")
+                          if f.is_file() and not f.is_symlink())),
+        "mtime": st.st_mtime,
+    }
+    if is_pak:
+        try:
+            m = sloppak_mod.load_manifest(p) or {}
+        except Exception:
+            m = {}
+        arrs = [str(a.get("name", a.get("id", ""))) for a in (m.get("arrangements") or [])
+                if isinstance(a, dict)]
+        stems = [str(s.get("id", "")) for s in (m.get("stems") or []) if isinstance(s, dict)]
+        try:
+            has_cover = sloppak_mod.read_cover_bytes(p, m) is not None
+        except Exception:
+            has_cover = False
+        # The optional identity/catalog keys, listed only when present — the
+        # Get-info panel's "what this pack carries vs what's missing" readout.
+        identity = {k: m.get(k) for k in
+                    ("mbid", "isrc", "genres", "track", "disc", "album_artist",
+                     "feedpak_version", "language")
+                    if m.get(k) not in (None, "", [])}
+        info["manifest"] = {
+            "title": str(m.get("title", "")), "artist": str(m.get("artist", "")),
+            "album": str(m.get("album", "")), "year": str(m.get("year", "") or ""),
+            "arrangements": arrs, "stems": stems,
+            "has_cover": has_cover, "has_lyrics": bool(m.get("lyrics")),
+            "authors": [a.get("name", "") if isinstance(a, dict) else str(a)
+                        for a in (m.get("authors") or [])],
+            "identity": identity,
+        }
+    # The enrichment verdict, so Get info can say "Matched (auto, 96%)" /
+    # "Pinned by you" / "Not matched" alongside the file facts.
+    row = appstate.meta_db.get_enrichment(filename)
+    if row:
+        info["match"] = {k: row.get(k) for k in
+                         ("match_state", "match_source", "match_score",
+                          "canon_artist", "canon_title", "canon_album", "canon_year")}
+    return info

lib/routers/loops.py

@@ -15,10 +15,15 @@ router = APIRouter()
 
 @router.get("/api/loops")
 def list_loops(filename: str):
-    rows = appstate.meta_db.conn.execute(
-        "SELECT id, name, start_time, end_time FROM loops WHERE filename = ? ORDER BY start_time",
-        (filename,)
-    ).fetchall()
+    # Hold the DB lock for the read: the shared single connection
+    # (check_same_thread=False) is serialized through meta_db._lock by every
+    # writer, so an unlocked SELECT here can overlap a POST/DELETE commit.
+    db = appstate.meta_db
+    with db._lock:
+        rows = db.conn.execute(
+            "SELECT id, name, start_time, end_time FROM loops WHERE filename = ? ORDER BY start_time",
+            (filename,)
+        ).fetchall()
     return [{"id": r[0], "name": r[1], "start": r[2], "end": r[3]} for r in rows]
 
 
@@ -30,17 +35,20 @@ def save_loop(data: dict):
     end = data.get("end")
     if not filename or start is None or end is None:
         return {"error": "Missing fields"}
-    if not name:
-        count = appstate.meta_db.conn.execute(
-            "SELECT COUNT(*) FROM loops WHERE filename = ?", (filename,)
-        ).fetchone()[0]
-        name = f"Loop {count + 1}"
-    with appstate.meta_db._lock:
-        appstate.meta_db.conn.execute(
+    db = appstate.meta_db
+    with db._lock:
+        # COUNT + INSERT under one lock so two unnamed POSTs can't read the same
+        # count and both mint "Loop N" (the count is only used to name the row).
+        if not name:
+            count = db.conn.execute(
+                "SELECT COUNT(*) FROM loops WHERE filename = ?", (filename,)
+            ).fetchone()[0]
+            name = f"Loop {count + 1}"
+        db.conn.execute(
             "INSERT INTO loops (filename, name, start_time, end_time) VALUES (?, ?, ?, ?)",
             (filename, name, float(start), float(end))
         )
-        appstate.meta_db.conn.commit()
+        db.conn.commit()
     return {"ok": True, "name": name}
 
 

lib/routers/playlists.py

@@ -0,0 +1,267 @@
+"""Playlists + custom playlist covers (fee[dB]ack v0.3.0).
+
+Extracted verbatim from ``server.py`` (R3). Edits: ``@app`` -> ``@router``,
+``meta_db`` -> ``appstate.meta_db``, ``CONFIG_DIR`` -> ``appstate.config_dir``
+(both read at call time through the seam), and ``_clean_str`` now imports from
+``reqfields``. See ``appstate.py``.
+"""
+
+import logging
+import os
+import tempfile
+from pathlib import Path
+
+from fastapi import APIRouter
+from fastapi.responses import FileResponse, JSONResponse
+
+import appstate
+from reqfields import _clean_str
+
+log = logging.getLogger("feedBack.server")
+
+router = APIRouter()
+
+# Cache policy for the custom-cover file response: revalidate every time so a
+# replaced cover is never served stale (pairs with the mtime-ns URL token).
+_ART_CACHE_HEADERS = {"Cache-Control": "no-cache"}
+
+
+def _playlist_cover_path(pid) -> Path | None:
+    """Filesystem path of a playlist's optional custom cover image (PNG),
+    stored under CONFIG_DIR. Returns None for a non-integer id."""
+    try:
+        pid = int(pid)
+    except (TypeError, ValueError):
+        return None
+    return appstate.config_dir / "playlist_covers" / f"{pid}.png"
+
+
+def _playlist_cover_url(pid) -> str | None:
+    cover = _playlist_cover_path(pid)
+    if not cover or not cover.exists():
+        return None
+    try:
+        # Nanosecond mtime so a same-second replace/remove/re-upload still
+        # changes the cache-bust token (int seconds could collide → stale image).
+        mt = cover.stat().st_mtime_ns
+    except OSError:
+        mt = 0
+    return f"/api/playlists/{pid}/cover?v={mt}"
+
+
+@router.get("/api/playlists")
+def api_list_playlists():
+    lists = appstate.meta_db.list_playlists()
+    for pl in lists:
+        pl["cover_url"] = _playlist_cover_url(pl["id"])
+    return lists
+
+
+@router.post("/api/playlists")
+def api_create_playlist(data: dict):
+    name = _clean_str(data.get("name"))
+    if not (1 <= len(name) <= 100):
+        return JSONResponse({"error": "Playlist name must be 1–100 characters."}, status_code=400)
+    # kind='album' = a curated album (§7.2): hand-picked works, a chosen chart
+    # per slot, played front-to-back on the queue. Absent/None = a regular mix.
+    kind = _clean_str(data.get("kind")) or None
+    if kind not in (None, "album"):
+        return JSONResponse({"error": "kind must be 'album' or omitted"}, status_code=400)
+    return appstate.meta_db.create_playlist(name, kind=kind)
+
+
+@router.get("/api/playlists/{pid}")
+def api_get_playlist(pid: int):
+    pl = appstate.meta_db.get_playlist(pid)
+    if pl is None:
+        return JSONResponse({"error": "not found"}, status_code=404)
+    pl["cover_url"] = _playlist_cover_url(pid)
+    return pl
+
+
+@router.patch("/api/playlists/{pid}")
+def api_rename_playlist(pid: int, data: dict):
+    pl = appstate.meta_db.get_playlist(pid)
+    if pl is None:
+        return JSONResponse({"error": "not found"}, status_code=404)
+    if pl["system_key"]:
+        return JSONResponse({"error": "System playlists cannot be renamed."}, status_code=400)
+    name = _clean_str(data.get("name"))
+    if not (1 <= len(name) <= 100):
+        return JSONResponse({"error": "Playlist name must be 1–100 characters."}, status_code=400)
+    appstate.meta_db.rename_playlist(pid, name)
+    return appstate.meta_db.get_playlist(pid)
+
+
+@router.delete("/api/playlists/{pid}")
+def api_delete_playlist(pid: int):
+    pl = appstate.meta_db.get_playlist(pid)
+    if pl is None:
+        return JSONResponse({"error": "not found"}, status_code=404)
+    if pl["system_key"]:
+        return JSONResponse({"error": "System playlists cannot be deleted."}, status_code=400)
+    if not appstate.meta_db.delete_playlist(pid):   # vanished under us (concurrent delete)
+        return JSONResponse({"error": "not found"}, status_code=404)
+    cover = _playlist_cover_path(pid)       # drop any custom cover with the playlist
+    if cover and cover.exists():
+        try:
+            cover.unlink()
+        except OSError:
+            pass
+    return {"ok": True}
+
+
+@router.post("/api/playlists/{pid}/songs")
+def api_add_playlist_song(pid: int, data: dict):
+    if appstate.meta_db.get_playlist(pid) is None:
+        return JSONResponse({"error": "not found"}, status_code=404)
+    filename = _clean_str(data.get("filename"))
+    if not filename:
+        return JSONResponse({"error": "filename required"}, status_code=400)
+    if appstate.meta_db.add_playlist_song(pid, filename) is None:   # playlist vanished under us
+        return JSONResponse({"error": "not found"}, status_code=404)
+    pl = appstate.meta_db.get_playlist(pid)
+    return pl if pl is not None else JSONResponse({"error": "not found"}, status_code=404)
+
+
+@router.patch("/api/playlists/{pid}/songs/{filename:path}")
+def api_update_playlist_slot(pid: int, filename: str, data: dict):
+    """Edit one curated-album slot: {"arrangement": name|null} pins/clears the
+    slot's arrangement; {"chart_filename": fn} swaps the slot to another chart
+    of the same work (position + pin kept). Albums only — a mix has no slots."""
+    pl = appstate.meta_db.get_playlist(pid)
+    if pl is None:
+        return JSONResponse({"error": "not found"}, status_code=404)
+    if pl.get("kind") != "album":
+        return JSONResponse({"error": "Slot editing is for albums."}, status_code=400)
+    kwargs = {}
+    if "chart_filename" in data:
+        new_fn = _clean_str(data.get("chart_filename"))
+        if not new_fn:
+            return JSONResponse({"error": "chart_filename must be a filename"}, status_code=400)
+        kwargs["new_filename"] = new_fn
+    if "arrangement" in data:
+        arr = data.get("arrangement")
+        if arr is not None and not (isinstance(arr, str) and 1 <= len(arr.strip()) <= 100):
+            return JSONResponse({"error": "arrangement must be a name or null"}, status_code=400)
+        kwargs["arrangement"] = arr.strip() if isinstance(arr, str) else None
+    if not kwargs:
+        return JSONResponse({"error": "nothing to update"}, status_code=400)
+    if appstate.meta_db.update_playlist_slot(pid, filename, **kwargs) is None:
+        return JSONResponse(
+            {"error": "no such slot, or the chart isn't a version of this song"},
+            status_code=400)
+    return appstate.meta_db.get_playlist(pid)
+
+
+@router.delete("/api/playlists/{pid}/songs/{filename:path}")
+def api_remove_playlist_song(pid: int, filename: str):
+    if appstate.meta_db.get_playlist(pid) is None:
+        return JSONResponse({"error": "not found"}, status_code=404)
+    appstate.meta_db.remove_playlist_song(pid, filename)
+    pl = appstate.meta_db.get_playlist(pid)
+    return pl if pl is not None else JSONResponse({"error": "not found"}, status_code=404)
+
+
+@router.post("/api/playlists/{pid}/reorder")
+def api_reorder_playlist(pid: int, data: dict):
+    pl = appstate.meta_db.get_playlist(pid)
+    if pl is None:
+        return JSONResponse({"error": "not found"}, status_code=404)
+    order = data.get("order")
+    if not isinstance(order, list) or not all(isinstance(f, str) for f in order):
+        return JSONResponse({"error": "order must be a list of filenames"}, status_code=400)
+    # Require an exact permutation of the playlist's current songs: a list with
+    # duplicates, omissions, or extras would otherwise produce duplicate
+    # positions / a partial reorder while still returning 200.
+    current = [s["filename"] for s in pl["songs"]]
+    if len(order) != len(current) or sorted(order) != sorted(current):
+        return JSONResponse(
+            {"error": "order must be a permutation of the playlist's current songs"},
+            status_code=400,
+        )
+    appstate.meta_db.reorder_playlist(pid, order)
+    return appstate.meta_db.get_playlist(pid)
+
+
+@router.post("/api/playlists/{pid}/cover")
+async def api_set_playlist_cover(pid: int, data: dict):
+    """Set a playlist's custom cover from a base64 / data-URL image (PNG/JPG).
+    Overrides the content-dependent (song-art) cover. Stored as a small PNG
+    thumbnail under CONFIG_DIR/playlist_covers/."""
+    if appstate.meta_db.get_playlist(pid) is None:
+        return JSONResponse({"error": "not found"}, status_code=404)
+    import base64
+    import io
+    b64 = data.get("image", "")
+    # Guard the type before the `","` membership test — a non-string image
+    # (e.g. {"image": 123} / null) would otherwise raise TypeError → 500.
+    # Mirrors the avatar/song-art upload guard.
+    if not isinstance(b64, str) or not b64:
+        return JSONResponse({"error": "No image data"}, status_code=400)
+    if "," in b64:
+        b64 = b64.split(",", 1)[1]
+    if not b64:
+        return JSONResponse({"error": "No image data"}, status_code=400)
+    try:
+        img_data = base64.b64decode(b64)
+    except Exception:
+        return JSONResponse({"error": "Invalid base64"}, status_code=400)
+    cover = _playlist_cover_path(pid)
+    cover.parent.mkdir(parents=True, exist_ok=True)
+    # Decode/validate the image — a bad payload is a CLIENT error (400), and the
+    # message stays generic so it can't echo internals.
+    try:
+        from PIL import Image
+        img = Image.open(io.BytesIO(img_data)).convert("RGB")
+        img.thumbnail((640, 640))                  # covers stay small
+    except Exception:
+        return JSONResponse({"error": "Invalid image"}, status_code=400)
+    # Persist. A save/replace failure is a SERVER error (500, logged, no
+    # filesystem detail leaked) — the pre-split handler mislabeled these as 400
+    # and echoed the exception. A unique temp name in the cover dir (not a shared
+    # `{pid}.png.tmp`) means two concurrent uploads can't clobber each other's
+    # temp file; the atomic replace publishes. Re-check the playlist still exists
+    # just before publishing so a delete that raced the decode above can't leave
+    # an orphan cover — cheap belt-and-braces; FeedBack is single-user
+    # (Principle I), so a full per-playlist lock would be for a race the
+    # deployment model precludes.
+    tmp = None
+    try:
+        # mkstemp is inside the try too: an unwritable dir / full disk raises
+        # here, and that's the same class of persistence failure as save/replace.
+        fd, tmp_name = tempfile.mkstemp(prefix=f".{pid}.", suffix=".png.tmp", dir=str(cover.parent))
+        tmp = Path(tmp_name)
+        with os.fdopen(fd, "wb") as f:
+            img.save(f, "PNG")
+        if appstate.meta_db.get_playlist(pid) is None:
+            tmp.unlink(missing_ok=True)
+            return JSONResponse({"error": "not found"}, status_code=404)
+        tmp.replace(cover)
+    except Exception:
+        if tmp is not None:
+            tmp.unlink(missing_ok=True)
+        log.exception("playlist cover save failed (pid=%s)", pid)
+        return JSONResponse({"error": "could not save cover"}, status_code=500)
+    return {"ok": True, "cover_url": _playlist_cover_url(pid)}
+
+
+@router.get("/api/playlists/{pid}/cover")
+def api_get_playlist_cover(pid: int):
+    cover = _playlist_cover_path(pid)
+    if not cover or not cover.exists():
+        return JSONResponse({"error": "not found"}, status_code=404)
+    # no-cache (revalidate) like song art, so a replaced cover is never served
+    # stale — pairs with the mtime-ns cache-bust token on the URL.
+    return FileResponse(str(cover), media_type="image/png", headers=_ART_CACHE_HEADERS)
+
+
+@router.delete("/api/playlists/{pid}/cover")
+def api_delete_playlist_cover(pid: int):
+    cover = _playlist_cover_path(pid)
+    if cover and cover.exists():
+        try:
+            cover.unlink()
+        except OSError:
+            pass
+    return {"ok": True}

lib/routers/wanted.py

@@ -0,0 +1,45 @@
+"""Wishlist / "wanted" API (feedBack#636) — songs the user wants but doesn't own.
+
+Extracted verbatim from ``server.py`` (R3); edits: ``@app`` -> ``@router``,
+``meta_db`` -> ``appstate.meta_db``, ``_clean_str`` from ``reqfields``.
+"""
+
+from fastapi import APIRouter
+from fastapi.responses import JSONResponse
+
+import appstate
+from reqfields import _clean_str
+
+router = APIRouter()
+
+
+@router.get("/api/wanted")
+def api_list_wanted():
+    """The wishlist — songs the user wants but doesn't own yet (newest first)."""
+    return {"wanted": appstate.meta_db.list_wanted()}
+
+
+@router.post("/api/wanted")
+def api_add_wanted(data: dict):
+    """Add a not-owned song to the wishlist. `artist`/`title` are required (at
+    least one non-empty); `source`/`source_ref`/`note` are optional. Idempotent
+    on identity so producers (find_more ownership-diff, manual add) can re-post."""
+    if not isinstance(data, dict):
+        return JSONResponse({"error": "body must be an object"}, status_code=400)
+    artist = _clean_str(data.get("artist"))
+    title = _clean_str(data.get("title"))
+    if not artist and not title:
+        return JSONResponse({"error": "artist or title required"}, status_code=400)
+    row = appstate.meta_db.add_wanted(
+        artist=artist, title=title,
+        source=_clean_str(data.get("source")) or "manual",
+        source_ref=_clean_str(data.get("source_ref")),
+        note=_clean_str(data.get("note")),
+    )
+    return {"ok": True, "wanted": row}
+
+
+@router.delete("/api/wanted/{wanted_id}")
+def api_remove_wanted(wanted_id: int):
+    """Remove a wishlist entry by id."""
+    return {"ok": appstate.meta_db.remove_wanted(wanted_id)}

tests/test_highway_ws_authors.py

@@ -16,6 +16,8 @@ import pytest
 import yaml
 from fastapi.testclient import TestClient
 
+from routers.ws_highway import _sanitize_authors
+
 
 # ── _sanitize_authors unit tests ────────────────────────────────────────────
 
@@ -35,7 +37,7 @@ def server_mod(monkeypatch, tmp_path):
 
 
 def test_sanitize_authors_valid(server_mod):
-    out = server_mod._sanitize_authors(
+    out = _sanitize_authors(
         {
             "authors": [
                 {"name": "Azure", "role": "charter", "email": "a@b.c", "url": "x"},
@@ -53,7 +55,7 @@ def test_sanitize_authors_valid(server_mod):
 
 
 def test_sanitize_authors_skips_malformed(server_mod):
-    out = server_mod._sanitize_authors(
+    out = _sanitize_authors(
         {
             "authors": [
                 {"name": ""},          # blank name → skipped
@@ -69,7 +71,7 @@ def test_sanitize_authors_skips_malformed(server_mod):
 
 @pytest.mark.parametrize("manifest", [None, {}, {"authors": None}, {"authors": "x"}, "nope"])
 def test_sanitize_authors_absent_or_nonlist(server_mod, manifest):
-    assert server_mod._sanitize_authors(manifest) == []
+    assert _sanitize_authors(manifest) == []
 
 
 # ── song_info WS integration ────────────────────────────────────────────────
@@ -118,6 +120,9 @@ def make_client(tmp_path, monkeypatch):
         monkeypatch.setattr(server, "load_plugins", lambda *a, **kw: None)
         monkeypatch.setattr(server, "startup_scan", lambda: None)
         monkeypatch.setattr(server, "SLOPPAK_CACHE_DIR", tmp_path / "cache")
+        # ws_highway reads the cache dir through the appstate seam now.
+        import appstate as _appstate
+        monkeypatch.setattr(_appstate, "sloppak_cache_dir", tmp_path / "cache")
         return server
 
     (tmp_path / "dlc").mkdir()

tests/test_highway_ws_instrument_routing.py

@@ -96,6 +96,9 @@ def make_client(tmp_path, monkeypatch):
         monkeypatch.setattr(server, "load_plugins", lambda *a, **kw: None)
         monkeypatch.setattr(server, "startup_scan", lambda: None)
         monkeypatch.setattr(server, "SLOPPAK_CACHE_DIR", tmp_path / "cache")
+        # ws_highway reads the cache dir through the appstate seam now.
+        import appstate as _appstate
+        monkeypatch.setattr(_appstate, "sloppak_cache_dir", tmp_path / "cache")
         return server
 
     (tmp_path / "dlc").mkdir()

tests/test_highway_ws_notation.py

@@ -123,6 +123,9 @@ def make_client(tmp_path, monkeypatch):
         monkeypatch.setattr(server, "load_plugins", lambda *a, **kw: None)
         monkeypatch.setattr(server, "startup_scan", lambda: None)
         monkeypatch.setattr(server, "SLOPPAK_CACHE_DIR", tmp_path / "cache")
+        # ws_highway reads the cache dir through the appstate seam now.
+        import appstate as _appstate
+        monkeypatch.setattr(_appstate, "sloppak_cache_dir", tmp_path / "cache")
         return server
 
     (tmp_path / "dlc").mkdir()

tests/test_loops_concurrency.py

@@ -0,0 +1,84 @@
+"""Concurrent unnamed loop saves must get unique names.
+
+`save_loop` auto-names an unnamed loop `Loop {count+1}`. If the `COUNT(*)` runs
+outside the DB lock (as it did before the fix), two simultaneous unnamed POSTs
+read the same count and both mint the same name. A `threading.Barrier` releases
+all workers into `save_loop` at once to force that interleave.
+
+Single-user app, so this race is unlikely in practice — but the fix is one lock
+scope, and the test pins it.
+"""
+
+import threading
+
+import pytest
+
+import appstate
+from metadata_db import MetadataDB
+from routers import loops
+
+
+@pytest.fixture()
+def meta_db(tmp_path):
+    prev = appstate.meta_db
+    db = MetadataDB(tmp_path)
+    appstate.configure(meta_db=db)
+    yield db
+    db.conn.close()
+    appstate.configure(meta_db=prev)
+
+
+def test_concurrent_unnamed_saves_get_unique_names(meta_db):
+    workers = 16
+    barrier = threading.Barrier(workers)
+    names, errors = [], []
+    lock = threading.Lock()
+
+    def save():
+        try:
+            barrier.wait()                       # release all at once
+            r = loops.save_loop({"filename": "song.feedpak", "start": 0.0, "end": 1.0})
+            with lock:
+                names.append(r["name"])
+        except Exception as e:                   # noqa: BLE001 — surface any thread error
+            with lock:
+                errors.append(e)
+
+    threads = [threading.Thread(target=save) for _ in range(workers)]
+    for t in threads:
+        t.start()
+    for t in threads:
+        t.join()
+
+    assert not errors, errors
+    assert len(names) == workers
+    # The load-bearing assertion: no two auto-named loops collide.
+    assert len(set(names)) == workers, f"duplicate loop names: {sorted(names)}"
+    # And the DB agrees — every insert landed.
+    stored = meta_db.conn.execute(
+        "SELECT COUNT(*) FROM loops WHERE filename = ?", ("song.feedpak",)
+    ).fetchone()[0]
+    assert stored == workers
+
+
+def test_list_and_save_do_not_error_under_interleave(meta_db):
+    """A read overlapping writes must not raise (shared connection, one lock)."""
+    stop = threading.Event()
+    errors = []
+
+    def reader():
+        while not stop.is_set():
+            try:
+                loops.list_loops("song.feedpak")
+            except Exception as e:               # noqa: BLE001
+                errors.append(e)
+
+    r = threading.Thread(target=reader)
+    r.start()
+    try:
+        for i in range(40):
+            loops.save_loop({"filename": "song.feedpak", "name": f"n{i}", "start": 0.0, "end": 1.0})
+    finally:
+        stop.set()
+        r.join()
+    assert not errors, errors

tests/test_playlist_cover_errors.py

@@ -0,0 +1,107 @@
+"""Playlist cover upload: client vs server errors, no temp litter, no leak.
+
+The pre-split handler caught decode AND persistence failures in one `except`,
+returned 400 for both, and echoed the exception (`Invalid image: {e}`) — so a
+disk/permission failure was mislabeled as a client error and could leak a
+filesystem path. These pin the split.
+"""
+
+import base64
+import io
+
+import pytest
+from fastapi.testclient import TestClient
+from PIL import Image
+
+import appstate
+from metadata_db import MetadataDB
+from routers import playlists
+
+
+@pytest.fixture()
+def client(tmp_path):
+    prev = (appstate.meta_db, appstate.config_dir)
+    db = MetadataDB(tmp_path)
+    appstate.configure(meta_db=db, config_dir=tmp_path)
+    app_ = __import__("fastapi").FastAPI()
+    app_.include_router(playlists.router)
+    try:
+        yield TestClient(app_), tmp_path
+    finally:
+        db.conn.close()
+        appstate.configure(meta_db=prev[0], config_dir=prev[1])
+
+
+def _png_b64():
+    buf = io.BytesIO()
+    Image.new("RGB", (8, 8), (10, 20, 30)).save(buf, "PNG")
+    return base64.b64encode(buf.getvalue()).decode()
+
+
+def _make_playlist(client):
+    return client.post("/api/playlists", json={"name": "P"}).json()["id"]
+
+
+def test_valid_cover_saves_and_leaves_no_temp(client):
+    c, tmp_path = client
+    pid = _make_playlist(c)
+    r = c.post(f"/api/playlists/{pid}/cover", json={"image": _png_b64()})
+    assert r.status_code == 200
+    cover_dir = tmp_path / "playlist_covers"
+    assert (cover_dir / f"{pid}.png").exists()
+    # The atomic-publish temp file must not linger.
+    assert not list(cover_dir.glob("*.tmp"))
+
+
+def test_undecodable_image_is_a_400_without_leaking(client):
+    c, _ = client
+    pid = _make_playlist(c)
+    # valid base64, not a valid image
+    r = c.post(f"/api/playlists/{pid}/cover", json={"image": base64.b64encode(b"not an image").decode()})
+    assert r.status_code == 400
+    body = r.json()["error"]
+    assert body == "Invalid image"          # generic — no exception detail echoed
+    assert "playlist_covers" not in body    # no filesystem path leak
+
+
+def test_save_failure_is_a_500_not_a_400(client, monkeypatch):
+    """A persistence failure (here: Image.save raising) must be a logged 500,
+    not a 400 — the whole point of the decode/persist split. Negative-checks
+    against the pre-fix behavior, which returned 400 for exactly this."""
+    c, tmp_path = client
+    pid = _make_playlist(c)
+    payload = _png_b64()                     # build BEFORE patching save
+
+    def boom(self, fp, *a, **k):
+        raise OSError("disk full")
+
+    monkeypatch.setattr(Image.Image, "save", boom)
+    r = c.post(f"/api/playlists/{pid}/cover", json={"image": payload})
+
+    assert r.status_code == 500
+    assert "disk full" not in r.json()["error"]           # no internal detail
+    assert not list((tmp_path / "playlist_covers").glob("*.tmp"))  # temp cleaned up
+
+
+def test_temp_creation_failure_is_a_500(client, monkeypatch):
+    """mkstemp raising (unwritable dir / full disk) must hit the same logged 500
+    path as a save failure, not escape as an unhandled server error."""
+    import tempfile as _tempfile
+
+    c, _ = client
+    pid = _make_playlist(c)
+    payload = _png_b64()
+
+    def boom(*a, **k):
+        raise OSError("read-only file system")
+
+    monkeypatch.setattr(_tempfile, "mkstemp", boom)
+    r = c.post(f"/api/playlists/{pid}/cover", json={"image": payload})
+    assert r.status_code == 500
+    assert "read-only" not in r.json()["error"]
+
+
+def test_upload_to_missing_playlist_is_404(client):
+    c, _ = client
+    r = c.post("/api/playlists/9999/cover", json={"image": _png_b64()})
+    assert r.status_code == 404

tests/test_playlists_api.py

@@ -6,6 +6,10 @@ import sys
 import pytest
 from fastapi.testclient import TestClient
 
+# Moved to routers/playlists in R3; reads appstate.config_dir, which the
+# `server` fixture configures via CONFIG_DIR before this is called.
+from routers.playlists import _playlist_cover_path
+
 
 @pytest.fixture()
 def server(tmp_path, monkeypatch, isolate_logging):
@@ -168,6 +172,6 @@ def test_cover_rejects_non_string_image_with_400_not_500(client):
 def test_deleting_playlist_removes_custom_cover(client, server):
     pid = client.post("/api/playlists", json={"name": "Doomed"}).json()["id"]
     client.post(f"/api/playlists/{pid}/cover", json={"image": _png_b64()})
-    assert server._playlist_cover_path(pid).exists()
+    assert _playlist_cover_path(pid).exists()
     client.delete(f"/api/playlists/{pid}")
-    assert not server._playlist_cover_path(pid).exists()
+    assert not _playlist_cover_path(pid).exists()

tests/test_ws_highway_disconnect.py

@@ -0,0 +1,110 @@
+"""A client that disconnects mid-stream is routine, not a logged error.
+
+The highway WS streams ~15 message batches before its keep-alive loop. A
+disconnect during that streaming used to fall through to the blanket
+`except Exception` and log `highway_ws unhandled error`. The dedicated
+`except WebSocketDisconnect: return` makes it quiet.
+"""
+
+import asyncio
+import importlib
+import json
+import logging
+import sys
+
+import pytest
+import yaml
+from fastapi import WebSocketDisconnect
+
+
+def _write_sloppak(dlc_root):
+    pak = dlc_root / "disc.sloppak"
+    pak.mkdir(parents=True)
+    (pak / "arrangements").mkdir()
+    (pak / "arrangements" / "lead.json").write_text(json.dumps({
+        "notes": [], "chords": [], "anchors": [], "handshapes": [],
+        "templates": [], "beats": [{"time": 0.0, "measure": 1}],
+        "sections": [{"name": "intro", "number": 1, "time": 0.0}],
+    }))
+    (pak / "manifest.yaml").write_text(yaml.safe_dump({
+        "title": "Disc", "artist": "T", "album": "", "year": 2026, "duration": 10.0,
+        "arrangements": [{"id": "lead", "name": "Lead", "file": "arrangements/lead.json"}],
+        "stems": [],
+    }, sort_keys=False))
+    return pak
+
+
+class _DisconnectingWS:
+    """Accepts, then raises WebSocketDisconnect on the first streamed send —
+    i.e. a client that drops mid-stream."""
+    def __init__(self):
+        self.sends = 0
+
+    async def accept(self):
+        pass
+
+    async def send_json(self, data):
+        self.sends += 1
+        # Raise only on the FIRST send. If the handler wrongly kept streaming
+        # after catching the disconnect, later sends would succeed and `sends`
+        # would climb past 1 — the exactly-one assertion catches that.
+        if self.sends == 1:
+            raise WebSocketDisconnect(code=1001)
+
+    async def receive_text(self):
+        raise WebSocketDisconnect(code=1001)
+
+    async def close(self):
+        pass
+
+
+@pytest.fixture()
+def server(tmp_path, monkeypatch):
+    (tmp_path / "dlc").mkdir()
+    monkeypatch.setenv("CONFIG_DIR", str(tmp_path / "config"))
+    monkeypatch.setenv("DLC_DIR", str(tmp_path / "dlc"))
+    monkeypatch.setenv("FEEDBACK_SKIP_STARTUP_TASKS", "1")
+    sys.modules.pop("server", None)
+    mod = importlib.import_module("server")
+    yield mod, tmp_path / "dlc"
+    conn = getattr(getattr(mod, "meta_db", None), "conn", None)
+    if conn is not None:
+        getattr(mod, "_join_background_db_threads", lambda: None)()
+        conn.close()
+    sys.modules.pop("server", None)
+
+
+class _Capture(logging.Handler):
+    def __init__(self):
+        super().__init__()
+        self.messages = []
+
+    def emit(self, record):
+        self.messages.append(record.getMessage())
+
+
+def test_midstream_disconnect_is_not_logged_as_error(server):
+    """The first streamed send (`loading`) raises WebSocketDisconnect; the
+    handler must return quietly, not log `highway_ws unhandled error`.
+
+    A raw handler on `feedBack.server` is used rather than pytest's `caplog`
+    because `configure_logging()` (run at server import) reroutes that logger
+    through the structlog pipeline, which caplog's fixture doesn't observe.
+    """
+    _server, dlc = server
+    _write_sloppak(dlc)
+    from routers.ws_highway import highway_ws
+
+    cap = _Capture()
+    cap.setLevel(logging.ERROR)
+    lg = logging.getLogger("feedBack.server")
+    lg.addHandler(cap)
+    try:
+        ws = _DisconnectingWS()
+        asyncio.run(highway_ws(ws, "disc.sloppak", arrangement=0))  # must not raise
+    finally:
+        lg.removeHandler(cap)
+
+    assert ws.sends == 1, f"handler kept streaming after the disconnect ({ws.sends} sends)"
+    unhandled = [m for m in cap.messages if "highway_ws unhandled error" in m]
+    assert not unhandled, f"disconnect logged as unhandled error: {unhandled}"