Refocus the improvement plan on speed and new features

This commit is contained in:
Claude
2026-09-29 17:40:57 +00:00
parent 92985d209a
commit a0ffc1b493

View File

@@ -1,153 +1,159 @@
# ytplayer — Improvement Plan (2026-09-29)
# ytplayer — Speed & Features Plan (2026-09-29)
Grounded in the current tree: `frontend/app.js` 9.8k lines / 30+ module objects,
`styles.css` 4.3k, `server/server.js` 1.9k with ~30 routes, 11 unit/server test
files (~110 cases), 5 Playwright specs, **no CI**, no lint/format config.
The feature set is already broad (lyrics, party, remote, presenter, stats,
transitions, uploads). The biggest returns now come from **hardening and
maintainability**, not more features.
Scope: **make the app faster** (startup, search, pressing play, moving around)
and **add new features**. Hardening/refactor work is intentionally out of scope.
Priority key: **P0** do next · **P1** this quarter · **P2** when there's room.
Effort: S ≤ ½ day · M 1–3 days · L a week+.
## Where the time goes today (measured against prod)
Measured with `curl` from a cloud container on 2026-09-29, so absolute numbers
include this client's network; the *differences* are what matter.
| What | Measured | Of which is the app itself |
|------|----------|----------------------------|
| Trivial request (`/api/version`) | ~1.25 s (≈0.9 s TLS setup, ≈0.35 s request) | ~0.35 s round trip through VPS → WireGuard → homelab |
| Download `app.js` | 2.8 s at ~150 KB/s | **426 KB sent uncompressed** — no `content-encoding`, no ETag |
| `/api/search` (new query) | 4.1–5.0 s | ~3.5 s is one yt-dlp spawn (already `--flat-playlist`) |
| `/api/search` (repeat, 3-min cache) | 1.2 s | cache hit = baseline |
| `/api/streams` (first play) | **7.5 s** | ~6 s is `yt-dlp -J` |
| `/api/streams` (second play) | 1.2 s | `streamCache` hit |
Shell sizes: `app.js` 426 KB → **118 KB gzip**; `styles.css` 161 KB → 32 KB;
`index.html` 32 KB → 8 KB. Total first load **618 KB → 158 KB** just by
compressing. Plus a render-blocking Google Fonts stylesheet (3 families, 10
weights) before first paint.
The three biggest wins, in order: **(1) compress + ETag the shell, (2) stop
paying a cold yt-dlp spawn on every search and first play, (3) start that work
before the user taps.**
---
## Phase 1 — Safety net (P0)
## Part A — Speed
Nothing below is safe to refactor without it; the CLAUDE.md history (update
banner "fixed three times", the thumb-cache opaque trap, the toast-loop tab
crash) shows regressions have repeatedly shipped silently.
### A1. App startup (first paint, cold and warm)
| # | Item | Effort | Done when |
|---|------|--------|-----------|
| 1.1 | **GitHub Actions CI**: `node --test frontend/*.test.js`, `cd server && bun run test`, Playwright chromium smoke specs. Run on every push/PR. | S | Red CI blocks a merge; badge in README. |
| 1.2 | Fix root `npm test` — it runs `node --test frontend/`, which CLAUDE.md says fails on Node 22. Change to `node --test frontend/*.test.js` and add `test:server`, `test:e2e`, `test:all`. | S | `npm run test:all` green locally and in CI. |
| 1.3 | **Lint + format**: Biome (one binary, zero config sprawl) with `no-unused-vars`, `no-undef`, `useAwait`, `noFallthrough`. Start with `--diagnostic-level=error` so it lands without a mass reformat commit. | S | CI step; existing code passes. |
| 1.4 | **Regression tests for the known fragile paths**: (a) `/sw.js` BUILD_TAG regex injection survives a bumped fallback literal; (b) `maybeShowUpdateBanner` single-rule behaviour; (c) thumb cache accepts CORS-mode responses (status 0 opaque trap); (d) `toast()` cap with deferred removal terminates. | M | Each has a test that fails on the historical bug. |
| 1.5 | **Server route contract tests** for `/api/streams`, `/api/search`, `/api/user/sync`, `/api/profile/*` against a stubbed yt-dlp binary (a script that prints fixture JSON). Guards the "JSON shapes mirror the Tauri bridge" rule. | M | Shape snapshot per endpoint. |
| # | Change | Expected gain | Effort |
|---|--------|---------------|--------|
| A1.1 | **Compress static + JSON.** At boot (and when `BUILD_TAG` changes) pre-compress every shell file with `Bun.gzipSync` / brotli, keep them in memory, serve by `Accept-Encoding` with `Vary: Accept-Encoding`. JSON API responses via `hono/compress`. **Never** compress `/api/play`, `/api/media/*` (Range media). | Shell 618 → ~158 KB; ~2 s off a cold load at the measured rate | S |
| A1.2 | **ETags on the shell.** Everything is `Cache-Control: no-cache`, which is right (see CLAUDE.md update flow), but without an ETag every revalidation re-downloads the full body. Use the content hash already computed for `BUILD_TAG` per file → `304 Not Modified`. Keeps the update-flow rules intact. | Non-SW loads and SW revalidations drop to a few hundred bytes | S |
| A1.3 | **Self-host the fonts** as subset `woff2` (Latin only, only the weights used: `--ui`, `--display`, `--mono`), `<link rel="preload">` the display face, `font-display: swap`. Removes 2 third-party origins (DNS + TLS each) from the critical path and makes fonts work offline via the existing `ytplayer-fonts` cache. `fitLyricLines` already re-fits on `document.fonts` load, so late swaps are safe. | 0.3–1 s off first paint on mobile | S |
| A1.4 | **`defer` the scripts** and minify at Docker build time with esbuild `--minify-whitespace --minify-syntax` (**no identifier mangling** — the files share top-level globals as classic scripts). Keep sources unminified in git; `BUILD_TAG` still hashes `./public`. | app.js gzip ~118 → ~80 KB, parse time down | S |
| A1.5 | **Defer non-critical boot work.** `boot()` wires everything synchronously. Move `warmOfflineThumbs`, `preloadPlaylist` sweeps, stats commits, remote/party sockets and the inbox poll behind `requestIdleCallback` (fallback `setTimeout 1500`). Measure with a `performance.mark` around boot first. | Faster time-to-interactive on older phones | M |
| A1.6 | **Lazy-load rare features** (Party, Presenter, Remote, EQ, Share/GIF, the video editor) with `import()` on first use. Needs those sections pulled out of `app.js` into their own files — do only the ones that are large and self-contained. Every new file must be added to the SW `SHELL` list (or offline breaks). | 20–35 % less JS parsed at start | M–L |
## Phase 2 — Security & abuse hardening (P0/P1)
### A2. Search (4–5 s → target < 1.5 s)
| # | Item | Pri | Effort | Notes |
|---|------|-----|--------|-------|
| 2.1 | **Rate-limit expensive routes.** `/api/search`, `/api/channel`, `/api/streams`, `/api/playlist/expand`, `/api/download` each spawn yt-dlp/ffmpeg with no per-IP limit — a trivial loop can exhaust CPU and get the homelab IP bot-gated by YouTube (which already degrades saves to 360p, see BACKLOG). Token bucket per first `X-Forwarded-For` hop, plus a global concurrency cap on yt-dlp spawns with a small queue. | P0 | M | Reuse the IP logic already written for `REMOTE_SAME_NETWORK`. |
| 2.2 | **Profile name enumeration.** Names are the only credential and may be 3 chars (`PROFILE_NAME_RE`), so `/api/profile/load` + `/api/playlist/inbox` are brute-forceable and nothing throttles them. Minimum: strict per-IP limit on 404s from those routes + raise the minimum for *new* profiles to ~10 chars or suggest a generated passphrase. Optional later: an opt-in per-profile PIN (hash stored) that `save` requires. | P0 | S–M | Keep "name = passkey" UX for existing profiles. |
| 2.3 | **Security headers** via `hono/secure-headers`: CSP (self + ytimg/ggpht images + the listed CDNs), `X-Content-Type-Options`, `Referrer-Policy`, `frame-ancestors 'self'`. Start CSP in `Report-Only` for a week — app.js builds a lot of `innerHTML` (124 sites). | P1 | M | Admin page gets a stricter policy. |
| 2.4 | **innerHTML audit**: lint rule / grep gate that every interpolation into `innerHTML` goes through `escapeHtml` (67 call sites today vs 124 innerHTML writes). Convert the risky ones (titles, channel names, chat, notes, profile names from URLs) to `textContent` or a tiny `html\`\`` tagged template that escapes by default. | P1 | M | Party chat + shared playlists carry attacker-controlled text. |
| 2.5 | Body-size limits on every JSON POST (profile already has `PROFILE_MAX_BYTES`; check `/api/user/sync`, notes, party/remote WS frames). | P1 | S | |
| 2.6 | Finish the **cookies jar** backlog item (mount `YTDLP_COOKIES` via a compose volume + an admin-page "cookies expire in N days" warning) — only if 360p fallback saves are noticed. | P2 | S | Already documented in BACKLOG. |
| # | Change | Effort |
|---|--------|--------|
| A2.1 | **Direct InnerTube search.** Call `https://www.youtube.com/youtubei/v1/search` from Bun with `fetch` (WEB client context), map `videoRenderer` items to the existing slim card shape. One HTTPS call instead of starting Python. Keep yt-dlp as the automatic fallback on any parse error / non-200, so a YouTube change degrades to today's speed, not to broken. Add a contract test on a saved response fixture. | M |
| A2.2 | **Suggestions as you type** from `suggestqueries.google.com/complete/search?client=youtube&ds=yt` (proxied + cached server-side, debounced 150 ms client-side), mixed with the existing on-device `RecentSearches`. | S |
| A2.3 | **Start the search on Enter-intent**: fire the request on the debounced input once the query is stable for ~600 ms, so pressing Enter usually hits the 3-min server cache (the ⚡ indicator already exists). | S |
| A2.4 | **Stream results in**: render the library/uploads hits (local DB, instant) immediately, then the YouTube results when they land. | S |
## Phase 3 — Break up `app.js` (P1)
### A3. Pressing play (7.5 s cold → target < 2 s)
9.8k lines in one file is the main drag on every change (and on AI-assisted
edits, which have to re-read huge ranges). The modules are already
IIFE-shaped (`Notes`, `Remote`, `Transition`, `Wave`, `Party`, `Presenter`,
`SectionRail`, `EQ`, …), so the split is mechanical.
| # | Change | Effort |
|---|--------|--------|
| A3.1 | **Coalesce concurrent resolves.** `resolveStreams()` checks `streamCache` but has no in-flight map, so a warm-up + the real request (or two devices) spawn two yt-dlp processes for one id. Add `inflightStreams: Map<id, Promise>`. Prerequisite for A3.2. | S |
| A3.2 | **Warm on intent.** New `GET /api/streams/warm?v=` (204, fire-and-forget, low priority, capped concurrency) that fills `streamCache`. Client calls it: for the top 3 results when a search renders; on `pointerdown`/hover of a card (≥150 ms hover on desktop); for the next 2 queue items when a song starts. Cold plays then hit the 1.2 s cached path. | S–M |
| A3.3 | **Persistent extractor process.** Replace per-call `spawn(yt-dlp)` with a small long-lived Python worker (`scripts/ytdlp-worker.py`) that imports `yt_dlp` once and answers JSON requests over stdin/stdout (or a unix socket). Saves Python start-up, extractor import and — with a persistent `--cache-dir` on the data volume — repeated player-JS / n-challenge work. Keep the spawn path as fallback; restart the worker if it dies or after N requests. Applies to search/channel/expand too. | M |
| A3.4 | **Instant UI on tap.** Show title, thumbnail (already cached by the SW), lyrics and related immediately from the card data while `/api/streams` is pending, with a thin progress bar on the player — no blank player. | S |
| A3.5 | **Fast first frame**: start playback on the lowest adaptive height that looks acceptable on the device (e.g. 360p on phones), then switch up once buffered, instead of waiting on the highest quality's first bytes through the proxy. | M |
1. **Native ES modules, no bundler.** `<script type="module" src="app.js">`,
one file per existing module under `frontend/modules/`. No build step keeps
the Tauri / zero-native shells and the SW precache simple.
2. **Order of extraction** (lowest coupling first): `RecentSearches`, `EQ`,
`PiP`, `Gestures`, `External`, `Share`, `StatsTrack`, `Wave`,
`Transition`, `SectionRail`, `Presenter`, `Remote`, `Party`, `Notes`,
then `Player` and the render/sidebar core last.
3. Shared state (`data`, `queue`, `queueIndex`, `persist()`, `toast()`,
`API`) moves to `frontend/modules/state.js` and is imported, not global.
4. **SW precache list** must be generated, not hand-maintained: have the
server (which already walks `./public` for `BUILD_TAG`) inject the shell
file list into `/sw.js` alongside the tag. Otherwise a new module file
missing from `SHELL` breaks offline — the exact class of bug CLAUDE.md
warns about.
5. Move pure logic out as testable units the way `lyrics-core.js` and
`stats-core.js` already are: queue/shuffle math, A-B marker resolution
(per-playlist vs fallback), party clock extrapolation, sync merge.
6. Same treatment for `server/server.js`: split into `routes/search.js`,
`routes/streams.js`, `routes/profile.js`, `routes/playlist.js`,
`ytdlp.js` (spawn + `runYtdlpResilient`), mirroring how `notes.js` /
`uploads.js` already register their routes.
### A4. Playback throughput & seeking
**Done when:** no frontend file > 1.5k lines, no server file > 600, all
tests + e2e green, SW offline check passes (the real-browser procedure in
CLAUDE.md → Testing).
- **The VPS → homelab WireGuard link carries every media byte** (`/api/play`
proxies googlevideo; the server cache serves from the homelab). Measure it
(`iperf3` over the tunnel) before optimising anything else here.
- If it's the bottleneck: a small **Traefik/nginx cache on the VPS** for
`/api/media/<id>?g=<gen>` — those URLs are immutable by design (the gen is in
the URL), so they can be cached aggressively with Range support
(`slice` module). Hot songs then come straight from the VPS.
- Mark `/api/media/<id>?g=` responses `Cache-Control: public, max-age=31536000,
immutable` so repeat plays on the same device hit the browser cache.
- Waveform peaks: `ytpPeaks` is already local; also send peaks with a long
`max-age` since they're per-gen.
## Phase 4 — Reliability & observability (P1)
### A5. Feels-faster polish
| # | Item | Effort |
|---|------|--------|
| 4.1 | **Structured logs** (JSON lines: route, status, ms, videoId, yt-dlp client used, error class) instead of ad-hoc `console.*`. | S |
| 4.2 | **`/api/health`** that actually checks: DB writable, `MEDIA_DIR` free space, yt-dlp version + age, last successful extraction time, queue depth. Point the Docker healthcheck at it (today it pings `/api/version`, which stays green while yt-dlp is broken). | S |
| 4.3 | **yt-dlp staleness**: the image fetches `latest` only at build time, and "a 2-month-old one 403s on every download". Add a nightly in-container self-update (`yt-dlp -U` into the data volume, fall back to the baked binary) or a scheduled rebuild. | S |
| 4.4 | **Client error reporting**: `window.onerror` + `unhandledrejection` + media element `error` events → throttled `POST /api/client-log` (build tag, UA, route). Surfaces the iOS/HEVC/playback failures that currently only show up when a user complains. Visible in `/admin`. | M |
| 4.5 | **DB backups**: notes already get a daily JSON dump; do the same (or a `VACUUM INTO` copy) for `profiles`, `users`, `uploads`. Document a restore drill. | S |
| 4.6 | **Deploy on merge**: a GitHub Action that calls the Dokploy deploy API (key as a repo secret) after CI passes on `main`, then polls `/api/version` for the new `buildTag`. Replaces the manual SSH hop; keep the `deploy-prod` skill as the manual fallback. | M |
- View Transitions API between Home ↔ playlist ↔ search (progressive; no-op
where unsupported).
- Optimistic playlist edits (add/remove/reorder paint first, sync after — the
sync is already debounced).
- `content-visibility: auto` on long playlist/history lists; virtualise lists
over ~300 rows.
- Image decoding: `loading="lazy" decoding="async"` + fixed aspect-ratio boxes on
every thumbnail (no layout shift while scrolling results).
## Phase 5 — Performance (P1/P2)
### Measuring (do this first, keep it running)
- **First load**: measure with Lighthouse on a throttled mobile profile.
`app.js` + `styles.css` are ~14k lines parsed on every cold start; after the
Phase 3 split, lazy-`import()` the rarely used modules (Party, Presenter,
Remote, EQ, admin-ish editors) on first use.
- **Server search cache** (3 min, just added) → also cache `/api/streams`
metadata (not the signed URLs) and `/api/channel` briefly.
- **CSS**: split per feature alongside the modules; drop dead selectors
(run a coverage pass in Chromium DevTools over the main flows).
- **DB**: check indexes for the hot queries (`video_history` has one;
verify `media_cache.last_access` for LRU, `video_note_revs(video_id, kind)`).
- **Media cache**: expose hit rate in `/api/media/stats` so the 10 GiB budget
and `MEDIA_AUTO_MAX_SECONDS` can be tuned from data.
- Add `performance.mark`s: `boot-start`, `first-render`, `search-submit →
results-rendered`, `play-tap → playing`. Report the medians to a tiny
`POST /api/perf` (sampled, no PII) and show p50/p90 in `/admin`.
- Server-side: log yt-dlp duration per call type, and cache hit rates for
search/streams/media.
- Targets: cold load < 2 s on 4G, repeat load < 0.5 s (SW), search < 1.5 s,
tap-to-sound < 2 s cold / < 1 s warmed.
## Phase 6 — UX & accessibility (P1/P2)
---
- **Accessibility pass** (only ~40 `aria-` attributes across index.html +
styles): labelled icon-only buttons (transport, ↦/⇥/⤨, ⚠ Broken, rail
icons), focus-visible styles, `prefers-reduced-motion`, keyboard access to
drag-reorder, live-region announcements for toasts and track changes.
Add `@axe-core/playwright` to CI on the main screens.
- **Onboarding**: the app now has many power features that are hard to
discover (service mode, presenter, remote, party, transitions, EQ). One
"What's new / Tips" sheet tied to the build tag, dismissible.
- **Settings organisation**: group into Playback · Lyrics & service · Sync &
profile · Offline & storage · Advanced; add search within settings.
- **Offline clarity**: global indicator when offline + which views work,
and a storage screen showing OPFS vs thumb cache vs quota.
- **Profile security UX** (pairs with 2.2): "Your profile name is your
password" hint + generate-a-strong-name button.
## Part B — New features
## Phase 7 — Feature ideas (P2, after 1–4)
Ranked by fit with how the app is actually used (worship sets, sing-alongs,
offline playback). Each is sized; most reuse machinery that already exists.
Ranked by fit with the worship/service use case:
1. **Set-list planning**: date-stamped service plans (songs + key + notes +
order), shareable read-only link for the band, one-tap into service mode.
2. **Chord charts** alongside lyrics (ChordPro import, transpose using the
existing `@ Key G` tag).
3. **Per-song key/tempo playback** (pitch-preserving rate already exists;
add pitch shift via a Web Audio worklet, desktop only because of iOS).
4. **Presenter themes** (background image/video, font, safe-area margins)
and a stage/confidence-monitor view showing the next line.
5. **Multi-user admin** (roles instead of one `ADMIN_PASSWORD`).
### B1. Worship & service
## Phase 8 — Docs & housekeeping (P2)
| Feature | What it is | Reuses | Effort |
|---------|-----------|--------|--------|
| **Service plans** | Date-stamped set lists: songs in order + key, notes, who leads; a read-only share link for the band; one tap opens service mode on it. | playlists, per-song notes, share codes, service mode | M |
| **Chord charts** | ChordPro import/paste, chords shown above lyric lines, **transpose** ± semitones using the existing `@ Key G` tag, capo helper. | lyrics editor + `lyrics-core.js` parser | M–L |
| **Confidence monitor** | Presenter variant for the stage: current line large, next line below, song section and a clock. | Presenter + `fitLyricLines` | S |
| **Presenter themes** | Background colour/image/looping video, font choice, safe-area margins, lower-third mode for livestreams (transparent background for OBS). | Presenter | M |
| **Pitch shift** | Play a song in the band's key: pitch-preserving key change via a Web Audio worklet (desktop/Android; iOS keeps original because of the Web Audio lock-screen issue already documented). | EQ wiring, rate control | M |
| **Count-in & click** | Optional metronome click + 4-beat count-in from the `@ 70 BPM` tag for practice. | lyrics tags, Web Audio | S |
| **Section loops** | Loop a chorus/bridge by tapping a `# Section` in the lyrics (sets A-B from the section's stamps). | A-B loop, lyric sections | S |
- `CLAUDE.md` is excellent but ~400 lines and growing; move per-feature
deep dives to `docs/architecture/*.md` and keep CLAUDE.md as the index +
the "never do X" rules.
- README: quickstart, screenshots, env var table (currently only in compose
comments).
- Retire or clearly quarantine the Tauri/zero-native shells if they're no
longer shipped — every JSON-shape change currently has to consider them.
- Version: `APP_VERSION` is hard-coded `1.0.0` in three places; derive it
from one source or drop it in favour of `buildTag`.
### B2. Listening & discovery
| Feature | What it is | Effort |
|---------|-----------|--------|
| **Radio / autoplay mix** | When the queue ends, keep going with related videos weighted by your play counts and skips. | M |
| **Smart playlists v2** | Rules like "played ≥ 5× in 30 days", "saved offline", "has synced lyrics", "under 5 min". `SMART_PLAYLISTS` already exists — make them user-defined. | S–M |
| **Stats wrap-up** | Monthly/yearly recap card (top songs, minutes, streak) from `data.stats`, shareable as an image. | S |
| **Sleep/wake alarm** | Start a playlist at a set time (while the app is open / PWA foreground). | S |
| **Lyrics search** | Search across all saved songs' lyrics ("which song has *'goodness of God'*"). Server-side over `video_notes`. | S |
### B3. Offline & library
| Feature | What it is | Effort |
|---------|-----------|--------|
| **Download manager** | One screen: queued/active/done saves with progress, retry, pause-all, storage used per playlist. | M |
| **Auto-offline favourites** | Keep the N most-played songs saved automatically, evict the least-played. | S |
| **Uploads for everyone** | Let profile users (not just admin) upload audio to their own library, with a per-profile quota. | M |
### B4. Together
| Feature | What it is | Effort |
|---------|-----------|--------|
| **Collaborative playlists** | A playlist several profiles can edit (last-write-wins per entry, change feed). | M–L |
| **Party queue voting** | Guests suggest songs; host approves or upvotes decide order. | M |
| **Reactions in party** | Lightweight emoji reactions over the video/lyrics. | S |
---
## Suggested order
1. Week 1: 1.1, 1.2, 1.3, 2.1, 2.2, 4.2, 4.3
2. Week 2: 1.4, 1.5, 2.5, 4.1, 4.4, 4.5
3. Weeks 3–5: Phase 3 split (server first — smaller and better tested),
with 2.3/2.4 riding along as modules move
4. Then: 4.6 auto-deploy, Phase 5 perf, Phase 6 a11y/UX
5. Features (Phase 7) once CI + the split are in place
1. **Week 1 — quick speed wins:** add the timing marks (Measuring), A1.1
compression, A1.2 ETags, A1.3 fonts, A3.1 coalescing, A3.2 warm-on-intent,
A3.4 instant UI. These are all small, and together they fix most of what
feels slow today.
2. **Week 2:** A2.1 InnerTube search + A2.2 suggestions, A1.4 defer/minify.
3. **Week 3:** A3.3 persistent yt-dlp worker, A1.5 deferred boot work, then
measure the WireGuard link (A4) and decide on the VPS media cache.
4. **Then features**, starting with the small high-fit ones: section loops,
confidence monitor, count-in, lyrics search, stats wrap-up — then
service plans and chord charts.
Each item is sized to be one commit per CLAUDE.md's commit rules, and most
can be queued with the `plan-queue` skill for step-by-step execution.
Each row is sized to be one commit (CLAUDE.md commit rules) and can be queued
with the `plan-queue` skill.