feat(audit): Batch 14 — perf (code-split + seed DCE) + first-run default

- PERF8: code-split the non-default tabs. Library/History/Terminal/Settings are
  React.lazy chunks behind a Suspense; Downloads (the launch tab) stays eager. The
  production build now emits separate SettingsView (~90kB), LibraryView (~29kB),
  HistoryView (~16kB), TerminalView (~8kB) chunks — ~140kB out of the initial
  bundle. The stores these views use are imported eagerly elsewhere, so lazy-
  loading the views never delays store startup/persistence (per the C2 note).
- L128: gate the three preview seed arrays (downloadSeed / sources seeds / history
  seed) on `import.meta.env.DEV && PREVIEW`. In the packaged build DEV is
  statically false, so Rollup constant-folds and dead-code-eliminates the seed
  literals; the browser dev preview (DEV true) still gets them.
- SR4: clipboardWatch now defaults false for new installs (privacy — no clipboard
  read on focus until opted in). Existing installs keep their saved value
  (electron-store fills only missing keys); the onboarding tip points to the
  setting. NOTE: this flips a signature convenience off-by-default for new users —
  flagged for the maintainer, trivially reversible.

Deferred with rationale (CODE-AUDIT.md): PERF3 (core-queue micro-opt on a bounded
list, risky unattended), PERF6/L162 (bounded by virtualization + conflicts with
PERF5's hoisted renderItem), SR5 (MP3 = deliberate compatibility default; the label
bug was M18), W12 (fallback-of-a-fallback icon), W15 (GPU software-rendering
workaround — external driver dependency), L104 (History virtualization needs the
L161 layout rework + live verification; bounded by the 500-cap).

Verified: typecheck (node+web) + 279 tests + eslint + production build green
(code-split chunks confirmed in output); touched files prettier-clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-07-01 17:35:40 -04:00
parent 71f0b2b68f
commit 0f784847a3
7 changed files with 236 additions and 148 deletions
+72 -12
View File
@@ -211,6 +211,21 @@ L109 (visual, with UX26), L161 (layout, live-verify), UI21 (visual), UI23 (cooki
live-verify). All green: typecheck (node+web) + 279 tests + eslint + production build; touched files
prettier-clean.
**Session 2026-07-01 pass 11 (Batch 14 — perf + first-run defaults + Windows):** two real bundle/startup wins
plus one first-run default. **PERF8** — Library/History/Terminal/Settings are `React.lazy` chunks behind a
`Suspense` (Downloads stays eager); the production build now splits ~140 kB (incl. the 90 kB SettingsView) out
of the initial bundle. **L128** — the three preview seed arrays are gated on `import.meta.env.DEV && PREVIEW`,
so Rollup dead-code-eliminates them from the packaged renderer (DEV statically false) while the browser dev
preview keeps them. **SR4**`clipboardWatch` now defaults `false` for new installs (privacy-first; existing
installs keep their saved value; Onboarding tip repointed) — *a product default flip flagged for the
maintainer, trivially reversible.* **Deferred with rationale:** **PERF3** (core-queue micro-opt on a bounded
list — risk > reward unattended, revisit in the Batch 15 live pass), **PERF6/L162** (bounded by
virtualization; a prop conflicts with PERF5's hoisted renderItem), **SR5** (MP3 = deliberate
max-compatibility default; the label bug was M18), **W12** (fallback-of-a-fallback icon), **W15** (the GPU
software-rendering workaround — external driver dependency), and **L104** (History virtualization needs the
L161 layout rework + live verification; the 500-cap bounds it). All green: typecheck (node+web) + 279 tests +
eslint + production build; touched files prettier-clean.
**Deliberately deferred** (need live-app/visual verification or are larger refactors the
audit itself defers to 1.x): the wire-or-cut features (M5/M6/UX1), the god-file/store
refactors (C1, C2, H1), the SIMP* helpers + shared-UI-token work, the CC* consolidations,
@@ -916,9 +931,12 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
remaining `<Spinner>`s are standalone status-row indicators (the "Fetching…"/"Waiting to start…"/indexing
rows), not a button + adjacent spinner. No separate-adjacent-spinner pattern survives.*
- [ ] **L104 — History isn't virtualized.** It renders all filtered rows, while Downloads and Library use `VirtualList`.
*Moved to Batch 14 (perf): virtualizing History is a rendering-performance change (it belongs with the other
perf items — PERF3/L162), not copy/cleanup. Capped at 500 entries (L141), so the current cost is bounded but
worth virtualizing for consistency; done in the perf batch.*
*Deferred (bounded; needs the L161 layout rework + live verification): History is capped at 500 rows (L141),
so the un-virtualized render is bounded — not a real perf problem. Virtualizing it requires giving the list a
fixed-height internal scroll container (the same `height:100%`/`overflow` rework L161 flags as needing
live verification at multiple window sizes) *and* re-homing its container-level Ctrl+A and per-row Delete
keyboard handling onto the virtualized scroller. That's a layout+interaction change best done watched, so it
rides with L161 rather than shipping unattended for a list that's already capped.*
- [x] **L105 — `MediaKind` declared twice.** In both [ipc.ts](src/shared/ipc.ts) and
[store/downloads.ts](src/renderer/src/store/downloads.ts); components import it from both. Single source it.
*Fixed: the store now imports `MediaKind` from `@shared/ipc` and re-exports it, so the duplicate local
@@ -1018,11 +1036,14 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
(the header is now `title`), and the shared `Banner` content (UI19). `title` also gained `minWidth:0` so it
shrinks correctly in a flex row. (Remaining correct-but-inline truncations migrate to the util incrementally.)*
- [x] **L127 — `Notification` sets no icon** — completion toasts used the default Electron icon though `getAppIconPath()` existed. *Fixed with W13 (cached `getAppIconImage()` passed as `icon` at both notification sites).*
- [ ] **L128 — Preview seed/mock data ships in production.** The `PREVIEW` runtime check can't be
- [x] **L128 — Preview seed/mock data ships in production.** The `PREVIEW` runtime check can't be
tree-shaken, so each store's seed arrays + fake ticker are bundled into the Electron renderer.
*Deferred to Batch 14 (perf/bundle): this is a build-time dead-code-elimination concern (gate the seed
imports behind `import.meta.env` so Rollup drops them from the packaged renderer), grouped with PERF8's
code-splitting where the lazy-loading is set up. Left open on purpose.*
*Fixed: the three preview seed arrays (`downloadSeed`, `sources` `seedSources`/`seedItemsBySource`,
`history` seed) are now gated on `import.meta.env.DEV && PREVIEW`. In the packaged renderer build
`import.meta.env.DEV` is statically `false`, so Rollup constant-folds `false && …` and dead-code-eliminates
the whole seed literal; the browser dev preview (`vite`, DEV `true`) still gets them. (The small `PREVIEW`
fake-ticker function stays — it's tied to the runtime flag and is a few lines; the bulky part was the data,
now dropped.)*
- [x] **L129 — `Hint align` is silently ignored for left/right placements** yet callers still pass it.
*Fixed: `HintProps` is now a discriminated union — `align` is only accepted with `placement` `top`/`bottom`;
passing it with `left`/`right` is a compile error instead of a silent no-op. No caller needed changing (the
@@ -1189,7 +1210,9 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
a layout change best validated with the app open at several window sizes — a watched pass, not an unattended
edit.*
- [ ] **L162 — Per-thumbnail store subscription.** Every `MediaThumb` calls `useResolvedDark()` (two store
subscriptions) instead of receiving a resolved theme prop — N subscriptions per list.
subscriptions) instead of receiving a resolved theme prop — N subscriptions per list. *Deferred — same as
PERF6: virtualization/the 500-cap bounds the mounted count, and a prop conflicts with PERF5's hoisted
`renderQueueRow`; documented there.*
- [x] **L163 — QueueItem makes 9 separate `useDownloads` selector calls** instead of one destructured
selection — repeated boilerplate per row. *Fixed: one `useShallow` selection replaces the 10 individual
subscriptions in [QueueItem.tsx](src/renderer/src/components/QueueItem.tsx) (the actions are stable, so the
@@ -1708,6 +1731,10 @@ DPI handled by Chromium). The deviations:
- [ ] **W12 — Fallback tray icon is single-resolution.** The embedded fallback is a 32×32 PNG, so on
150%/200% DPI the tray glyph is blurry when the real multi-size `.ico` is absent. **Standard:** a
multi-size fallback (or rely only on the `.ico`).
*Deferred (low value, needs the real tray to judge): the fallback only renders in the unlikely case the
bundled multi-size `build/icon.ico` is missing (W14 regenerated it), so it's a fallback-of-a-fallback. A
multi-res embedded base64 is fiddly to author and best verified on a real high-DPI tray — not worth an
unattended change for a path that shouldn't be hit.*
- [x] **W13 — Notifications set no icon.** `new Notification({title, body})` omitted `icon`; on the **portable**
build (no installed AUMID shortcut) Windows toasts showed a generic icon. *Fixed: both notification sites
([download.ts](src/main/download.ts) completion/failure toast + [index.ts](src/main/index.ts) background-running
@@ -1724,6 +1751,11 @@ DPI handled by Chromium). The deviations:
- [ ] **W15 — Software rendering is global.** Hardware acceleration is disabled for a documented GPU
workaround; on high-DPI/large windows this trades GPU compositing for CPU, which can be sluggish.
**Standard:** re-enable HW accel where the target GPU allows (the memo already flags revisiting this).
*Deferred (deliberate workaround, external dependency): `disableHardwareAcceleration()` is the fix for the
dev machine's GPU/driver blank-overlay bug (documented in index.ts and the project memo) — the very reason
the app hand-rolls non-portal overlays. Re-enabling HW accel can't be validated until the NVIDIA driver
issue is confirmed resolved on the target GPU, so it's intentionally left until that watched hardware pass.
PERF8 (this batch) trims the first-paint cost that software rendering compounds.*
- [x] **W16 — Touch targets too small / hover-only labels.** Icon-only row actions are ~32px, 4px apart,
4 per row (below the ~40px touch guideline), and their only labels are hover tooltips (`Hint`) that
never appear on touch. **Standard:** ≥40px touch targets and non-hover labels. *(extends UX17)* *Fixed:
@@ -2183,11 +2215,23 @@ the end so the picture is complete. **Verified-clean first:** card/label/button
Defensible for chasing YouTube changes, but a risky default for a commercial release. **Fix:** consider `stable` as the shipped default.
- [x] **SR3 — `autoDownloadNew` defaults to `true`.** The moment a user marks a channel "watched," new
uploads auto-download with no per-event consent — can quietly fill a disk. **Fix:** default off (let watching be "notify," downloading be opt-in).
- [ ] **SR4 — `clipboardWatch` defaults to `true`.** The app reads the clipboard on every window focus from
- [x] **SR4 — `clipboardWatch` defaults to `true`.** The app reads the clipboard on every window focus from
first launch (R7/L138). A privacy-conscious default would be off or first-run-prompted. **Fix:** default off, or ask on first run.
- [ ] **SR5 — Audio defaults to MP3 (lossy).** `defaultAudioQuality: 'Best (MP3)'` + `audioFormat: 'mp3'`
*Fixed: `DEFAULT_SETTINGS.clipboardWatch` is now `false` ([shared/ipc.ts](src/shared/ipc.ts)) — a new install
doesn't read the clipboard until the user opts in (a one-toggle setting), and the Onboarding tip was reworded
to point there ("Turn on clipboard detection in Settings…"). Existing installs keep their saved value
(electron-store only fills missing keys), so no one who already enabled it is affected. **Product note for
the maintainer:** this flips a signature convenience off-by-default for new users — trivially reversible if
you'd rather it stay on.*
- [x] **SR5 — Audio defaults to MP3 (lossy).** `defaultAudioQuality: 'Best (MP3)'` + `audioFormat: 'mp3'`
give a lossy re-encode by default; m4a/opus are higher quality at the same size. Minor (MP3 = max
compatibility), but "Best" implying MP3 is a quality mismatch (M18).
compatibility), but "Best" implying MP3 is a quality mismatch (M18).
*Resolved as a deliberate default (documented): MP3 is the maximum-compatibility container (plays on every
device/player), which is the right *default* for a general-audience downloader — a user who wants
m4a/opus/flac picks it in the format options. The "Best" *label* mismatch (the real bug) was fixed in M18
(the preset labels and `--audio-quality` handling now match the chosen format). Changing the shipped default
container away from MP3 is a product-taste call with a real compatibility downside, so it stays MP3 by
intent, not oversight.*
### Stuck states & progress glitches
@@ -2242,6 +2286,11 @@ acceptable first:** memoized `QueueItem`s don't re-render when their item is unc
on every add/done/error/cancel/progress-driven change; with M33's unbounded channel enqueue (thousands of
items) every event becomes O(n), and several events fire per second during active downloads. **Fix:**
track running/queued counts + a pointer instead of rescanning.
*Deferred (core queue, risk > reward unattended): M33 added the enqueue cap, so `items` is bounded (a few
thousand at most), and an O(n) scan over that a few times per second is negligible in practice. Rewriting
`pump()` to a count+pointer model touches the app's central concurrency logic — exactly the download-engine
code that Batch 15 verifies live — so it's not worth the regression risk for a micro-optimization on a
bounded list. Revisit with the Batch 15 live pass if profiling ever flags it.*
- [x] **PERF4 — `summarizeQueue` runs twice per progress tick.** Once in App's store subscription (which
also pushes a taskbar IPC every tick, L25) and once in DownloadsView's render (no `useMemo`, L94) — each
O(items), every ~second per active download. **Fix:** compute once, memoize, throttle the taskbar push.
@@ -2254,11 +2303,22 @@ acceptable first:** memoized `QueueItem`s don't re-render when their item is unc
stable module-level functions in [DownloadsView.tsx](src/renderer/src/components/DownloadsView.tsx).*
- [ ] **PERF6 — Per-thumbnail store subscriptions.** Each `MediaThumb` calls `useResolvedDark()` (two
store subscriptions) (L162); a long list creates N×2 subscriptions. **Fix:** resolve once, pass as a prop.
*Deferred (bounded + a conflicting tradeoff): the lists that use `MediaThumb` are virtualized (Downloads
queue, Library) or capped (History ≤500), so only the ~visible rows mount — N is small and each
`useResolvedDark` is a cheap memoized boolean selector. Threading `isDark` as a prop would also fight PERF5
(the hoisted, stable `renderQueueRow`, which would have to close over `isDark`), and moving the hook up to
`QueueItem` yields the same one-subscription-per-row. Low value, mild conflict — left as documented.*
- [x] **PERF7 — (ref R3) O(n²) media-items rewrites** dominate the main-process cost when downloading a
channel; closed by the cached/atomic `jsonStore` (SIMP1) — the single highest-value perf fix.
- [ ] **PERF8 — No code-splitting.** All views (incl. the rarely-used Terminal/Settings) load up front in
- [x] **PERF8 — No code-splitting.** All views (incl. the rarely-used Terminal/Settings) load up front in
one renderer bundle alongside Fluent UI + React; software rendering (W15) compounds first-paint cost.
Minor for a desktop app, but lazy-loading heavy views would trim startup. **Fix:** `React.lazy` the heavy tabs.
*Fixed: Library/History/Terminal/Settings are `React.lazy` chunks behind a `Suspense` in
[App.tsx](src/renderer/src/App.tsx) (Downloads, the launch tab, stays eager). The production build now emits
separate `SettingsView` (~90 kB), `LibraryView` (~29 kB), `HistoryView` (~16 kB), and `TerminalView`
(~8 kB) chunks — ~140 kB out of the initial bundle, loaded on first visit. The stores these views use are
imported eagerly elsewhere (App's `useDownloads` + `import './store/sources'`, and downloads → history), so
lazy-loading the *view* never delays store startup or persistence (the C2 note already set this up).*
**Priority:** PERF7/R3 (scaling cliff) and PERF1/PERF2 (per-download redundant I/O + crypto) are the only
ones with real user-visible impact; the rest are good hygiene. None warrant work before the correctness