Fix dedup apply lock-hang, mawk-broken tag passes; add track delete, fix button/select CSS
confirm_and_apply no longer marks candidates confirmed before their apply job actually succeeds (a stuck/skipped_lock run was silently vanishing confirmed deletes without deleting anything), switches the fuzzy pass to --apply-pairs to avoid a full rescan on every confirm, and adds a job timeout so a hung subprocess can't hold the global pipeline lock forever. dedup-library.sh's Pass 2-4 use gawk-only features (gensub, POSIX interval regex) but were running under mawk (Debian's default awk) in the deployed image, throwing silent syntax errors on every run -- switched those invocations to gawk explicitly and added it to the Dockerfile. Also: per-track delete button on the library list/detail pages, and two CSS fixes (the primary button's hover gradient was getting clobbered by the base .btn:hover rule's plain background, and the select dropdown arrow had no background-size so it rendered oversized).
This commit is contained in:
@@ -18,6 +18,11 @@ FROM python:3.12-slim-bookworm AS runtime
|
||||
|
||||
# libicu72: sldl (.NET self-contained publish) runtime dependency.
|
||||
# ffmpeg: convert-m4a.sh. libchromaprint-tools: fpcalc for fingerprinting.
|
||||
# gawk: dedup-library.sh's Pass 2-4 use gensub() and POSIX bracket classes
|
||||
# that plain mawk (Debian's default /usr/bin/awk) doesn't support -- it
|
||||
# silently threw "syntax error" on every run, so Pass 2-4 dedup detection
|
||||
# has been a no-op in every deployed image so far. Scripts now invoke
|
||||
# `gawk` explicitly rather than relying on the `awk` alternative.
|
||||
RUN apt-get update && apt-get install -y --no-install-recommends \
|
||||
libicu72 \
|
||||
ca-certificates \
|
||||
@@ -26,6 +31,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
|
||||
flac \
|
||||
id3v2 \
|
||||
curl \
|
||||
gawk \
|
||||
&& rm -rf /var/lib/apt/lists/*
|
||||
|
||||
COPY --from=builder /opt/venv /opt/venv
|
||||
|
||||
@@ -101,3 +101,17 @@ async def retag_track(
|
||||
changed_by = user.get("email") or user.get("sub", "unknown")
|
||||
await retag.retag_from_url(item["path"], url, changed_by, keep_genre=keep_genre)
|
||||
return RedirectResponse(url=f"/library/track/{item_id}", status_code=303)
|
||||
|
||||
|
||||
@router.post("/track/{item_id}/delete")
|
||||
async def delete_track(
|
||||
item_id: int,
|
||||
user: dict = Depends(require_auth),
|
||||
db=Depends(get_db),
|
||||
):
|
||||
deleted_by = user.get("email") or user.get("sub", "unknown")
|
||||
try:
|
||||
await library_edit.delete_track(db, item_id, deleted_by)
|
||||
except ValueError:
|
||||
raise HTTPException(404, "no such track")
|
||||
return RedirectResponse(url="/library?deleted=1", status_code=303)
|
||||
|
||||
@@ -13,6 +13,14 @@ _SCRIPT = "dedup-library.sh"
|
||||
_FUZZY_SCRIPT = "find-fuzzy-dupes.py"
|
||||
_FUZZY_PASS = "fuzzy_audio"
|
||||
|
||||
# Generous ceiling for any pipeline script invoked here. Without one, a
|
||||
# stuck subprocess (e.g. a stalled stat() on the NAS mount) holds the single
|
||||
# global pipeline lock forever -- 2026-07-08 incident: one stuck dedup:apply
|
||||
# call held the lock for 16+ hours, silently skipping every later confirm
|
||||
# click, each of which still got marked confirmed=True (see confirm_and_apply)
|
||||
# and vanished from the pending list without anything actually being deleted.
|
||||
_JOB_TIMEOUT_SECONDS = 1800
|
||||
|
||||
|
||||
def _script_for_pass(pass_name: str) -> str:
|
||||
return _FUZZY_SCRIPT if pass_name == _FUZZY_PASS else _SCRIPT
|
||||
@@ -49,7 +57,7 @@ def _parse_json_lines(output: str) -> list[dict]:
|
||||
async def _run_scan(job_key: str, script_name: str, triggered_by: str) -> DedupRun:
|
||||
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
|
||||
job_key, [script, "--json"], triggered_by=triggered_by, timeout=_JOB_TIMEOUT_SECONDS
|
||||
)
|
||||
candidates = _parse_json_lines(output)
|
||||
|
||||
@@ -111,68 +119,101 @@ async def scan_fuzzy(triggered_by: str = "manual") -> DedupRun:
|
||||
return await _run_scan("dedup:scan_fuzzy", _FUZZY_SCRIPT, triggered_by)
|
||||
|
||||
|
||||
def _write_apply_args(script_name: str, script: str, group: list[DedupCandidate], stamp: int) -> list[str]:
|
||||
"""Build the --apply invocation for one pass's confirmed group.
|
||||
|
||||
find-fuzzy-dupes.py gets --apply-pairs: it applies the exact keep/delete
|
||||
pairs the user already reviewed without re-scanning the whole library
|
||||
for acoustic matches (that rescan is what hung for 16+ hours on
|
||||
2026-07-08, holding the global pipeline lock). dedup-library.sh has no
|
||||
equivalent fast path -- its tag-based passes are cheap to rescan, so
|
||||
--apply --only-paths (full rescan, filtered to the confirmed list) is
|
||||
fine there.
|
||||
"""
|
||||
if script_name == _FUZZY_SCRIPT:
|
||||
pairs_file = settings.logs_dir / f"dedup-confirm-{stamp}-{script_name}.jsonl"
|
||||
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
|
||||
)
|
||||
+ "\n"
|
||||
)
|
||||
return [script, "--apply-pairs", str(pairs_file), "--json"]
|
||||
|
||||
confirm_file = settings.logs_dir / f"dedup-confirm-{stamp}-{script_name}.txt"
|
||||
confirm_file.parent.mkdir(parents=True, exist_ok=True)
|
||||
confirm_file.write_text("\n".join(c.delete_path for c in group) + "\n")
|
||||
return [script, "--apply", "--only-paths", str(confirm_file), "--json"]
|
||||
|
||||
|
||||
async def confirm_and_apply(candidate_ids: list[int], confirmed_by: str) -> DedupRun | None:
|
||||
"""Apply only the confirmed candidate deletions.
|
||||
|
||||
Candidates are only marked confirmed=True once we know their apply job
|
||||
actually ran (status == "success") -- if the shared pipeline lock is
|
||||
held by something else, or the job times out/crashes, those candidates
|
||||
are left untouched so they stay visible in the pending list instead of
|
||||
silently vanishing without being deleted (see 2026-07-08 incident).
|
||||
|
||||
Cheap pre-check here: skip anything where delete_path or keep_path no
|
||||
longer exists (something already changed it since the scan). The real
|
||||
longer exists (something already changed it since the scan). Further
|
||||
re-verification of ranking happens for free inside the underlying
|
||||
script itself: --apply --only-paths re-runs its passes from scratch and
|
||||
recomputes keep/delete for every group before consulting the only-paths
|
||||
allowlist, so if a group's ranking flipped since the scan (e.g. the old
|
||||
keep_path is gone and delete_path is now the last copy), the script's
|
||||
fresh pass will assign delete_path the KEEP role instead -- it never
|
||||
reaches a DELETE branch for it, so --only-paths naming it is simply
|
||||
never consulted. No duplicate ranking logic needed here.
|
||||
script itself -- see _write_apply_args().
|
||||
|
||||
dedup-library.sh (tag-based passes) and find-fuzzy-dupes.py (acoustic
|
||||
fingerprint pass) are separate scripts, each with its own --apply
|
||||
--only-paths invocation -- candidates are grouped by pass_name and each
|
||||
group is applied through the script that actually produced it.
|
||||
fingerprint pass) are separate scripts -- candidates are grouped by
|
||||
pass_name and each group is applied through the script that actually
|
||||
produced it.
|
||||
"""
|
||||
db = SessionLocal()
|
||||
try:
|
||||
candidates = [db.get(DedupCandidate, cid) for cid in candidate_ids]
|
||||
candidates = [c for c in candidates if c is not None and not c.applied]
|
||||
|
||||
now = time.time()
|
||||
confirmed_candidates = []
|
||||
for c in candidates:
|
||||
if not Path(c.delete_path).exists() or not Path(c.keep_path).exists():
|
||||
continue
|
||||
c.confirmed = True
|
||||
c.confirmed_by = confirmed_by
|
||||
c.confirmed_at = now
|
||||
confirmed_candidates.append(c)
|
||||
db.commit()
|
||||
|
||||
if not confirmed_candidates:
|
||||
candidates = [c for c in candidates if Path(c.delete_path).exists() and Path(c.keep_path).exists()]
|
||||
if not candidates:
|
||||
return None
|
||||
|
||||
by_script: dict[str, list[DedupCandidate]] = {}
|
||||
for c in confirmed_candidates:
|
||||
for c in candidates:
|
||||
by_script.setdefault(_script_for_pass(c.pass_name), []).append(c)
|
||||
|
||||
now = time.time()
|
||||
finished_at = now
|
||||
log_paths = []
|
||||
ran_candidates: list[DedupCandidate] = []
|
||||
for script_name, group in by_script.items():
|
||||
confirm_file = settings.logs_dir / f"dedup-confirm-{int(now * 1000)}-{script_name}.txt"
|
||||
confirm_file.parent.mkdir(parents=True, exist_ok=True)
|
||||
confirm_file.write_text("\n".join(c.delete_path for c in group) + "\n")
|
||||
|
||||
script = str(settings.pipeline_dir / "lib" / script_name)
|
||||
argv = _write_apply_args(script_name, script, group, int(now * 1000))
|
||||
job_run, _output = await pipeline_runner.run_job_capture(
|
||||
"dedup:apply",
|
||||
[script, "--apply", "--only-paths", str(confirm_file), "--json"],
|
||||
argv,
|
||||
triggered_by=f"manual:{confirmed_by}",
|
||||
timeout=_JOB_TIMEOUT_SECONDS,
|
||||
)
|
||||
finished_at = job_run.finished_at
|
||||
finished_at = job_run.finished_at or finished_at
|
||||
if job_run.log_path:
|
||||
log_paths.append(job_run.log_path)
|
||||
|
||||
still_there = {c.delete_path for c in confirmed_candidates if Path(c.delete_path).exists()}
|
||||
if job_run.status != "success":
|
||||
# Lock contention, crash, or timeout -- nothing in this
|
||||
# group was actually run. Leave confirmed=False so these
|
||||
# stay in the pending list for a retry.
|
||||
continue
|
||||
|
||||
for c in group:
|
||||
c.confirmed = True
|
||||
c.confirmed_by = confirmed_by
|
||||
c.confirmed_at = now
|
||||
ran_candidates.extend(group)
|
||||
db.commit()
|
||||
|
||||
if not ran_candidates:
|
||||
return None
|
||||
|
||||
still_there = {c.delete_path for c in ran_candidates if Path(c.delete_path).exists()}
|
||||
actually_deleted = 0
|
||||
for c in confirmed_candidates:
|
||||
for c in ran_candidates:
|
||||
if c.delete_path not in still_there:
|
||||
c.applied = True
|
||||
actually_deleted += 1
|
||||
@@ -183,7 +224,7 @@ async def confirm_and_apply(candidate_ids: list[int], confirmed_by: str) -> Dedu
|
||||
finished_at=finished_at,
|
||||
mode="apply",
|
||||
deleted=actually_deleted,
|
||||
kept=len(confirmed_candidates) - actually_deleted,
|
||||
kept=len(ran_candidates) - actually_deleted,
|
||||
log_path=";".join(log_paths) or None,
|
||||
)
|
||||
db.add(apply_run)
|
||||
|
||||
@@ -13,6 +13,14 @@ from app.services import beets_service, pipeline_runner
|
||||
EDITABLE_FIELDS = ["artist", "title", "album", "albumartist", "genres", "grouping", "track", "year"]
|
||||
|
||||
|
||||
def _run_beet_remove(item_id: int) -> subprocess.CompletedProcess:
|
||||
return subprocess.run(
|
||||
["beet", "remove", "-d", "-f", f"id:{item_id}"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
|
||||
|
||||
def _run_beet_modify(item_id: int, changes: dict[str, str]) -> subprocess.CompletedProcess:
|
||||
"""subprocess with an argument list (no shell=True) -- user-submitted
|
||||
field values never pass through a shell, so there's no injection risk
|
||||
@@ -87,3 +95,32 @@ async def update_track_fields(
|
||||
)
|
||||
|
||||
return changed_fields
|
||||
|
||||
|
||||
async def delete_track(db: Session, item_id: int, deleted_by: str) -> None:
|
||||
"""Remove a single track from beets and delete its file on disk, via
|
||||
the same `beet remove -d -f` primitive the dedup passes use to delete a
|
||||
loser -- exposed directly here for "I just want this one gone" cases
|
||||
dedup doesn't cover (a bad rip, something you decided you don't want,
|
||||
etc.)."""
|
||||
item = beets_service.get_item(item_id)
|
||||
if item is None:
|
||||
raise ValueError(f"no beets item with id={item_id}")
|
||||
|
||||
result = await asyncio.to_thread(_run_beet_remove, item_id)
|
||||
if result.returncode != 0:
|
||||
raise RuntimeError(f"beet remove failed: {result.stderr.strip()}")
|
||||
|
||||
db.add(
|
||||
ManualFixAudit(
|
||||
beets_item_id=item_id,
|
||||
file_path=item.get("path", ""),
|
||||
field="__deleted__",
|
||||
old_value=item.get("path"),
|
||||
new_value=None,
|
||||
changed_by=deleted_by,
|
||||
changed_at=time.time(),
|
||||
source="manual_delete",
|
||||
)
|
||||
)
|
||||
db.commit()
|
||||
|
||||
+26
-2
@@ -320,7 +320,19 @@ main {
|
||||
background-size: 220% auto;
|
||||
box-shadow: 0 10px 30px -12px rgba(167, 139, 250, 0.6);
|
||||
}
|
||||
.btn-primary:hover { background-position: 100% 50%; box-shadow: 0 10px 34px -8px rgba(167, 139, 250, 0.75); }
|
||||
/* Re-declare the full `background` shorthand here, not just
|
||||
background-position: .btn:hover above sets `background` (shorthand) to
|
||||
a flat color at equal specificity, and since it comes first in source
|
||||
order .btn-primary:hover must repeat the shorthand to still win --
|
||||
otherwise it silently wipes out the gradient image/size, leaving the
|
||||
near-black .btn-primary text on a near-black background ("the glow
|
||||
makes the button black"). */
|
||||
.btn-primary:hover {
|
||||
background: var(--gradient-holo);
|
||||
background-size: 220% auto;
|
||||
background-position: 100% 50%;
|
||||
box-shadow: 0 10px 34px -8px rgba(167, 139, 250, 0.75);
|
||||
}
|
||||
|
||||
.btn-danger {
|
||||
background: rgba(251, 113, 133, 0.1);
|
||||
@@ -365,7 +377,19 @@ input:focus, textarea:focus, select:focus {
|
||||
border-color: var(--iris-violet);
|
||||
box-shadow: 0 0 0 3px rgba(167, 139, 250, 0.18);
|
||||
}
|
||||
select { appearance: none; background-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 20 20' fill='none'%3E%3Cpath d='M5 7.5 10 12.5 15 7.5' stroke='%239799ac' stroke-width='1.6' stroke-linecap='round' stroke-linejoin='round'/%3E%3C/svg%3E"); background-repeat: no-repeat; background-position: right 0.75rem center; padding-right: 2.25rem; }
|
||||
/* No intrinsic width/height on the inline SVG means browsers fall back to
|
||||
an oversized default replaced-element size for the background-image
|
||||
(no background-size was set here) -- that's what was rendering the
|
||||
arrow huge enough to sit on top of the "(any)" text. Pin it to a small
|
||||
fixed size explicitly. */
|
||||
select {
|
||||
appearance: none;
|
||||
background-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 20 20' fill='none'%3E%3Cpath d='M5 7.5 10 12.5 15 7.5' stroke='%239799ac' stroke-width='1.6' stroke-linecap='round' stroke-linejoin='round'/%3E%3C/svg%3E");
|
||||
background-repeat: no-repeat;
|
||||
background-position: right 0.75rem center;
|
||||
background-size: 0.85rem 0.85rem;
|
||||
padding-right: 2.25rem;
|
||||
}
|
||||
|
||||
.checkbox-field {
|
||||
display: flex;
|
||||
|
||||
@@ -8,6 +8,10 @@
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{% if request.query_params.get('deleted') %}
|
||||
<div class="notice success">Track deleted.</div>
|
||||
{% endif %}
|
||||
|
||||
<form method="get" action="/library" class="panel" style="display:flex; gap:0.75rem; flex-wrap:wrap; align-items:flex-end; max-width:none;">
|
||||
<div class="field">
|
||||
<label for="q">Search</label>
|
||||
@@ -47,7 +51,13 @@
|
||||
<td>{{ item.title }}</td>
|
||||
<td class="muted">{{ item.albumartist }}</td>
|
||||
<td><span class="badge badge-muted">{{ item.format }}</span></td>
|
||||
<td><a href="/library/track/{{ item.id }}" class="btn btn-sm btn-ghost">Edit</a></td>
|
||||
<td class="actions-cell">
|
||||
<a href="/library/track/{{ item.id }}" class="btn btn-sm btn-ghost">Edit</a>
|
||||
<form method="post" action="/library/track/{{ item.id }}/delete" class="inline"
|
||||
onsubmit="return confirm('Delete {{ item.artist }} — {{ item.title }}? This removes the file from disk permanently.')">
|
||||
<button type="submit" class="btn btn-sm btn-danger">Delete</button>
|
||||
</form>
|
||||
</td>
|
||||
</tr>
|
||||
{% else %}
|
||||
<tr class="empty-row"><td colspan="5">
|
||||
|
||||
@@ -50,4 +50,13 @@
|
||||
<button type="submit" class="btn">Retag</button>
|
||||
</form>
|
||||
</div>
|
||||
|
||||
<h2>Danger zone</h2>
|
||||
<div class="panel" style="max-width:560px;">
|
||||
<p class="muted" style="margin-top:0;">Removes this track from beets and deletes the file from disk. Not reversible.</p>
|
||||
<form method="post" action="/library/track/{{ item.id }}/delete"
|
||||
onsubmit="return confirm('Delete {{ item.artist }} — {{ item.title }}? This removes the file from disk permanently.')">
|
||||
<button type="submit" class="btn btn-danger">Delete this track</button>
|
||||
</form>
|
||||
</div>
|
||||
{% endblock %}
|
||||
|
||||
@@ -254,7 +254,7 @@ log "[$(date -Iseconds)] === Pass 2: case-insensitive title/album/artist dedup =
|
||||
beet ls -f "\$id${SEP}\$albumartist${SEP}\$album${SEP}\$title${SEP}\$path" \
|
||||
2>/dev/null > /tmp/all-tracks.txt
|
||||
|
||||
awk -F"${SEP}" -v sep="$SEP" '
|
||||
gawk -F"${SEP}" -v sep="$SEP" '
|
||||
# Sentinel titles carry zero identity — multiple distinct tracks can land on
|
||||
# the same value when spotify-retag misses and the Soulseek source had a
|
||||
# blank/placeholder title. Grouping by sentinel title = deleting different
|
||||
@@ -321,7 +321,7 @@ beet ls -f "\$id${SEP}\$albumartist${SEP}\$album${SEP}\$title${SEP}\$path" \
|
||||
# Normalize: lowercase + strip everything except a-z 0-9. Group by the
|
||||
# concatenation. Suppresses dupe pairs already caught by Pass 2 (same exact
|
||||
# key) by also tracking the raw key per group and skipping single-key groups.
|
||||
awk -F"${SEP}" -v sep="$SEP" '
|
||||
gawk -F"${SEP}" -v sep="$SEP" '
|
||||
function norm(s, t) {
|
||||
t = tolower(s)
|
||||
gsub(/[^a-z0-9]/, "", t)
|
||||
@@ -420,7 +420,7 @@ CROSS_ALBUM_KEEP_RE='^(porterrobinson|madeon|variousartists)$'
|
||||
# against "Apple Music Live" recordings, etc.
|
||||
ALBUM_KEEP_RE='(soundtrack|originalmotionpicture|ost|vgm|gameost|liveat|livein|livefrom|livesession|applemusiclive|applelive|bbclive|bbcsession|mtvunplugged|unplugged|concert|inconcert|extended|remix|remixed|remixes|originalmix|radioedit|radiomix|clubmix|djedit|djmix|dubmix|vocalmix|instrumentalmix|longversion|shortversion|albumversion|originalversion)'
|
||||
|
||||
awk -F"${SEP}" -v sep="$SEP" \
|
||||
gawk -F"${SEP}" -v sep="$SEP" \
|
||||
-v artist_keep="$CROSS_ALBUM_KEEP_RE" \
|
||||
-v album_keep="$ALBUM_KEEP_RE" '
|
||||
function norm(s, t) {
|
||||
|
||||
Reference in New Issue
Block a user