From a0ffc1b493bc597be0c91c425c8fdf5bb4a90af4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 17:40:57 +0000 Subject: [PATCH] Refocus the improvement plan on speed and new features --- docs/improvement-plan.md | 254 ++++++++++++++++++++------------------- 1 file changed, 130 insertions(+), 124 deletions(-) diff --git a/docs/improvement-plan.md b/docs/improvement-plan.md index 4275df7..129a0a9 100644 --- a/docs/improvement-plan.md +++ b/docs/improvement-plan.md @@ -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`), `` 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`. 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.** `