Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ KJ Controller is a web-based karaoke show management application. A Flask backen
| `sing_store.py` | ~260 | `SingStore` class: SQLite CRUD for `sing_requests` + `sing_push_subscriptions` + event-token helpers (regenerate / enable / auto-approve) on `rotation_meta` |
| `routes.py` | ~1000 | Flask Blueprint with all route handlers (includes `/rotation/requests/*` admin endpoints for the public request form). Hosts the playability gates: tier-1 inline `_playability_gate` (link/upload/download hard-block) + tier-2 async render verification (`_enqueue_tier2` → single-worker queue → `_run_tier2_check` against the active renderer, stamps `playability_warning`) |
| `wait_estimate.py` | ~80 | Pure function `compute_estimate(entries, target_id, cfg)` producing `{position, expected_s, range_low_s, range_high_s, spread_source, close_to_front, now_singing}`. Uses tonight's sung-entry variance for the range; falls back to a configurable minimum spread. |
| `sing_resolve.py` | ~120 | Pure decision logic for singer-submission download fallback: `classify_error` (unavailable → advance to next candidate vs transient → retry same) and `next_candidate_index` (bounded by `MAX_CANDIDATES`). No yt-dlp/network/Flask deps → exhaustively unit-tested. |

### Dependency Flow

Expand Down Expand Up @@ -421,6 +422,15 @@ Additional `static-sing/` assets added in sub-project #4:
- iOS Safari requires Add-to-Home-Screen (installed PWA) before push works. The confirmation page detects iOS non-standalone and renders an instructional card explaining the install flow.
- Housekeeping: on event-token regeneration, `cleanup_stale_push_subscriptions` deletes subs on other tokens older than 7 days. Keeps the table bounded.

### Singer submission download fallback

When an approved singer submission's YouTube download fails, the download worker auto-heals instead of surfacing a dead ❌ the KJ must fix by hand (motivated by the 2026-07-09 live incident where a picked "Say My Name" version was a *private video*).

- **The download attempt is the probe.** `media.download_video` swallows yt-dlp errors and returns `(None, None)`, so it now records the reason on `media._last_error`. On failure the single-threaded `_download_worker` reads it and calls `sing_resolve.classify_error`.
- **Advance vs retry.** `unavailable` (private/removed/blocked) → advance to the next ranked candidate version; `transient` (timeout/429/`bgutil`/network, and any *unknown* error) → retry the same candidate up to `MAX_TRANSIENT_RETRIES`, then advance. Bounded by `MAX_CANDIDATES` (3). Because the worker is sequential, retries re-queue (back of line) rather than sleep-blocking.
- **Candidate list.** `approve_sing_request` attaches a ranked YouTube candidate list to the queue item, built from the `versions[]` snapshot via the existing `_pick_version_from_kj_pick` translator + `_ranked_version_indices`. Since binding a `kj_pick` version rewrites `source_meta`, `_preserve_versions_meta` re-attaches the snapshot at both binding sites so the list survives. v1 falls back across YouTube-type candidates only (cross-source local/Divebar is a documented follow-up).
- **Outcome.** On a successful fallback the request source is rebound (`update_request_source`) so `/my-requests` reflects the version that landed, and the singer gets a `resolved_alt` push. When every candidate is exhausted the entry surfaces the normal terminal ❌ for the KJ plus an `unavailable` push. Non-sing downloads (KJ manual, Divebar) are untouched.

## VNC Screen Preview

The KJ Controller web UI includes a live thumbnail of the Pi's screen via an embedded VNC viewer. This lets the KJ see what's on the HDMI output without a direct line of sight to the display.
Expand Down
20 changes: 20 additions & 0 deletions docs/TROUBLESHOOTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,26 @@ ssh nomadpc "curl -s -X POST http://127.0.0.1:5001/fix_audio -H 'Content-Type: a
Track A auto-recovery (v0.68.0+) auto-restarts the engine and shows the KJ an amber banner if a
file still crashes it, so a crash is a ~2s blip rather than a dead show.

## Singer Submission Shows "Unavailable" or Auto-Swapped Version

A singer submitted a song through the `/sing` UI and it either quietly played a **different
version** than expected, or shows a **red ❌ download-failed** with the singer told "we couldn't
find a playable version."

- **This is expected auto-fallback behaviour (2026-07-09+).** The version the singer/KJ picked was
an unavailable YouTube video (private, deleted, region-blocked). Rather than dead-ending, the
download worker automatically tries the next-best candidate version of the same song and rebinds
the request to whichever one downloads.
- **Auto-swap (no action needed):** the entry ends up linked to a working version and `/my-requests`
shows it. If the swapped version is a poor match, use the rotation 🔗 link / "Try Another" button
to pick a different one manually — same as before.
- **Terminal ❌ (KJ action):** every candidate was unavailable (or there were no alternates — e.g. a
single-version song, or a raw pasted URL). Link a working file manually via the rotation entry.
- **Everything shows unavailable / nothing downloads:** that's a different problem — check YouTube
health (cookies, yt-dlp version) and whether the `bgutil` PO-token helper at `127.0.0.1:4416` is
reachable. Persistent transient errors (timeouts/429) exhaust the bounded retries and then surface
as terminal ❌. `journalctl -u kj-controller` shows `Sing fallback:` lines tracing each decision.

## Docker Containers Not Running

```bash
Expand Down
163 changes: 163 additions & 0 deletions docs/archive/2026-07-09-singer-submission-validation-plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
# Plan: Auto-resolving singer submissions (download validation + fallback)

**Created:** 2026-07-09
**Branch:** feat/sess-20260717-0153-singer-submission-validation
**Status:** Implemented (pending review/merge)
**Design spec:** [2026-07-09-singer-submission-validation-design.md](./2026-07-09-singer-submission-validation-design.md)

## As-Built Deviations

Discovered during implementation (all reflected in the code + tests):

- **D1 — failure reason via `media._last_error`, not a caught exception.** `download_video`
swallows yt-dlp errors and returns `(None, None)`; it now records the reason on
`media._last_error`, which the single (serialized) download worker reads. No second network probe.
- **D2 — transient exhaustion advances, not terminates.** Transient retries per candidate then fall
through to the next candidate; only an exhausted candidate list is terminal. Unknown errors
default to transient.
- **D3 — no new sing-request status.** Terminal = the existing rotation `failed` download_status +
an `unavailable` push; success rebinds via the existing `update_request_source`.
- **D4 — pivot from the planned client change.** Direct `youtube`/`kn` picks are only ever
single-version (no alternates), so attaching `versions[]` client-side adds nothing. The real fix
is `_preserve_versions_meta`: keep the `versions[]` snapshot through `kj_pick` binding (both the
admin approve route and `resolve_kj_pick_best`) so multi-version songs — the incident case — have
candidates. `sing.js` was **not** changed.
- **D5 — v1 scope: YouTube-type fallback only.** Cross-source (local/Divebar) fallback is a
documented follow-up.
- Open questions resolved: push uses the existing `notify_request_decision` (+ two new copy steps);
no `unavailable` status enum needed; caps kept at 3 candidates / 2 transient retries.

## Overview

When a singer's picked YouTube version can't be downloaded (private/deleted video, as in the
2026-07-09 live incident), the download worker should automatically fall back to the next-best
candidate version of the same song, notifying the singer only when nothing is playable. Async, no
false rejections on transient network blips, zero added latency on the happy path.

## Requirements

- [ ] On a download failure classified as **unavailable**, auto-advance to the next candidate
version and retry, without KJ intervention.
- [ ] On a failure classified as **transient** (timeout / 429 / `bgutil` / network), retry the
**same** candidate — do not consume the candidate list.
- [ ] Cap total resolution effort: ≤ 3 distinct candidates tried; bounded transient retries.
- [ ] When a fallback succeeds, rebind the request's source so `/my-requests` reflects the version
actually used; when all candidates fail, mark the request terminally "unavailable" and flag
the rotation entry for the KJ.
- [ ] Singer is notified on terminal states only (version-changed / no-version), via existing
`/my-requests` polling (baseline) and Web Push (enhancement).
- [ ] Single direct `youtube`/`kn` picks carry the group's ranked `versions[]` so they have
fallback candidates too.
- [ ] Non-functional: no submit-path latency added; worker stays single-threaded and never
sleep-blocks the queue; no infinite re-queue loops.

## Technical Approach

**Reuse over new machinery.** The "candidate list" is the existing `versions[]` snapshot already
stored in `source_meta` (today only for `kj_pick`). Fallback = advance an index over that list and
reuse `_pick_version_from_kj_pick`'s translation of `versions[i] → (source_type, source_ref,
source_meta)`.

**Fallback lives in the download worker's existing error branch** (`routes.py:713–739`). Today that
branch discards the exception and marks the item `error`. We change it to:
1. Capture the exception message.
2. For **sing-request-backed** items only (identified by a new `request_id` + `candidates` on the
queue item), consult the resolver.
3. `unavailable` + candidates remain under cap → translate next candidate, `update_request_source`,
reset the item to `queued` (worker re-picks it), continue.
4. `transient` under retry cap → bump an attempt counter, reset to `queued` (back of line — no
`sleep`, since the worker is sequential), continue.
5. Exhausted / terminal → mark request `unavailable`, flag rotation entry, fire one Web Push.

Non-sing downloads (KJ manual, divebar) keep today's behaviour untouched.

**Error classification** is isolated in a pure, unit-tested module so the risky part (deciding
unavailable vs transient) is testable without yt-dlp or the network. Unknown errors default to
`transient` (safer: retry same candidate rather than wrongly discarding a good one).

### Trade-offs considered

- *Separate metadata probe vs download-as-probe* → download-as-probe (per spec): no happy-path
latency, reuses the download path.
- *Transient retry inline (`sleep`) vs re-queue* → re-queue with attempt counter, because the
single worker thread must not block other singers' downloads.
- *New candidate schema vs reuse `versions[]`* → reuse; `_pick_version_from_kj_pick` already
translates it and the client already produces it.

## Implementation Steps

1. [ ] **`sing_resolve.py` (new, pure).**
- `classify_error(message: str) -> "unavailable" | "transient"` with pattern tables
(unavailable: `Private video`, `Video unavailable`, `has been removed`, `blocked in your
country`, account-terminated; transient: timeouts, `HTTP Error 429/5xx`, `bgutil`, connection
reset). Unknown → `transient`.
- `next_candidate_index(total, tried) -> int | None` and a `MAX_CANDIDATES = 3`,
`MAX_TRANSIENT_RETRIES = 2` constants.
2. [ ] **Unit tests `tests/test_sing_resolve.py`** — real yt-dlp strings incl. the incident's
`ERROR: [youtube] _vMTtVPhd80: Private video`; ordering/cap/exhaustion; unknown→transient.
3. [ ] **Client `static-sing/sing.js`** — attach the group's ranked `versions[]` in `source_meta`
for direct `youtube`/`kn` submissions (mirror the existing `kj_pick` line ~658). Preserves
existing single-pick behaviour otherwise.
4. [ ] **`approve_sing_request` (routes.py ~4844 queue_item build)** — for `youtube`/`kn`, add to
the queue item: `request_id`, `candidates` (the `versions[]` list, may be empty), `current_index`
(0), `tried` ([]), `transient_attempts` (0). No behaviour change when `candidates` is empty
beyond richer terminal messaging.
5. [ ] **`_download_worker` error branch (routes.py:713–739)** — capture `exc`; extract
`_attempt_sing_fallback(app, item, str(exc))` helper that implements steps 3–5 of the approach.
Keep non-sing items on the current path.
6. [ ] **`sing_store.py`** — add a terminal status transition (e.g. `mark_unavailable(request_id,
reason)`) and confirm `update_request_source` is used for successful rebinds. Ensure the status
enum/index covers `unavailable`.
7. [ ] **Singer-facing surface** — extend `_public_request_view` (sing.py) to expose the resolution
state + the version label actually used, so `/my-requests` renders "finding…/locked in
(label)/unavailable" without a new endpoint.
8. [ ] **Web Push (enhancement) `push_dispatcher.py`** — add a targeted per-request send for the two
terminal states. *Open question:* confirm the dispatcher exposes a per-singer/subscription send
(vs only rotation-mutation broadcasts). If not trivially available, ship steps 1–7 first
(polling covers the UX) and add push as a follow-up.
9. [ ] **Integration tests** — worker + stub downloader: (a) c0 unavailable→c1 ok (rebind + one
push), (b) all unavailable (terminal + KJ flag + one push), (c) transient→same candidate retried,
list not consumed, (d) fallback candidate already on disk → linked via `_existing_media_for`,
no re-download.
10. [ ] **Docs** — `docs/ARCHITECTURE.md` (new module + fallback flow), `docs/CHANGELOG.md` (dated
entry), `docs/TROUBLESHOOTING.md` (what "unavailable — KJ notified" means).

## Files to Create/Modify

| File | Action | Description |
|------|--------|-------------|
| `kj-controller/sing_resolve.py` | Create | Pure error classifier + candidate iteration + caps |
| `kj-controller/tests/test_sing_resolve.py` | Create | Unit tests for the resolver |
| `kj-controller/static-sing/sing.js` | Modify | Attach `versions[]` for direct youtube/kn picks |
| `kj-controller/routes.py` | Modify | Queue-item fields; `_attempt_sing_fallback`; worker error branch |
| `kj-controller/sing_store.py` | Modify | `mark_unavailable` transition; status enum coverage |
| `kj-controller/sing.py` | Modify | Expose resolution state/label in `_public_request_view` |
| `kj-controller/push_dispatcher.py` | Modify | Targeted terminal-state push (enhancement) |
| `kj-controller/tests/test_sing_fallback*.py` | Create | Integration tests for worker fallback |
| `docs/ARCHITECTURE.md`, `docs/CHANGELOG.md`, `docs/TROUBLESHOOTING.md` | Modify | Document module + flow |

## Testing Strategy

- **Unit:** `classify_error` (incl. incident string, unknown→transient), candidate iteration/caps.
- **Integration:** worker-with-stub scenarios (a)–(d) above; assert request status, source rebind,
push count, and no-double-download.
- **Manual (pre-merge, local):** run kj-controller locally, submit a request pointing at a known
private video with a working alternate in `versions[]`; confirm auto-fallback, `/my-requests`
copy, and KJ rotation flag. `cd kj-controller && pytest --cov`.

## Open Questions

- [ ] Does `push_dispatcher` expose a per-request/subscription send, or only rotation-mutation
broadcasts? Determines whether step 8 lands in this PR or as a fast follow.
- [ ] Confirm the sing_requests `status` enum should gain `unavailable` (vs reusing `rejected` with
a reason). Prefer a distinct `unavailable` for singer-facing copy.
- [ ] Candidate cap (3) and transient-retry cap (2) — tune during integration if needed.

## Rollback Plan

- Pure code addition behind the existing worker path; nothing changes for non-sing downloads.
- If fallback misbehaves in production, revert the `routes.py` worker-branch change (and the
`sing.js` `versions[]` attachment) — the system returns to today's "single attempt, red ❌,
manual KJ fix" behaviour with no data migration to undo (new queue-item fields are ignored).
- This is a **backend** change → requires `systemctl restart kj-controller`; deploy only in a
maintenance window, never mid-show.
9 changes: 9 additions & 0 deletions kj-controller/docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@ Dated entries, newest first. Each entry notes any required deploy steps.

---

## 2026-07-17 - Auto-fallback for failed singer-submission downloads (v0.89.0)

**Deploy:** backend change (`routes.py`, `media.py`, `push_dispatcher.py`, new `sing_resolve.py`) → **requires `systemctl restart kj-controller`** (interrupts playback — deploy between songs). No DB migration; new queue-item fields are in-memory only and ignored on rollback.

- When an approved singer submission's YouTube download fails because the video is **unavailable** (private/deleted/blocked — the 2026-07-09 "Say My Name" private-video incident), the download worker now automatically advances to the next-best candidate version of the same song instead of dead-ending at a red ❌ the KJ had to fix by hand. Only a request with **no** playable candidate left surfaces the terminal failure (and now pushes the singer an honest "we couldn't find a playable version — your KJ has been notified").
- **No false rejections.** Transient failures (timeouts, HTTP 429, the `bgutil` PO-token helper being down, network blips — and any unrecognised error) retry the *same* candidate a bounded number of times before advancing, so a flaky network never discards a good video. Classification lives in a pure, exhaustively unit-tested `sing_resolve` module.
- On a successful fallback the request is rebound to the version that actually downloaded (so `/my-requests` shows the right one) and the singer gets a "we queued an alternate version" push.
- Mechanics: `media.download_video` records its failure reason on `media._last_error`; the single-threaded `_download_worker` classifies it and walks a ranked candidate list attached to the queue item (bounded to 3 candidates). The `versions[]` snapshot is preserved through `kj_pick` binding so multi-version songs — the common case — have alternates to try. YouTube-type fallback only in v1; cross-source (local/Divebar) fallback is a follow-up.

## 2026-07-17 - Disable all mpv audio processing (pitch + vocals guide) by default (v0.87.0)

**Deploy:** backend change (`mpv_manager.py`, `routes.py`, `config.py`) → **requires `systemctl restart kj-controller`** (interrupts playback — deploy between songs). No DB migration. Frontend hides the affected controls automatically on the next status/renderer poll (no cache-bust needed beyond the version bump).
Expand Down
Loading