From 59c369c9e2aab398614ffcc322d5d4e53a4c31a6 Mon Sep 17 00:00:00 2001 From: andrew Date: Thu, 9 Jul 2026 11:13:58 -0600 Subject: [PATCH] Fix playlist status page, dedup delete-by-path bug, duplicate scan rows; add public IP dashboard card Playlist detail page's status section (and the new two-column compare view) was never rendering: the router awaited status_service.playlist_status(), but that function is plain sync, so awaiting its dict return raised "object dict can't be used in 'await' expression" on every load, silently caught and shown as a generic fetch error. Removed the erroneous await. find-fuzzy-dupes.py's --apply and --apply-pairs both deleted by a `path::` regex query even though the beets id was already available in scope -- same mixed-path-storage issue (pre/post beets-2.11-upgrade items store absolute vs. library-relative paths) dedup-library.sh already worked around by deleting via id instead. This silently failed to match for most confirmed fuzzy-audio deletions (verified: 19 of 20 in one run). Added a delete_id column to dedup_candidates, threaded through the scan/apply pipeline, and switched both delete call sites to `id:`. Dedup scans also never checked whether a pair was already sitting in the pending list, so every re-scan (including the daily schedule) added a new row for the same unreviewed duplicate -- cleaned up 38 redundant rows already in production and added a check so future scans skip a pair that's already pending. Dashboard gets a Public IP stat card (cached 10 min, fetched via ipify) as a quick confidence check that outbound traffic is actually routed through gluetun's VPN and not the home connection. --- app/models.py | 1 + app/routers/dashboard.py | 5 ++- app/routers/playlists.py | 2 +- app/services/dedup_review_service.py | 34 ++++++++++++++++++-- app/services/network_service.py | 31 +++++++++++++++++++ app/templates/dashboard.html | 10 ++++++ pipeline/lib/find-fuzzy-dupes.py | 46 ++++++++++++++++++---------- schema/0003_dedup_delete_id.sql | 10 ++++++ 8 files changed, 118 insertions(+), 21 deletions(-) create mode 100644 app/services/network_service.py create mode 100644 schema/0003_dedup_delete_id.sql diff --git a/app/models.py b/app/models.py index dc337e6..ad29dab 100644 --- a/app/models.py +++ b/app/models.py @@ -110,6 +110,7 @@ class DedupCandidate(Base): pass_name = Column(String, nullable=False) keep_path = Column(String, nullable=False) delete_path = Column(String, nullable=False) + delete_id = Column(Integer, nullable=True) delete_size_bytes = Column(Integer, nullable=True) confirmed = Column(Boolean, nullable=False, default=False) confirmed_by = Column(String, nullable=True) diff --git a/app/routers/dashboard.py b/app/routers/dashboard.py index 71dcf83..e2a202d 100644 --- a/app/routers/dashboard.py +++ b/app/routers/dashboard.py @@ -1,3 +1,4 @@ +import asyncio import time from fastapi import APIRouter, Depends, Request @@ -7,7 +8,7 @@ from sqlalchemy import select from app.db import get_db from app.models import JobRun from app.security.deps import require_auth -from app.services import beets_service, credential_service, playlist_service, scheduler_service +from app.services import beets_service, credential_service, network_service, playlist_service, scheduler_service router = APIRouter(tags=["dashboard"]) templates = Jinja2Templates(directory="app/templates") @@ -32,6 +33,7 @@ async def dashboard(request: Request, user: dict = Depends(require_auth), db=Dep db.execute(select(JobRun).order_by(JobRun.started_at.desc()).limit(10)).scalars() ) recent_tracks = beets_service.recently_added(time.time() - _RECENT_TRACKS_WINDOW_SECONDS) + public_ip = await asyncio.to_thread(network_service.public_ip) return templates.TemplateResponse( request, "dashboard.html", @@ -45,5 +47,6 @@ async def dashboard(request: Request, user: dict = Depends(require_auth), db=Dep "recent_tracks": recent_tracks, "auth_states": credential_service.auth_states(db), "humanize_job_key": scheduler_service.humanize_job_key, + "public_ip": public_ip, }, ) diff --git a/app/routers/playlists.py b/app/routers/playlists.py index 254aae8..e3472fa 100644 --- a/app/routers/playlists.py +++ b/app/routers/playlists.py @@ -49,7 +49,7 @@ async def playlist_detail( status = None error = None try: - status = await status_service.playlist_status(db, playlist.name, playlist.spotify_url) + status = status_service.playlist_status(db, playlist.name, playlist.spotify_url) except Exception as exc: error = str(exc) diff --git a/app/services/dedup_review_service.py b/app/services/dedup_review_service.py index 54ae360..381f1ef 100644 --- a/app/services/dedup_review_service.py +++ b/app/services/dedup_review_service.py @@ -41,6 +41,22 @@ def _is_pair_ignored(db, path_a: str, path_b: str) -> bool: return db.execute(query).first() is not None +def _is_pair_already_pending(db, keep_path: str, delete_path: str) -> bool: + """True if this exact keep/delete pair is already sitting in the + pending list from an earlier scan. Without this, every re-scan of a + duplicate the user hasn't reviewed yet (e.g. the daily scheduled scan) + added a brand-new row for the same pair, so it piled up multiple + identical entries in the table instead of just staying as one.""" + query = select(DedupCandidate.id).where( + DedupCandidate.applied == False, # noqa: E712 + DedupCandidate.confirmed == False, # noqa: E712 + DedupCandidate.ignored == False, # noqa: E712 + DedupCandidate.keep_path == keep_path, + DedupCandidate.delete_path == delete_path, + ).limit(1) + return db.execute(query).first() is not None + + def _parse_json_lines(output: str) -> list[dict]: candidates = [] for line in output.splitlines(): @@ -77,23 +93,34 @@ async def _run_scan(job_key: str, script_name: str, triggered_by: str) -> DedupR db.refresh(dedup_run) skipped_ignored = 0 + skipped_duplicate = 0 for c in candidates: if _is_pair_ignored(db, c["keep_path"], c["delete_path"]): skipped_ignored += 1 continue + if _is_pair_already_pending(db, c["keep_path"], c["delete_path"]): + skipped_duplicate += 1 + continue db.add( DedupCandidate( dedup_run_id=dedup_run.id, pass_name=c.get("pass", "unknown"), keep_path=c["keep_path"], delete_path=c["delete_path"], + delete_id=c.get("delete_id"), delete_size_bytes=c.get("delete_size_bytes"), ) ) + # Flush (not commit) so a duplicate pair emitted twice within + # this same scan's own output -- e.g. two passes agreeing on + # the same file -- is caught by the next iteration's check too, + # not just duplicates from a previous scan's committed rows. + db.flush() db.commit() db.refresh(dedup_run) - if skipped_ignored: - dedup_run.kept = (dedup_run.kept or 0) + skipped_ignored + skipped_total = skipped_ignored + skipped_duplicate + if skipped_total: + dedup_run.kept = (dedup_run.kept or 0) + skipped_total db.commit() db.refresh(dedup_run) return dedup_run @@ -144,7 +171,8 @@ def _write_apply_args(script_name: str, script: str, group: list[DedupCandidate] pairs_file.parent.mkdir(parents=True, exist_ok=True) pairs_file.write_text( "\n".join( - json.dumps({"keep_path": c.keep_path, "delete_path": c.delete_path}) for c in group + json.dumps({"keep_path": c.keep_path, "delete_path": c.delete_path, "delete_id": c.delete_id}) + for c in group ) + "\n" ) diff --git a/app/services/network_service.py b/app/services/network_service.py new file mode 100644 index 0000000..eeea51b --- /dev/null +++ b/app/services/network_service.py @@ -0,0 +1,31 @@ +import time + +import httpx + +# alembic rides gluetun's network namespace (network_mode: service:gluetun) +# specifically so sldl's Soulseek traffic stays VPN-routed -- this is the +# dashboard's confidence check that it's actually working: the container's +# own outbound IP should be the VPN's, not the home connection's. Cached +# in-process (no DB row, no job history) since it's a lightweight external +# check, not something worth tracking over time. +_CACHE_TTL_SECONDS = 600 +_cache: dict = {"ip": None, "checked_at": 0.0, "error": None} + + +def public_ip() -> dict: + now = time.time() + if _cache["ip"] is not None and now - _cache["checked_at"] < _CACHE_TTL_SECONDS: + return dict(_cache) + + try: + resp = httpx.get("https://api.ipify.org", timeout=5.0) + resp.raise_for_status() + _cache["ip"] = resp.text.strip() + _cache["error"] = None + except Exception as exc: + _cache["error"] = str(exc) + # Keep the last-known-good ip (if any) rather than blanking it -- + # a transient fetch failure shouldn't make the dashboard look like + # the VPN dropped when it's actually just ipify being slow. + _cache["checked_at"] = now + return dict(_cache) diff --git a/app/templates/dashboard.html b/app/templates/dashboard.html index 3400c21..98d0caa 100644 --- a/app/templates/dashboard.html +++ b/app/templates/dashboard.html @@ -28,6 +28,16 @@
{{ total_size_human }}
Approx. library size
+
+
+ {% if public_ip.ip %}{{ public_ip.ip }}{% else %}unknown{% endif %} +
+
+ + Public IP (via gluetun) +
+

File formats

diff --git a/pipeline/lib/find-fuzzy-dupes.py b/pipeline/lib/find-fuzzy-dupes.py index 7c499fd..256559a 100755 --- a/pipeline/lib/find-fuzzy-dupes.py +++ b/pipeline/lib/find-fuzzy-dupes.py @@ -136,16 +136,20 @@ class UnionFind: def _apply_pairs(pairs_file: str, emit_json: bool) -> int: """Apply already-confirmed keep/delete pairs (JSON lines: {"keep_path": - ..., "delete_path": ...}) without re-scanning the library for - duplicates. A full rescan recomputes Chromaprint similarity for every - pair in the library (10+ minutes on a ~4k track library, longer - whenever the scan cache is cold e.g. right after fingerprints.db gets - rewritten) -- wildly disproportionate for applying a decision a human - already reviewed. The one thing that actually needs re-checking here - is whether the keep/delete ranking flipped since confirmation (e.g. the - delete_path got upgraded to FLAC in the meantime); that's a cheap, - local, filesystem-only check via rank_file(), no fingerprinting - involved.""" + ..., "delete_path": ..., "delete_id": ...}) without re-scanning the + library for duplicates. A full rescan recomputes Chromaprint similarity + for every pair in the library (10+ minutes on a ~4k track library, + longer whenever the scan cache is cold e.g. right after fingerprints.db + gets rewritten) -- wildly disproportionate for applying a decision a + human already reviewed. The one thing that actually needs re-checking + here is whether the keep/delete ranking flipped since confirmation + (e.g. the delete_path got upgraded to FLAC in the meantime); that's a + cheap, local, filesystem-only check via rank_file(), no fingerprinting + involved. + + Deletes by beets id, not path -- see the id-vs-path comment on the + --apply loop in main() below for why a path:: query silently fails to + match for a large fraction of this library.""" pairs = [] with open(pairs_file) as f: for line in f: @@ -156,7 +160,7 @@ def _apply_pairs(pairs_file: str, emit_json: bool) -> int: print(f"[fuzzy-dupes] applying {len(pairs)} pre-confirmed pair(s), no rescan") deleted = failed = skipped = 0 for pair in pairs: - keep_path, delete_path = pair["keep_path"], pair["delete_path"] + keep_path, delete_path, delete_id = pair["keep_path"], pair["delete_path"], pair.get("delete_id") if not os.path.exists(delete_path): print(f" SKIP (already gone) {delete_path}") skipped += 1 @@ -165,9 +169,14 @@ def _apply_pairs(pairs_file: str, emit_json: bool) -> int: print(f" SKIP (ranking flipped or keep_path missing since confirm) {delete_path}") skipped += 1 continue - escaped = re.escape(delete_path) + if delete_id is not None: + query = f"id:{delete_id}" + else: + # Pre-existing candidate confirmed before delete_id started being + # stored -- fall back to the old (less reliable) path query. + query = f"path::{re.escape(delete_path)}" result = subprocess.run( - ["beet", "remove", "-d", "-f", f"path::{escaped}"], + ["beet", "remove", "-d", "-f", query], capture_output=True, text=True, ) if result.returncode != 0: @@ -331,6 +340,7 @@ def main() -> int: "pass": "fuzzy_audio", "keep_path": keeper_path, "delete_path": loser_path, + "delete_id": beets_id, "delete_size_bytes": loser_size, "similarity": round(score, 3), })) @@ -351,10 +361,14 @@ def main() -> int: print(f" SKIP (not in --only-paths confirm list) {path}") skipped_unconfirmed += 1 continue - # Escape regex metacharacters for path:: regex query - escaped = re.escape(path) + # id, not path:: -- the beets 2.11 upgrade left the DB with mixed + # path storage (pre-upgrade items store absolute /music/... paths, + # post-upgrade imports store library-relative paths), so no single + # path query form matches both populations. Same lesson + # dedup-library.sh's process_group() already learned. Ids are + # storage-format-proof. result = subprocess.run( - ["beet", "remove", "-d", "-f", f"path::{escaped}"], + ["beet", "remove", "-d", "-f", f"id:{beets_id}"], capture_output=True, text=True, ) if result.returncode != 0: diff --git a/schema/0003_dedup_delete_id.sql b/schema/0003_dedup_delete_id.sql new file mode 100644 index 0000000..a3dbcbd --- /dev/null +++ b/schema/0003_dedup_delete_id.sql @@ -0,0 +1,10 @@ +-- Stores the beets item id of the delete_path side of a fuzzy_audio +-- candidate. find-fuzzy-dupes.py was deleting by `path::` query, which +-- silently fails to match a large fraction of the library (mixed absolute +-- vs. library-relative path storage left over from the beets 2.11 +-- upgrade -- the same issue dedup-library.sh already works around by +-- deleting via `id:` instead). Nullable: existing pending candidates +-- scanned before this column existed fall back to the old path query. +ALTER TABLE dedup_candidates ADD COLUMN delete_id INTEGER; + +UPDATE schema_version SET version = 3;