Fix H5/H6/M21/L37-40/L47/L55/L68/L72/L76/L91/L98/L108/L165/L166 audit items
H5: HistoryEntry now stores formatId+formatHasAudio; redownload passes them
back to addFromUrl so the original yt-dlp format is reused, not guessed.
Compound quality labels stripped to the preset prefix for old history.
L72: thumbnail now passed in redownload so re-queued rows keep their art.
H6: TerminalView log capped at MAX_LOG_LINES=2000 to prevent unbounded growth.
M21: --audio-quality omitted for lossless audioFormats (flac/wav).
L166: fmtEta hour rollover fixed in all 3 locations -- 2h00:00 not 120:00.
L165: queueStats formatSpeed precision aligned with fmtBytes.
L55: QueueItem suppresses sizeLabel when already in probed-format quality label.
L47: probeMeta null sends empty meta event so Resolving clears via SR6.
L68: notifiedBackground resets on window show for subsequent close-while-downloading.
L76: dup warning falls back to URL when title is still a placeholder.
L91: SettingsView onFormatSelect uses indexOf instead of unsafe tuple cast.
L98: Embed chapters field gets a hint text.
L108: BrowserWindow created with explicit title AeroFetch.
L37: pure formatters extracted to src/main/lib/formatters.ts and unit-tested.
L38: buildArgs test covers webm thumbnail suppression.
L39: CROP_SQUARE_PPA test is structural not exact-string.
L40: looksLikeUrl/looksLikeSingleVideo extracted to src/renderer/src/lib/urlHelpers.ts.
typecheck + 242 tests + eslint + prettier green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+30
-17
@@ -20,6 +20,19 @@ L11 (Queue header counts the live queue), L50 (no "saved" for 0 cookies), L156 +
|
||||
reconciliation (M25 M26 L80 L81), and new unit tests (L35 isValidMediaItem, L36
|
||||
compareVersions). All verified: typecheck + 234 tests + eslint + prettier green.
|
||||
|
||||
**Session 2026-06-30 pass 2 (quality/format/UX polish):** H5 (history re-download now stores
|
||||
formatId/formatHasAudio + strips compound quality labels; thumbnail also passed — L72), H6
|
||||
(TerminalView log capped at 2000 lines), M21 (skip --audio-quality for lossless flac/wav),
|
||||
L166 (fmtEta hour rollover fixed in download.ts, store/downloads.ts, queueStats.ts — "2:00:00"
|
||||
not "120:00"), L165 (formatSpeed precision aligned with fmtBytes), L55 (QueueItem suppresses
|
||||
sizeLabel when already in probed-format quality label), L47 (probeMeta null sends empty meta
|
||||
event to clear "Resolving…"), L68 (notifiedBackground resets on window show), L76 (dup warning
|
||||
falls back to URL when title is placeholder), L91 (SettingsView unsafe tuple cast fixed), L98
|
||||
(embed-chapters hint added), L108 (window title set), L37 (formatters extracted to lib/formatters.ts;
|
||||
fmtBytes/fmtEta/parseProgress unit-tested including NA paths), L38 (webm thumbnail exclusion
|
||||
test), L39 (CROP_SQUARE_PPA structural test), L40 (looksLikeUrl/looksLikeSingleVideo extracted
|
||||
to lib/urlHelpers.ts; test imports pure module directly). All verified: typecheck + 242 tests + eslint + prettier green.
|
||||
|
||||
**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,
|
||||
@@ -183,11 +196,11 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
with H2's shared formatter.
|
||||
- [ ] **H4 — Carry raw numbers across the progress boundary.** `DownloadProgress` ships
|
||||
formatted `speed`/`eta` strings; `queueStats.ts` re-parses them back to numbers.
|
||||
- [ ] **H5 — History re-download silently changes quality.** `HistoryView.redownload` passes the
|
||||
- [x] **H5 — History re-download silently changes quality.** `HistoryView.redownload` passes the
|
||||
stored `quality` (for format-picker downloads this is a *label* like "720p · mp4 · 184 MB")
|
||||
back as `quality` with no `formatId`; `buildArgs.videoFormat()` has no matching case and falls
|
||||
back to `bv*+ba/b` (Best). The queued item shows "720p…" while actually fetching Best.
|
||||
- [ ] **H6 — TerminalView log grows unbounded.** `setLines((ls) => [...ls, …])` with no cap; a
|
||||
- [x] **H6 — TerminalView log grows unbounded.** `setLines((ls) => [...ls, …])` with no cap; a
|
||||
verbose run (`-F`, `--verbose`) streams thousands of lines into React state. Every other
|
||||
log/list in the app is capped — cap this too (and/or virtualize).
|
||||
- [x] **H7 — `cookies.txt` written in plaintext with no restrictive perms.** [cookies.ts](src/main/cookies.ts)
|
||||
@@ -269,7 +282,7 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
- [ ] **M20 — ProgressBar / native `<select>` accessibility.** QueueItem/DownloadsView
|
||||
`ProgressBar`s have no accessible name; several `Select`s carry both a `<Field label>` and an
|
||||
`aria-label` (double announcement).
|
||||
- [ ] **M21 — `--audio-quality` always emitted.** `buildArgs` passes `--audio-quality` even for
|
||||
- [x] **M21 — `--audio-quality` always emitted.** `buildArgs` passes `--audio-quality` even for
|
||||
lossless `audioFormat` (flac/wav), where it's meaningless; pair with M18's preset/format mismatch.
|
||||
- [x] **M22 — Backup export writes secrets in cleartext, silently.** [backup.ts](src/main/backup.ts)
|
||||
`exportBackup` serializes the *decrypted* settings (proxy creds, PO token, update token) to a
|
||||
@@ -443,10 +456,10 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
regression for the actual yt-dlp argv path (only the pure builder).
|
||||
- [x] **L35 — `isValidMediaItem` has no test** — the one persisted-JSON validator of six with no spec.
|
||||
- [x] **L36 — `compareVersions` differing-length components untested** (e.g. `1.2` vs `1.2.0`).
|
||||
- [ ] **L37 — `download.ts` formatters untested** (`parseProgress`/`fmtBytes`/`fmtEta`, incl. the `NA` paths).
|
||||
- [ ] **L38 — `buildArgs` webm-thumbnail exclusion branch untested** (`embedThumbnail && container!=='webm'`).
|
||||
- [ ] **L39 — `CROP_SQUARE_PPA` exact-string assertion** is a change-detector test (breaks on harmless reformat).
|
||||
- [ ] **L40 — `clipboardLink.test.ts` imports the zustand settings store** to test two pure helpers
|
||||
- [x] **L37 — `download.ts` formatters untested** (`parseProgress`/`fmtBytes`/`fmtEta`, incl. the `NA` paths).
|
||||
- [x] **L38 — `buildArgs` webm-thumbnail exclusion branch untested** (`embedThumbnail && container!=='webm'`).
|
||||
- [x] **L39 — `CROP_SQUARE_PPA` exact-string assertion** is a change-detector test (breaks on harmless reformat).
|
||||
- [x] **L40 — `clipboardLink.test.ts` imports the zustand settings store** to test two pure helpers
|
||||
(`looksLikeUrl`/`looksLikeSingleVideo`) — couples a pure-fn test to store init (see H2).
|
||||
- [x] **L41 — `package.json` `homepage` points at yt-dlp's GitHub**, not AeroFetch — wrong product
|
||||
URL (surfaced by the NSIS installer).
|
||||
@@ -458,7 +471,7 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
builder `copyright` "yt-dlp frontend" vs Sidebar caption "yt-dlp frontend".
|
||||
- [ ] **L46 — yt-dlp-missing error copy differs across 4 modules** (download/ytdlp/probe/indexer):
|
||||
"Reinstall AeroFetch, or drop…" vs "Download it into resources/bin/…" vs "Drop it into resources/bin/."
|
||||
- [ ] **L47 — `probeMeta` 30s timeout returns null silently** — no surfaced message, unlike every
|
||||
- [x] **L47 — `probeMeta` 30s timeout returns null silently** — no surfaced message, unlike every
|
||||
other yt-dlp timeout ("Timed out …").
|
||||
- [ ] **L48 — `cleanError` strips a leading `error:`** for live failures, while raw `ERROR:` text
|
||||
shows elsewhere (errorlog seed / terminal) — inconsistent error normalization.
|
||||
@@ -471,7 +484,7 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
- [ ] **L53 — `dialog.show*Dialog(win!, …)` non-null assertions** on a possibly-null window
|
||||
(chooseFolder, backup export/import) — can throw if the sender window is gone.
|
||||
- [ ] **L54 — DownloadBar preview `<img>` has no `onError` fallback** (inconsistent with `MediaThumb`'s retry).
|
||||
- [ ] **L55 — QueueItem meta line can show size twice** — a probed-format `quality` label already
|
||||
- [x] **L55 — QueueItem meta line can show size twice** — a probed-format `quality` label already
|
||||
contains the size, then `sizeLabel` is appended again.
|
||||
- [ ] **L56 — SponsorBlock ON with zero categories silently no-ops** (`buildArgs` guards on
|
||||
`length > 0`) with no UI warning.
|
||||
@@ -489,7 +502,7 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
- [ ] **L65 — Pause uses `taskkill /F`** like cancel — a forced kill risks an unflushed `.part` tail vs a graceful stop.
|
||||
- [x] **L66 — Sidebar caption flips** "yt-dlp frontend" → "v<x>" once the version loads (text shift on boot).
|
||||
- [ ] **L67 — Onboarding has no Skip and can't be revisited** (no "show tips again").
|
||||
- [ ] **L68 — Background-running notification fires once per process** (`notifiedBackground` never
|
||||
- [x] **L68 — Background-running notification fires once per process** (`notifiedBackground` never
|
||||
resets) — won't remind on later window closes.
|
||||
- [ ] **L69 — `settings.ts` mixes path helpers with persistence** (`getDefaultMediaDir`/
|
||||
`ensureMediaDirs`/`getDownloadArchivePath` alongside the store) — split a `paths.ts`.
|
||||
@@ -499,13 +512,13 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
`crypto` to exercise it.*
|
||||
- [ ] **L71 — Settings folder inputs are `readOnly`** with only a Browse button — no way to paste a
|
||||
known path.
|
||||
- [ ] **L72 — History re-download drops the thumbnail** (passes only title/channel), so the
|
||||
- [x] **L72 — History re-download drops the thumbnail** (passes only title/channel), so the
|
||||
re-queued row loses its thumbnail until re-probed.
|
||||
- [ ] **L73 — Two verbs for the same probe action** — DownloadBar "Fetch" vs LibraryView "Index".
|
||||
- [ ] **L74 — Accent swatches are triple-labeled** (`aria-pressed` + `aria-label` + `title`).
|
||||
- [ ] **L75 — Update-token field always visible** in the Software-update card, even when no update
|
||||
is pending — buries an advanced/rarely-needed input.
|
||||
- [ ] **L76 — Duplicate-warning may show a placeholder title** — `setDup(existing.title)` can be the
|
||||
- [x] **L76 — Duplicate-warning may show a placeholder title** — `setDup(existing.title)` can be the
|
||||
`titleFromUrl` placeholder before metadata resolves ('YouTube video (id)').
|
||||
- [ ] **L77 — ffmpeg/ffprobe versions have no re-check affordance** (load once; "not found" sticks
|
||||
with no retry) unlike the yt-dlp "Check version" button.
|
||||
@@ -544,7 +557,7 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
|
||||
- [ ] **L90 — Two idioms for "value in const array."** [settings.ts](src/main/settings.ts) `sanitizeOptions`
|
||||
uses `ARR.includes(x as never)` (L116–120) while `setSettings` uses `(ARR as readonly string[]).includes(x as string)` (L275/323). Pick one.
|
||||
- [ ] **L91 — Unsafe tuple cast / fragile encoding.** `value.split('|') as [MediaKind, string]`
|
||||
- [x] **L91 — Unsafe tuple cast / fragile encoding.** `value.split('|') as [MediaKind, string]`
|
||||
([SettingsView.tsx](src/renderer/src/components/SettingsView.tsx):411) trusts a `'|'`-joined
|
||||
`kind|quality` string; a value without `'|'` yields `undefined` quality. Use a typed pair, not a string.
|
||||
- [ ] **L92 — Preload names diverge from main fn names.** `markSourceItemDownloaded`↔`setMediaItemDownloaded`,
|
||||
@@ -559,7 +572,7 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
`…` (opens a dialog), but "Browse" / "Check for updates" / "Update now" don't. Adopt the `…` = opens-a-dialog convention.
|
||||
- [ ] **L97 — Advanced-field placeholders inconsistent.** Adjacent fields use an example (`web_safari`),
|
||||
a literal `(none)`, and `Optional — Gitea access token`. Pick one placeholder style.
|
||||
- [ ] **L98 — "Embed chapters" has no hint** while every sibling toggle in DownloadOptionsForm does.
|
||||
- [x] **L98 — "Embed chapters" has no hint** while every sibling toggle in DownloadOptionsForm does.
|
||||
- [ ] **L99 — URL input has an `id` but no label.** `input={{ id: 'aerofetch-url' }}` with no
|
||||
`<label for>`/`aria-label` (placeholder only) — see M28.
|
||||
- [ ] **L100 — Inconsistent elevation.** DownloadBar (`shadow4`) and CommandPalette (`shadow28`) float;
|
||||
@@ -577,7 +590,7 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
- [ ] **L106 — Refresh icons used interchangeably.** `ArrowClockwiseRegular` (retry, re-download,
|
||||
check-for-new, update-yt-dlp) vs `ArrowSyncRegular` (re-index, check-for-updates) for the same "redo/refresh" idea.
|
||||
- [ ] **L107 — Spinner sizes mixed** — `"tiny"` almost everywhere, `"extra-tiny"` for the QueueItem queued row.
|
||||
- [ ] **L108 — Main window sets no explicit `title`** (relies on the document `<title>`).
|
||||
- [x] **L108 — Main window sets no explicit `title`** (relies on the document `<title>`).
|
||||
- [ ] **L109 — Versions auto-load with no indicator.** yt-dlp/ffmpeg versions in About just appear;
|
||||
only the manual "Check" button shows a spinner.
|
||||
- [ ] **L110 — One muted-text token, many class aliases.** `colorNeutralForeground3` is re-declared as
|
||||
@@ -699,10 +712,10 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
(2-space bullet) in QueueItem/History/DownloadBar/DownloadsView, with `' · '` (1-space midd·dot) in
|
||||
LibraryView, and with `' • '` (1-space bullet) in QueueItem's "Paused • 42%". Three styles for one role.
|
||||
**Standard:** one separator constant (glyph + spacing).
|
||||
- [ ] **L165 — The two byte/speed formatters round differently.** `download.ts fmtBytes` uses
|
||||
- [x] **L165 — The two byte/speed formatters round differently.** `download.ts fmtBytes` uses
|
||||
`toFixed(v >= 100 ? 0 : 1)` ("512 MB") while `queueStats formatSpeed` uses `toFixed(1)` ("512.0 MB/s") —
|
||||
visibly different precision for the same magnitudes (compounds the duplicate-formatter issue H2/M9).
|
||||
- [ ] **L166 — ETA has no hour rollover.** `fmtEta` (download.ts/downloads.ts) and `formatEta`
|
||||
- [x] **L166 — ETA has no hour rollover.** `fmtEta` (download.ts/downloads.ts) and `formatEta`
|
||||
(queueStats) emit `M:SS` only, so a 2-hour ETA renders as **"120:00"**, whereas `fmtDuration`
|
||||
(indexerCore) correctly rolls to `H:MM:SS`. Unify on the hour-aware format.
|
||||
- [x] **L167 — `PROGRESS_TEMPLATE` is exported but used only inside `buildArgs.ts`** — a superfluous
|
||||
|
||||
Reference in New Issue
Block a user