Both shipped (ROADMAP.md Phase L/P) but CODE-AUDIT.md's Deferred section still listed them as pending. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
225 KiB
Code Audit
Living checklist of audit findings for AeroFetch. Security & correctness were reviewed 2026-06-23 (all fixed — see Completed). The architectural review (2026-06-29) added the structural items; a second polish pass (2026-06-29) added the smaller inconsistencies. Items carry stable IDs so we can check them off as they land this session.
Severity = structural leverage / risk of future bugs, not "app is broken" (it isn't).
Session 2026-06-30 (correctness/safety/strictness pass): landed B3 B4 B5 B7 (checksum-
filename match, newline-safe meta probe, canceled-event guards, cookie-login never-resolve),
L88 L146 (queue/trim edge cases), L168 + L169 (noUncheckedIndexedAccess +
noFallthroughCasesInSwitch now on — 0 errors after 8 real edge-case fixes), R5 (settings
write failure handled), W1 W5 W6 (min window size, seeded folder picker, parented sign-in
window), CL1 (shared stdout markers), L147 (dead macOS branches removed), M8 (StatusChip
already unified the status→label map), M36 (Library selection only counts actionable rows),
L11 (Queue header counts the live queue), L50 (no "saved" for 0 cookies), L156 + L57
(schedule picker min), L159 L15 L3 (dead-code/comment cleanup), package/builder metadata
(L4 L41 L42 L43 L45 L58), user-facing copy (L66 L154; partial M37/SR9 jargon), doc
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.
Session 2026-06-30 pass 3 (reliability — B/R/L races & silent failures): L140 + L148 (the active-map
teardown race — cancelDownload/pauseDownload release the concurrency slot synchronously and an
identity-guarded releaseActive + per-spawn cookie jar make same-id retry/resume safe), L141 (renderer
history capped at 500 + url de-dup to mirror main), R6 (writeJsonAtomic reports + logs write failures and
keeps the data dirty for retry instead of an empty catch — log half; toast → CC8), R7 (encryptSecret warns
before the plaintext fallback). All verified: typecheck + 243 tests + eslint green; the 5 touched files are
prettier-clean. Deferred this pass (need UI/live-app or larger design): B2/B6 (cancel buttons + abort
wiring), R4 (can't safely identify a download's .part files at cancel time — needs the yt-dlp "Destination"
path tracked), L139 (needs a spawn↔auto-update interlock), L142 (multi-site IPC-return reconciliation,
generalises M34), L157 (cross-store cancel-on-remove, ties to C2), R9 (clock-skew — "note for awareness").
Session 2026-06-30 pass 4 (Critical — store architecture): C1 (single source of truth for the
preview mock + Settings defaults — DEFAULT_SETTINGS in @shared, one typed mockApi.ts, centralized
isPreview) and C2 (broke the downloads ↔ sources circular import via a pure typed event bus,
store/coordinator.ts — neither store imports the other now). All verified: typecheck + 243 tests +
eslint green; the touched files are prettier-clean (3 pre-existing format violations in
DownloadOptionsForm/LibraryView/SettingsView are untouched and out of scope).
Session 2026-07-01 (main-process structure — the H1-adjacent decompose batch): L2 (the ~200-line
flat IPC block moved into one src/main/ipc.ts registerIpcHandlers(getMainWindow); index.ts is now
lifecycle + window creation and imports the shared theme helpers from ipc.ts), L69 (path helpers split
into src/main/paths.ts; settings.ts is now purely the electron-store layer), L10 (a main-process
src/main/constants.ts centralizes the spawn timeouts, execFile buffers, stderr tail, and the three store
caps — nine modules migrated off bare literals), CL2 (buildArgs → single BuildArgsInput object),
CL3 (startDownload's stream/close/error/watchdog wiring extracted into wireChildProcess), and L1
(the scheduled-download promoter tick moved from store-module load into an App useEffect so a store import
no longer starts a stray timer). All verified: typecheck (node+web) + 253 tests + eslint + production build
green; the touched files are prettier-clean (the 8 remaining prettier warnings are all pre-existing,
untouched files — DownloadOptionsForm/SettingsView/settings/*Card).
Session 2026-07-01 pass 2 (the "finish the audit" batch — phase-by-phase PRs): landed Phase 1
(doc/stale-checkbox reconciliation — L82, L89, plus ticks for wire-or-remove M5/M6/UX1, MAX_ENQUEUE_BATCH,
and the UX/ref duplicates whose roots already shipped), Phase 2 (build hygiene — L83 .blockmap off via
nsis.differentialPackage:false, L84 dist/ clean step), and Phase 3 (main foundation): CC8
(in-house file-backed leveled logger logger.ts + renderer log:write sink), the
execFileAsync consolidation (lib/exec.ts — the five hand-rolled spawn-and-read
wrappers in probe/ytdlp×2/ffmpeg/indexer/download.probeMeta; SIMP2, and the foundation for CC4/CC5),
L136 (incognito is now enforced in main — no errorlog entry, no title/URL in the OS toast, no login/
browser cookies), and R9 (the yt-dlp update throttle no longer skips forever after a backward clock
correction). Each phase is its own branch/PR stacked on refactor/h1-decompose-god-files. All green:
typecheck + 263 tests + eslint + prettier. Next: M4 (queue persistence — its own focused PR), the
download-engine lifecycle items (R4/L65/L139/L157/B2/B6 — need a live download to verify), the CC4/CC5
tails (SIMP5 line-buffer + SIMP16 net.request), then Phases 4–8 (renderer tokens+primitives, logic/perf,
UX/Windows, copy/a11y). Per the run's policy, subjective-visual and live-verify-only items are implemented
but left unchecked with a flagged list for a watched pass.
Session 2026-07-01 pass 3 (Batch 6 — the renderer design-token foundation): landed the shared
design-scale module components/ui/tokens.ts — one home for
SPACE (the spacing ramp), RADIUS (control/surface/card tiers over Fluent's radii), ELEVATION
(flat/raised/overlay), Z (overlay/tooltip), ICON (16/20/24/40), MOTION, SCRIM, and
META_SEP — then migrated the scattered literals across 17 renderer files. Structurally-verified items
checked off: UI2 (one 16px section gap on every screen via the view roots), UI4 (symmetric page
padding — the lone 28px horizontal is gone), UI5 (the intentional 4px icon-group / 8px control-row split
is now named + documented), UI6/UI7 (one thumbnail radius = Medium; page-card XLarge / list-item Large /
control Medium tiers), UI26 (a single motion timing + a global prefers-reduced-motion gate in
base.css), L100 (elevation scale — only the download bar (raised) and
command palette (overlay) cast a shadow; every other card stays flat/border-only), L101 (a z-index scale
replaces the two hardcoded 1000s), L121 (the renderer's one raw rgba(0,0,0,0.32) scrim → Fluent's
theme-aware colorBackgroundOverlay), L125 (icon sizes single-source from ICON), L160 (inline margins
tokenized — values unchanged), and L164 (one metadata separator •; the intra-value · inside
compound quality labels is deliberately untouched). UI3 and UI12 are implemented but left unchecked for a
watched visual pass — they shift a few pixels: card padding 14/20→16 (QueueItem, Settings cards), and the
former-18px chrome glyphs (sidebar nav + theme toggles, IconButton md) → 20 plus the private badge 14→16. All
green: typecheck (node+web) + 268 tests + eslint + production build; the touched files are prettier-clean (the
9 remaining prettier warnings are all pre-existing, untouched files).
Session 2026-07-01 pass 4 (Batch 7 — type & color roles): added
components/ui/text.ts — a useTextStyles hook with the single
muted secondary-text color and the shared title treatment, plus a header documenting the app's type-role
convention (Subtitle2 = titles/section headers · Text = row titles · Body1 = body copy · Caption1 = small
secondary text). Landed: L110 (the one muted color, was eight aliases hint/sub/meta/srcSub/…/previewMeta —
adopted in the list views: QueueItem, HistoryView, LibraryView, Sidebar, DownloadBar, PlaylistPanel;
color-identical, so no visual change; the combined/shared-across-cards aliases — stats/count/emptyHint,
settings hint, CommandPalette, inline — migrate incrementally), L114 (one row-title component: the
duplicated semibold-ellipsis title style is now text.title, and Library's three <span> titles became
<Text> — size-identical since Fluent's base font is already fontSizeBase300), L123/L124 (the
Caption1/Body1/Text roles are now written down in text.ts and followed), UI13 (verified: Filled is used
only on the two brand marks, Regular everywhere else — the convention already held), and UI11 (no literal
px font sizes remain — the last tile/badge glyphs snap to ICON tiers: Sidebar mark 20, Onboarding mark 24,
Library srcIcon 20, History emptyBadge 24; the fontSizeBase* token usages already satisfied the standard).
The four tile-glyph deltas (≤2px) join UI12's watched visual pass. All green: typecheck (node+web) + 268 tests
- eslint + production build; touched files prettier-clean (9 pre-existing warnings untouched).
Session 2026-07-01 pass 5 (Batch 8 — form primitives): L112 (removed the redundant dynamic
On/Off label from all 15 <Field>-wrapped <Switch>es in DownloadOptionsForm + the settings cards — the
Field already names each switch, and the toggle's position + aria-checked already convey state, matching
Library's switches), L133 (made the custom Select <Field>-aware
via useFieldControlProps_unstable({ supportsLabelFor: true }), so a Field's visible label names the native
<select> through htmlFor and its hint associates via aria-describedby; dropped the now-duplicate
aria-label from every Field-wrapped select (9 in total) — the two standalone selects, download-bar quality
and history filter, keep theirs), L129 (the
Hint align prop is now a discriminated union — allowed only with top/bottom placement, a type error
with left/right instead of being silently ignored), L134 (one busy convention — disabled +
icon→Spinner; converted AboutCard's version-check button off its adjacent-spinner layout so it matches
Fetch/Index/update), UI17 (the two appearance="secondary" buttons — Library "Check for new", Terminal
"Stop" — are now subtle, the app's standard non-primary appearance), L111 (convention clarified &
verified: Fluent Field hint for control-associated help, <Caption1 muted> only for standalone card
notes/status — control help already uses Field hint uniformly), and L135 (verified role-appropriate:
Textarea for the multiline trim ranges "one per line", Input for single-line fields). UI16 left
unchecked — button size-by-role is subjective; current usage already broadly follows it (screen-primary
large, row/toolbar small, settings default), so specific tweaks join the watched visual pass. All green:
typecheck (node+web) + 268 tests + eslint + production build; touched files prettier-clean.
Session 2026-07-01 pass 6 (Batch 9 — surface primitives): three new shared UI primitives —
EmptyState (L116/L117: one centered empty block — icon +
Body1 message + optional muted hint, with a compact variant for in-content placeholders — adopted by
Downloads/History/Library and, compact, by the Terminal log + Diagnostics), Banner
- LinkSuggestion (UI19: one tinted notification row
replacing five hand-rolled style blocks — the DownloadBar suggestion/channel-nudge/dup-warning + the Library
copied-link suggestion), and a shared
useTextStyles().truncate(L126: single-line-ellipsis util, adopted in Library rows + the playlist panel + Banner). Plus L102 (Settings "Default format" now uses the sameSegmentedControl+ qualitySelectas the DownloadBar — one mental model; thekind|qualitystring encoding is gone), and four resolved-by-verified-convention: L103 (busy = in-button icon→Spinner everywhere; done in L134), L115 (list-row actions icon-only+tooltip vs card-toolbar actions labelled — role-appropriate), L120 (Fluent<Card>for static content cards, the Batch-6 tokenized surface recipe for interactive/ list-item cards), and UI20 (toggles = solid brand, list-selection = brand-tint; the cited Library pill is gone via UI18). L128 deferred to Batch 14 (build-time DCE of preview seeds, grouped with PERF8). All green: typecheck (node+web) + 268 tests + eslint + production build; touched files prettier-clean.
Session 2026-07-01 pass 7 (Batch 10 — CC consistency & conventions): the contained consolidations landed;
the sweeping ones are honestly deferred. Landed: a shared stdout line-splitter
lib/lineBuffer.ts (createLineBuffer) replacing the duplicated newline loop in
download.ts + terminal.ts (CC3/CC4, + test/lineBuffer.test.ts, 7 cases), and
cleanError on all yt-dlp stderr — getYtdlpVersion/updateYtdlp now use the same cleanError(r.stderr) || r.error?.message || '' idiom as download/probe/indexer (CC6). CL6 resolved (the one real redundancy was
L15; clearDir/openFile/showInFolder are deliberate view-model wrappers). Deferred with the standard
documented — these are the architectural refactors the release gate already puts post-1.0, and several would be
reckless to do unattended: CC1 (renaming persisted setting keys is a breaking migration), CC5+CL4
(the last event-emitter wrap is the security-critical updater installer download — host re-validation per
redirect + SHA-256 verify — which can't be live-verified here), CC7 (cosmetic arg-ordering; the pure/impure
DI split already holds), CC9 (zod = new dep + validation rewrite), CC10 (jsonStore exists; the
electron-store split is deliberate for DPAPI), CC11 (constants centralized; only a config.ts move left),
CC12 (file-tree reorg = broad churn), CC13→L93, CC14→L142. Net: CC3/CC4/CC6/CL6 checked; the rest
carry a documented standard + deferral rationale. All green: typecheck (node+web) + 275 tests + eslint +
production build; touched files prettier-clean.
Session 2026-07-01 pass 8 (Batch 11 — UX behavior gaps): a new in-app toast surface
(store/toasts.ts + ui/Toaster.tsx,
non-portal bottom-center, +unit-tested) is the batch's spine — it makes UX6 real: a shared
reveal.ts revealFile now surfaces open/reveal failures ("File not found — it
may have been moved…") instead of the old silent no-op, and safeShowInFolder was upgraded to return an error
string like safeOpenPath. Also landed UX9/UI25 (a CounterBadge on the sidebar Downloads nav item shows
the active count from any tab, via a count-only selector that doesn't re-render on ticks), UX15 (cancelAll
- a "Cancel all (N)" header button), UX24 (
disabledFocusable+Hintexplains the disabled "Check watched"), UX12 (an inline "Enable custom commands" button on the Terminal gate), and L144 (the dup guard now checks history → "Already downloaded: …"). Deferred with rationale: UX7/UX8 (Settings IA redesign — visual), UX18/UX26 (subjective/visual polish), L131 (arguably by-design, needs the live taskbar), and L142 (the history/templates/sources IPC-return reconciliation — a focused own-PR, CC14's tail). All green: typecheck (node+web) + 279 tests + eslint + production build; touched files prettier-clean.
Session 2026-07-01 pass 9 (Batch 12 — a11y & discoverability): L118 (URL-field focus decoupled — the
DownloadBar registers a focuser in the nav store and App calls focusUrlField() instead of
getElementById('aerofetch-url'); the hardcoded id is removed), L155 (accent swatches are now a real
radiogroup — aria-checked radios, roving tabindex, arrow-key nav + focus ring, matching the app's
SegmentedControl pattern), UX21 (an Onboarding "Press Ctrl+K anytime" tip + the palette's Settings action
shows "Ctrl ,"), and UX22 + L67 (a "Show welcome tips again" button in Settings → About re-opens onboarding,
making it revisitable). Verified-and-documented: UX17 (Fetch/Paste already have aria-label + focus-reachable
Hints — a11y met; a visible label is a compact-layout choice) and UI24 ("none by design" — every row action
is a visible inline button + keyboard, and Fluent's context Menu is portal-based, which the app avoids for the
GPU-flicker reason). All green: typecheck (node+web) + 279 tests + eslint + production build; touched files
prettier-clean.
Session 2026-07-01 pass 10 (Batch 13 — copy polish + misc-L cleanup): the contained fixes landed; the
feature/visual/larger items are documented with rationale. Fixed: L158 (drag-highlight flicker — a
dragDepth enter/leave counter in useDownloadBar so the dashed outline no longer blinks crossing child
elements), L92 (preload methods renamed to match main — setMediaItemDownloaded/syncWatchedSources,
compiler-enforced through the Api type), L149 (the three-sentence tray hint is now one line), L171
(one MOCK_CURRENT_VERSION so the preview mock's getAppVersion/currentVersion agree), and L5
(representative pass — the About card's monospace inlines → mono* classes; convention set: makeStyles for
static, inline style only for data-driven values). Resolved by verification/convention: L21
(SettingsView's "two patterns in one file" is moot post-H1 decomposition), L31 (dual theme switcher is
width-driven, not inconsistent), L130 (lowercase = interpolated fragment, capitalized = shown-raw
sentence), L152 (search sizes are role-appropriate), UI22 (the brand/entity/section icon-emphasis set
is now defined). Deferred with rationale: L51/L64 (features), L93 (CC13 pass), L104→Batch 14 (perf),
L109 (visual, with UX26), L161 (layout, live-verify), UI21 (visual), UI23 (cookie sign-in window theming,
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.
Session 2026-07-01 pass 12 (Batch 15 — download-engine lifecycle): the batch the run's policy reserves for
a watched live session — so only the safe, non-file-deleting item was implemented, and the rest are staged
with per-item live-verify rationale. Landed: L157 — removeSource now emits a sourceRemoved bus event
(C2) and the downloads store cancels every active item whose mediaItemId prefix (${sourceId}:, per
indexerCore) matches, reusing the proven cancel() path (implemented + type/lint/test-green; process-kill
worth a live confirm). Resolved by prior work: UX11 (M4 persists the queue, so a scheduled item fires on
next launch once due — no longer "never fires if quit"), UX20 (L68 resets notifiedBackground on window
show, so every close-to-tray re-fires the "still running" notice, not just the first). Deferred to the
watched live pass with rationale — each needs a real download to verify: R4 (delete .part on cancel —
file-deletion data-safety), L65 (graceful Ctrl-C pause vs taskkill /F), L139 (spawn↔auto-update
interlock — timing race), B2 (index AbortSignal + Cancel), B6 (app-update cancel — ties to CL4's
deferred updater), L143 (in-window quit-anyway). All green: typecheck (node+web) + 279 tests + eslint +
production build; touched files prettier-clean.
Run wrap-up (2026-07-01 — Batches 9–15 unattended)
All seven remaining audit batches were worked as stacked branches (audit/batch-9-surface-primitives →
audit/batch-15-download-lifecycle), each: implement → self peer-review → verify (typecheck node+web · tests ·
eslint · prod build · prettier) → push. The contained, safe, testable items landed; three categories were
honestly deferred with documented rationale rather than forced unattended: (1) large architectural
refactors the release gate already puts post-1.0 (CC9 zod, CC10/CC12/CC14 persistence/reorg, CL4 updater),
(2) subjective-visual items needing an eyeball (UI3/UI12/UI16/UX7/UX18/UI21/UX26 …), and (3) the download-engine
lifecycle changes that require a live download to verify (Batch 15's R4/L65/L139/B2/B6/L143). Two items need
maintainer awareness: SR4 flips clipboardWatch off-by-default for new installs (reversible), and Batch 15's
deferred set + L157's cancel-on-remove want a watched live smoke-test before merge.
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, PERF8 code-splitting, and the visual UI/UX polish long-tail. These are best done as their own PRs with the app running so the UI can actually be checked.
1.0 Release-Readiness Audit (lead-engineer synthesis)
A pre-release synthesis of the ~385 catalogued findings below — root causes, a release gate, and the consequential issues per dimension (severity · reasoning · fix · benefit). The detailed, ID'd backlog follows. No rewrite required; everything here is incremental.
Verdict
Not 1.0-ready as-is, but close — roughly 3–4 focused PRs away. The foundation is genuinely strong: disciplined security model (context isolation, sandbox, argv-injection defense, host-pinned signed-checksum updater), a clean pure/impure architecture, and good unit coverage of the pure logic. What blocks a commercial 1.0 is a thin layer of (a) real reliability bugs, (b) naive persistence, (c) built-but-unwired features, (d) first-run/perceived-quality gaps, and (e) the absence of enforcement/observability tooling. None require rearchitecting.
Root-cause themes (the 385 findings collapse to 8)
- Built-but-unwired features — command preview (M5), incognito (M6), per-download options/extraArgs (UX1) are plumbed end-to-end with no UI; the ROADMAP even marks them ✅ (M25). Decide wire-or-cut.
- "Two ways to do X" with no enforcement — persistence (M1), validation (CC9), async (CC5), errors (CC6), UI primitives (UI14/UI18/UI19), formatters (M9) — and no ESLint/Prettier to hold any line (CC2).
- Renderer-optimistic state never reconciles + a store cycle — M34/L142 (settings show unsaved values), C2 (downloads↔sources circular import).
- Naive JSON persistence — non-atomic writes (R1), corrupt→silent total data loss (R2), O(n²) full-file rewrites per completion (R3/PERF7), no cache.
- Download-engine reliability gaps — no stall timeout (B1), reverse-order batches (M32), orphan
.parton cancel (R4), retry-during-teardown race (L140). - First-run / perceived quality — light-default theme (SR1), nightly-default yt-dlp (SR2), placeholder icon (W14), stuck "Resolving…" (SR6), progress bar visibly restarting (SR7), unsigned build (SIGNING.md).
- Accessibility & Windows-native feel — focus rings (UI28/29), no list keyboard nav (W7), title-bar
theme (W3), text-field context menu (W4), no
aria-live(W17), input labels (M28). - No enforcement / observability — no lint (CC2),
noImplicitAny:false(M38), no logging (CC8/M29), no production source maps (L170).
Release gate
MUST fix before 1.0 (blockers) — correctness, data safety, security, trust: B1 · M32 · M35 · M34 · R1 · R2 · R3 · H7 · H8 · SR1 · SR6 · SR7 · W14 — all resolved as of this session. Code signing was dropped from the gate: it's a deliberate non-goal (no certificate will be purchased; the SmartScreen prompt on the unsigned build is accepted).
SHOULD fix for 1.0 (high): wire-or-cut the dead features (M5/M6/UX1) · a11y cluster (UI28/29, W4, W7,
W17, M28) · W3 native theming · lint + noImplicitAny (CC2/M38) · dev-jargon copy (M37/SR9) · destructive
confirmations (UX4) · PERF1/PERF2 (per-download redundant work).
DEFER to 1.x: the ~150 Low items · the simplification refactors (SIMP*, do incrementally) · the UI token system + shared primitives (UI/SIMP — high value, larger effort) · i18n · sqlite migration.
By dimension (severity · reasoning · fix · benefit)
Reliability
- [Critical] No download stall timeout (B1). Reasoning:
spawnhas no timeout + no--socket-timeout; a dead connection hangs forever and permanently consumes a concurrency slot. Fix:--socket-timeout+ an idle watchdog that kills+errors via the existingkillTree. Benefit: downloads self-recover on flaky networks. - [Critical] Non-atomic writes + silent corruption→data-loss (R1/R2). Reasoning:
writeFileSyncisn't atomic; a crash mid-write corrupts the store and the next read returns[], silently wiping history/ sources. Fix: atomic write (temp+rename) + back up a corrupt file before resetting, in onejsonStore. Benefit: user data survives crashes; no silent loss. - [High] Reverse-order batch downloads (M32) & history duplicates (M35). Reasoning:
addMany+pumppromote highest-index first;redownloadmints a new id defeating dedup. Fix: reverse the batch enqueue; dedup history by URL. Benefit: playlists download 1→N; clean history. - [High] Optimistic state never reconciles (M34). Reasoning: renderer keeps a value main rejected.
Fix: apply the validated
Settingsthe IPC returns. Benefit: the UI never lies about what's saved.
Performance
- [High] O(n²) media-items rewrite (R3/PERF7). Reasoning: every completion re-reads+rewrites the
whole (≤20k-item) file synchronously. Fix: in-memory cache + batched atomic writes (the same
jsonStore). Benefit: channel downloads stop hitching the main process. - [Medium] Per-download redundant work (PERF1/PERF2). Reasoning:
templates.jsonread + settings decrypt on every spawn even when unused. Fix: gatelistTemplates()on the consent flag; cache decrypted settings. Benefit: lower latency/IO per download. - [Low]
summarizeQueue/pumpO(n) per tick (PERF3/PERF4). Fix: memoize + count-based pump. Benefit: scales to large queues.
Maintainability
- [High] Triplicated Settings/
Api+ the preview mock (C1). Reasoning: a 4th touch-point per IPC method; drift-prone. Fix:DEFAULT_SETTINGSin@shared; one typedmockApi. Benefit: one place to change. - [High] Store circular dependency (C2). Fix: a coordinator/event bus. Benefit: removes a latent init crash.
- [Medium] Duplicated persistence/validation/async/formatters (M1/CC9/CC5/M9). Fix: the
lib/helpers in SIMP1–SIMP5. Benefit: ~−500 LOC and fewer divergence bugs. - [Medium] God files (H1). Fix: SettingsView via
<SettingsCard>/<ToggleField>(SIMP10). Benefit: 1104→~600 lines, reviewable.
Consistency
- [Medium] Two ways to do X across persistence, status chips, segmented controls, buttons, errors (M1/UI18/UI14/UI15/CC6). Fix: one shared primitive/standard each (see "Recommended single style"). Benefit: the app reads as one product.
- [Low] Naming/copy drift (CC1, L95/L150, separators L164). Fix: the conventions in CC1. Benefit: professional finish.
Readability
- [Medium] Magic strings/numbers + long signatures + deep nesting (CL1/CL2/CL4, L10). Fix:
constants.ts, options-objects, extracted helpers. Benefit: faster comprehension, fewer transpose bugs (CL2).
Polish (perceived quality)
- [High] First-run defaults (SR1/SR2/SR3). Reasoning: light-on-dark first launch, nightly yt-dlp by
default, auto-download-new on. Fix:
theme:'system',ytdlpChannel:'stable',autoDownloadNew:false. Benefit: the first 10 seconds feel native and safe. - [High] Stuck "Resolving…" (SR6) & restarting progress bar (SR7). Fix: clear placeholder on error; weight the two merge phases. Benefit: nothing looks hung/glitchy.
- [High] Placeholder icon (W14). Fixed: a designed mark (teal gradient square + top sheen, bold
rounded download glyph over a landing shelf) replaces the flat placeholder; multi-size
.icoregenerated. Benefit: brand credibility. (Unsigned build is a non-goal — no cert; the SmartScreen prompt is accepted.)
User experience
- [High] Per-download options are unreachable (UX1). Reasoning: must change global settings to tweak
one download. Fix: wire the existing
DownloadOptionsForminto the bar (plumbing exists). Benefit: reclaims a whole feature. - [High] Destructive actions have no confirmation/undo (UX4). Fix: confirm Clear/Remove/Delete. Benefit: prevents data loss.
- [Medium] Silent failures on Open/Show (UX6) and no global download status off the Downloads tab (UX9). Fix: surface errors; sidebar badge. Benefit: the app feels responsive and honest.
Accessibility & Windows-native (UX-critical for a Windows product)
- [High] Fragmented/invisible focus + no list keyboard nav (UI28/29, W7). Fix: one focus-ring + roving list focus + Delete/Ctrl+A. Benefit: keyboard- and Narrator-usable.
- [Medium] Title bar ignores in-app theme (W3); text fields have no context menu (W4); no
aria-live(W17); inputs lack accessible names (M28). Fix: syncthemeSource; add an editing context menu; live regions;aria-labels. Benefit: feels like a native, accessible Windows app.
Developer experience
- [High] No enforcement tooling (CC2) +
noImplicitAny:false(M38). Reasoning: style/type safety rely on discipline; implicitanyis allowed. Fix: Prettier + typescript-eslint + CI; re-enablenoImplicitAny. Benefit: the conventions stay true; regressions caught pre-merge. - [Medium] No logging + no prod source maps (CC8/M29/L170). Fix: one leveled file logger at every catch; hidden source maps. Benefit: field crashes become diagnosable instead of invisible.
- [Low] Stale roadmap/docs (M25/M26, L80–L82); release checksum is manual (H8). Fix: reconcile docs;
generate
.sha256inbuild:win. Benefit: trustworthy docs; updates don't silently fail to install.
Suggested PR sequence to 1.0
- Correctness & data safety: B1, M32, M35, M34, and the cached/atomic
jsonStore(R1/R2/R3/PERF7). - Security & trust: H7 (encrypt cookies), H8 (auto-generate checksums). (Code signing is a non-goal — no cert.)
- First-run polish: SR1/SR2/SR3 defaults, SR6/SR7 status, W14 icon, W3 title-bar theme.
- Wire-or-cut + a11y + tooling: UX1 (per-download panel) or remove M5/M6; focus/keyboard (UI28/29, W7,
W4); Prettier+ESLint+
noImplicitAny(CC2/M38). - Incremental cleanup (post-1.0): the SIMP refactors and the Low/UI/UX long tail, behind the new lint gate.
Critical
- C1 — Single source of truth for the IPC mock + Settings defaults.
const PREVIEWredeclared in 8 files;main.tsxreimplements the entireApi(~180 lines); the fullSettingsobject is hand-maintained in 3 places (main/settings.tsDEFAULTS,renderer/store/settings.tsFALLBACK,main.tsxMOCK_SETTINGS). AddDEFAULT_SETTINGStoshared/ipc.ts, extract the preview mock into onemockApi.tstyped asApi, centralizeisPreview. Fixed:DEFAULT_SETTINGSis now the canonical default in shared/ipc.ts; main'sDEFAULTS, the rendererFALLBACK(store/settings.ts), and the previewMOCK_SETTINGSall derive from it (preview-only field tweaks layered on top) instead of three hand-kept copies. The whole browser mock moved into one mockApi.ts typedWindow['api'](so a missing/mistyped method is a compile error), andmain.tsxshrank towindow.api = mockApi. ThePREVIEWcheck is single-sourced in isPreview.ts and imported (asPREVIEW) by all 8 sites. - C2 — Break the
downloads ↔ sourcesstore circular dependency.downloads.tsimportsuseSources;sources.tsimportsuseDownloads. Works only via lazy.getState(). Introduce a coordinator/event bus that owns cross-store reactions. Fixed: a tiny typed event bus store/coordinator.ts (no runtime deps — theAddEntryimport is type-only) now owns the two cross-store reactions.sourcesemitsenqueueDownloads(consumed bydownloads.addMany);downloadsemitsdownloadCompleted(consumed bysources.markDownloaded). Each store imports the bus, not the other, so the cycle is gone. Becausedownloadsno longer pulls insources,Appnow eagerly imports thesourcesstore for side-effects (import './store/sources') so its startup load + scheduled---synckickoff and thedownloadCompletedsubscription still run at launch — robust even once the views are lazy-loaded (PERF8).
High
- H1 — Decompose god files.
SettingsView.tsx(1104, ~11 cards, ~30 selector subs + ~20 useState),DownloadBar.tsx(858, ~17 state hooks),store/downloads.ts(614). Done onrefactor/h1-decompose-god-files: SettingsView →components/settings/*.tsx(one component per card + sharedsettingsStyles.ts), leaving a ~90-line shell (search box + display-toggle filter); each card still renders one<Card>DOM node so the filter is unchanged. DownloadBar → adownloadBar/useDownloadBar.tshook (all state + handlers, wrapper handlers keep the preview-invalidation side-effects) +downloadBar/styles.ts+downloadBar/PlaylistPanel.tsx, leaving a render-only shell. store/downloads.ts →downloadTypes.ts(types, re-exported so importers are untouched) +downloadItem.ts(buildItem/titleFromUrl/RESOLVING) +downloadSeed.ts(preview seed), leaving the store + recordCompletion + wiring. Behaviour unchanged; full typecheck + lint + production build + 248 tests green. - H2 — Consolidate scattered utilities.
youtubeId×2; 4 YouTube-URL-parsing variants; byte/speed/eta/duration formatting across 4 modules. URL half: one canonicalyoutubeIdnow lives in urlHelpers.ts (the complete watch/shorts/embed/live/youtu.be parse);thumb.ts,queueStats.ts(sameVideo), andstore/downloads.ts(titleFromUrl, which also fixes an old bug where any host with a?v=param was mislabeled "YouTube video") all import it — the three ad-hoc reimplementations are gone. Formatter half (done with H4):fmtBytes/fmtSpeed/fmtEtanow have a single home in @shared/format.ts imported by both sides; the main formatters module re-exports them and the renderer's three private copies (a byte formatter, an ETA formatter, and a speed string→number re-parser inqueueStats) are deleted. Per-item and aggregate speed now use the same 1024-based scale (the old aggregate used 1000, a latent inconsistency). Unit-tested intest/clipboardLink.test.ts+test/download.test.ts+test/queueStats.test.ts. - H3 — Fix
probe.ts → download.tsdependency direction (fmtBytesimport); resolves with H2's shared formatter. - H4 — Carry raw numbers across the progress boundary.
DownloadProgressships formattedspeed/etastrings;queueStats.tsre-parses them back to numbers. Fixed:DownloadProgressnow carriesspeedBytesPerSec/etaSecondsas raw numbers (main'sparseProgressno longer formats them); the renderer stores the raw numbers onDownloadItem,QueueItemformats them for display via @shared/format, andsummarizeQueuesums/maxes the numbers directly — the lossy string→number→string round-trip (andqueueStats'parseSpeed/parseEtaSeconds) is gone. Done together with the formatter half of H2. - H5 — History re-download silently changes quality.
HistoryView.redownloadpasses the storedquality(for format-picker downloads this is a label like "720p · mp4 · 184 MB") back asqualitywith noformatId;buildArgs.videoFormat()has no matching case and falls back tobv*+ba/b(Best). The queued item shows "720p…" while actually fetching Best. - 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). - H7 —
cookies.txtwritten in plaintext with no restrictive perms. cookies.tswriteFileSync(getCookiesFilePath(), …)stores live auth/session cookies unencrypted underuserData. In the portable build that'sAeroFetch-data/next to the exe (USB stick / shared Downloads folder), so anyone with folder access can read a logged-in session. Settings secrets are DPAPI-encrypted (settings.ts) but cookies are not. Encrypt at rest or document the exposure for the portable/shared-PC scenario the app explicitly targets. - H8 — Release checksum is a manual step the updater hard-requires. updater.ts
sets
REQUIRE_CHECKSUM = trueand refuses any update lacking a<asset>.sha256, butbuild:win(electron-vite build && electron-builder --win) never generates one. Evidence indist/: only0.4.1has.sha256files (hand-made); the current0.5.0build does not. If a release is published without manually adding the checksum, every client's in-app update fails ("This release has no checksum … refusing to install"). Generate the.sha256in the build/release script.
Medium
- M1 — Unify JSON persistence.
sources.tshas genericreadJsonArray/writeJson;history.ts/errorlog.ts/templates.tsreimplement it inline. - M2 — Remove dead code:
getDefaultFolder/download:default-folder(channel + preload + handler + mock, zero callers). - M3 — Remove duplicated completion side-effect in
store/downloads.ts(history write +markDownloadedin bothapplyEvent('done')and the preview ticker). Subsumed by C2. - M4 — Decide queue persistence explicitly. Scheduler/queue is renderer-memory only;
saved/scheduled items don't survive a quit. Persist or document as a non-goal in code. Fixed (persist, per the run decision): a main queue.ts store (the proven cached-atomiccreateJsonStore,queue.json) plusqueue:list/queue:saveIPC. The renderer mirrors its durable items on every queue change (aPersistedQueueItem= the stable subset ofDownloadItem; runtime-only progress/speed/eta are dropped, and a signature over the persistable subset means progress ticks don't re-save) and rehydrates on launch via anAppuseEffect. On restore adownloadingitem (its process died) normalizes back toqueued(yt-dlp continues the.part),saved+scheduledForsurvives so the promoter fires it when due,paused/errorreturn actionable;completed/canceledare never persisted. AqueueHydratedgate stops a launch-time store change wiping the file before it's read. Pure mappers extracted to queuePersist.ts and unit-tested (test/queuePersist.test.ts); plumbing is typecheck-verified. Live-verify note: the OS-level quit→relaunch cycle itself wasn't smoke-tested here — worth a manual confirm (schedule/park an item, quit, relaunch). - M5 — Command-preview feature is fully built but unwired.
CommandPreviewResulttype +command:previewchannel +previewCommand(download.ts) +formatCommandLine/quoteForDisplay(buildArgs) + preload method all exist, but no renderer component callswindow.api.previewCommand(only themain.tsxmock references it). Wire it into the DownloadBar Options panel, or remove the dead chain. Fixed: a "Show command / Hide command" toggle in DownloadBar.tsx callswindow.api.previewCommandand renders the exact yt-dlp argv (monospace). The preview mirrors whatdownload()spawns — probe-selectedformatId/formatHasAudio,quality, per-downloadoptions, andtrim— and is invalidated whenever any of those inputs (or the URL / a re-probe) changes, so the shown command never goes stale. Wired with M6/UX1 in the same panel. - M6 — Private/incognito mode is plumbed but unreachable.
incognitoflows throughAddOptions→buildItem→ history-skip → the QueueItem "Private" badge, but nothing in the UI ever setsincognito: true. Add the toggle or remove the dead plumbing. Fixed: an "Incognito mode (no logging, no history, no cookies)" checkbox in the DownloadBar Advanced panel setsincognitoon the enqueued item (viaAddOptions), lighting up the existing QueueItem "Private" badge. Reset to off after each download so it never silently carries over to the next. - M7 —
newId()duplicated 3× (store/downloads.ts,TerminalView.tsx,TemplateManager.tsx) with divergent fallback prefixes (item-/t-/tpl-). Extract one helper. Fixed: onenewId(prefix)helper; the three copies import it and pass their prefix. The fallback gained a monotonic counter so same-millisecond ids can't collide (subsumes L33/L70). Unit-tested intest/id.test.ts. - M8 — Two download-status→label maps.
STATUS_BADGE(QueueItem) andSTATUS_LABEL(LibraryView) independently map the same statuses (completed = "Completed" vs "Downloaded"). One shared map. - M9 — Three time formatters.
relTime(LibraryView),formatWhen(HistoryView),fmtSchedule(QueueItem) — consolidate into a date util. Fixed: all three moved todatetime.tsand imported by their views; unit-tested intest/datetime.test.ts. - M10 —
.urlshortcut parsing duplicated —parseUrlFile(DownloadBar) vsreadUrlShortcut(main/deeplink). DifferentURL=extractors for the same file format. - M11 — Inconsistent clipboard access. Reads use
navigator.clipboard.readText(paste button) andwindow.api.readClipboard(suggestion watcher); writes usenavigator.clipboard.writeText(copy report). Pick one strategy. - M12 — Shared
errorTextstyle.tokens.colorPaletteRedForeground1is applied inline 12× across 5 files instead of one class. Partially fixed: addeduseErrorTextStyles(error/errorPrefor pre-wrap multi-line output) and adopted it inSettingsView.tsx— the biggest offender — replacing all 6 inlinestyle={{ color: … }}error spans + its localerrorRowTextclass. The remaining red references live in per-componentmakeStylesclasses (DownloadBar/LibraryView/QueueItem/TerminalView), not inline; migrate those to the shared hook incrementally. - M13 — Inconsistent secret-field masking.
updateTokenusestype="password";proxy(may carryuser:pass@) andyoutubePoToken(a token) are plain-text Inputs. - M14 — Settings search mutates React-owned DOM. The search toggles each card's
el.style.displaydirectly; fragile (breaks if a card ever gets a conditionalstyle) and matches ontextContentincl. hidden text. Prefer state-driven filtering. Fixed: each card renders inside a React-owned wrapper whose visibility is driven by ahiddenstate array in SettingsView.tsx — the filter no longer writesdisplayonto a card's own node, so a card setting its own inline style can't break it. Matching still reads the wrapper'stextContent(so search covers every visible label without hand-maintained per-card keywords); the minor "matches hidden descendant text" point is unchanged and left as-is. - M15 — Nested interactive controls in
role="button"(a11y). LibraryView's group header is arole="button"div containing<Button>s (All / Download) — invalid ARIA / keyboard semantics. Make the header a real element with sibling buttons. - M16 — No renderer error boundary. An exception in any view unmounts the whole shell to
a blank window. Add a top-level boundary with a recover/reload affordance. Fixed: added
ErrorBoundarywrapping<App/>inmain.tsx. The fallback is dependency-free (no Fluent/theme provider — the crash may have come from that tree), logs viacomponentDidCatch, shows the error message, and offers a Reload button (window.location.reload(); persisted data lives in main, so nothing is lost). - M17 —
statusByUrlcollapses duplicate URLs. LibraryView maps URL→status into oneMap; with "Download anyway" duplicates, a row can reflect the wrong item's state. - M18 — Audio "quality" presets hard-code MP3.
QUALITY_OPTIONS.audiolabels ("Best (MP3)", "320 kbps") are shown even whendownloadOptions.audioFormatis opus/flac/wav, so the label contradicts the actual output format;--audio-qualityis also meaningless for lossless. - M19 — Sidebar-collapsed pref uses
localStoragewhile every other preference uses electron-store — won't back up/restore and is lost in the portable build across machines. - M20 — ProgressBar / native
<select>accessibility. QueueItem/DownloadsViewProgressBars have no accessible name; severalSelects carry both a<Field label>and anaria-label(double announcement). - M21 —
--audio-qualityalways emitted.buildArgspasses--audio-qualityeven for losslessaudioFormat(flac/wav), where it's meaningless; pair with M18's preset/format mismatch. - M22 — Backup export writes secrets in cleartext, silently. backup.ts
exportBackupserializes the decrypted settings (proxy creds, PO token, update token) to a plain JSON file; the UI caption only says "Does not include download history" — no warning that the file contains credentials. Fixed:proxy/youtubePoToken/updateTokenare stripped (set to'') before writing; SettingsView caption updated to say credentials are not included. - M23 —
MAX_ITEMStruncation can silently drop a source's items. sources.tsreplaceMediaItemsdoes[...new, ...others].slice(0, 20000); once the global total exceeds the cap, the tail (another source's items) is dropped on write and only reappears when that source is re-indexed. Cap per-source, or surface it. - M24 —
setSettingsaccepts unvalidated free strings. settings.ts storesrateLimit/proxy/defaultVideoQuality/defaultAudioQuality/youtubePlayerClientas any string. A badrateLimit("abc") only fails at download time; adefaultQualitynot inQUALITY_OPTIONSleaves the Settings dropdown's controlled value blank. Fixed:VIDEO_QUALITY_OPTIONS/AUDIO_QUALITY_OPTIONSadded toshared/ipc.tsand imported in bothsettings.ts(allowlist validation) andstore/downloads.ts(single source of truth).rateLimitvalidated against yt-dlp rate format regex;youtubePlayerClienttrimmed. - M25 — ROADMAP marks unwired features as ✅ shipped. ROADMAP.md Phase C
("Command preview … Surfaced as a Preview command button in the download bar") and Phase D
("Private / incognito mode — a download bar toggle") both carry
[x], but neither is wired in the UI (corroborates M5/M6). The roadmap overstates completion — reconcile the docs or finish the wiring. - M26 — ROADMAP accent palette is wholesale stale. ROADMAP.md Phase E describes "four accent presets — Toffee (original), Slate, Evergreen, Lavender" with a default of Toffee; the shipped theme.ts is rose / coral / amber / teal ("Sunset-to-sea") with a default of teal. The whole section documents a palette that no longer exists.
- M27 — Library batch-enqueue ignores per-item kind. sources.ts
enqueueItemsforcessettings.defaultKind(+ its quality) for every item, so a channel can't be downloaded as audio without flipping the global default — inconsistent with the DownloadBar playlist panel, which offers a per-item video/audio toggle. - M28 — Primary text inputs have no accessible name. The URL field (DownloadBar), add-source
(Library), search (History/Settings), and the Terminal args
Textarearely onplaceholderonly — which is not an accessible name. Addaria-label/<label>to each. (a11y; distinct from M20's Select/ProgressBar.) - M29 — Failures are swallowed everywhere. 29
.catch(() => {})sites across 15 files: nearly every IPC write/read discards its error with no log, no user feedback, and no telemetry. A failed settings write, history add, oropenPathsimply vanishes. Fixed: a sharedlogError(op)helper replaces every swallowed renderer catch with one consistentconsole.error('[AeroFetch] <op> failed:', e)(settings, history, templates, errorlog, sources, App, DownloadBar drag-drop, SettingsView version reads, LibraryView scheduled-sync); main-process sites (ytdlp auto-update, cookies loadURL) log inline. Four genuinely-silent catches left intentionally:pauseDownload/cancelDownload(process kill, state already applied),unlink(cleanup), andclipboard.writeText(browser permission — no user action available). Scope note: this delivers the "log" half — failures are no longer silent. The user-facing toast + a persistent log sink (rendererconsole.errorisn't captured in a packaged build) are deferred to CC8 (electron-log), which the audit already cross-references here. - M30 — A broken contextBridge degrades silently to mock mode. preload/index.ts
only
console.errors ifexposeInMainWorldthrows; the renderer then sees nowindow.electron, flipsPREVIEWtrue, and runs the browser mock (no real IPC) with no visible error. A real bridge failure looks like "preview." Fixed: preload sendsIpcChannels.preloadBridgeFailureon failure; main registersipcMain.once(IpcChannels.preloadBridgeFailure, …)inregisterIpcHandlers()(run before the window is created) and shows an error dialog + quits, so the failure is impossible to miss. - M31 — Default Electron menu (incl. Toggle DevTools/Reload) ships in production. No
Menu.setApplicationMenu(null)is called; withautoHideMenuBarthe default role menu is still Alt-accessible, exposing DevTools/reload to end users. Fixed:Menu.setApplicationMenu(null)called inapp.whenReady()when!is.dev; dev builds keep the menu for DevTools access. - M32 — Playlist/channel batches download in reverse order.
addManyprepends the whole batch in entry order ([entry1…entryN, …old]), butpump()promotes the highest-index queued item first (it reverses, assuming one-at-a-time prepends). So a selected playlist/channel downloads N → 1. Files are still named001…NNNcorrectly (byplaylistIndex), so a partial run leaves the last videos on disk — surprising. Enqueue batches in reverse, or havepump()respect insertion order. - M33 — Documented enqueue batch cap doesn't exist. ipc.ts:629 and
ROADMAP-PINCHFLAT claim the queue pulls "a batch at a time (e.g. 50)" via
MAX_ENQUEUE_BATCHso it "never holds the whole collection at once," but no such cap exists —enqueueItemsaddManys every selected item, so "Download all pending" on a 5,000-video channel puts 5,000 items in the store/queue at once. Implement the cap or fix the comment. - M34 — Optimistic settings updates never reconcile with main's validation. The settings store
does
set(partial)thensetSettings(partial).catch(() => {}), discarding the validatedSettingsmain returns. A value main rejects (e.g. an unsafefilenameTemplate, a clampedmaxConcurrent, a malformedrateLimit) still shows as accepted in the UI until restart — the user believes a setting saved that didn't. Apply the returned authoritative state. - M35 — History re-download creates duplicate rows.
redownloadre-queues viaaddFromUrl, which mints a new id; on completionaddHistorydedups by id (no match) and prepends a second row for the same video. Re-downloading from History accumulates duplicates. Dedup by URL, or reuse the entry id. - M36 — Library checkbox count ≠ "Download N selected". Every item row has a checkbox
(LibraryView.tsx), but
selectedActionablefilters to pending/error/canceled only, so selecting 5 rows (incl. 2 already-downloaded) shows "Download 3 selected." The checkbox count and the button count silently disagree. Fix: only show checkboxes on actionable rows, or count all selected. - M37 — End-user hints leak developer/internal references. Settings hints surface dev-facing
detail: "Requires aria2c.exe in resources/bin (see the README there)", "paste a read-only Gitea
token", "sent via
--extractor-args", "breaking downloads with 403 errors", "open a locked cookie database", and roadmap status ("automatic minting is planned"). These read as code comments, not product copy. Fix: rewrite for end users (hide repo/flag/HTTP-status jargon and roadmap notes). - M38 —
noImplicitAny: falsepartially defeatsstrict. The inherited@electron-toolkit/tsconfigsetsstrict: truebut explicitly"noImplicitAny": false", and neither project tsconfig re-enables it — so a parameter/variable with no inferrable type silently becomesanyproject-wide (the one real hole in an otherwise-strict setup;noUnusedLocals/noUnusedParameters/noImplicitReturnsare on, which is why dead locals don't accumulate). Fix: re-enablenoImplicitAnyintsconfig.node.json/tsconfig.web.json.
Low
- L1 — Module-load
setIntervalinstore/downloads.tsruns on import (incl. tests/preview). Fixed: the scheduled-download promoter tick moved out of store-module scope into anApp.tsxuseEffect(intervalSCHEDULE_TICK_MS, cleared on unmount). Importing the store in tests/preview no longer starts a stray never-cleared timer; the store keeps thepromoteDueScheduledaction, App owns the cadence. (The previewstartFakeTickerinterval is created on demand per launched item, not at load, so it was already fine.) - L2 —
index.tsflat IPC registration (~150 lines) — let each main module export its ownregister(ipcMain). Fixed: the whole ~200-line handler block moved into one dedicated ipc.ts exportingregisterIpcHandlers(getMainWindow)— the single place the renderer's IPC contract is wired. index.ts now imports it (plus the three shared theme helpersresolveBackgroundMode/applyNativeTheme/getSystemThemeInfo, also moved to ipc.ts) and shed ~50 imports it only needed for the handlers, leaving it as app-lifecycle + window creation. The window-coupled handlers (folder picker, theme-synced background, taskbar progress) take the live window via thegetMainWindowaccessor rather than a capturedmainWindow. - L3 — Stale comments.
getYtdlpVersionJSDoc still calls it the "Step-1 spike";vitest.config.tsclaims tests "only exercise buildArgs.ts" (9 test files now exist). - L4 —
electron-builder.ymlnits.copyright: yt-dlp frontendis not a copyright string (no holder/year); thefilesexcludetsconfig.tsbuildinfonever matches the realtsconfig.web.tsbuildinfo. - L5 — Inline
style={{}}vs makeStyles (25× across 8 files; SettingsView 14×). Notably Sidebar's active-nav inset shadow and DownloadOptionsForm's hint caption use inline styles where a class exists elsewhere. Addressed incrementally + convention set: the About card's four repeatedstyle={{ fontFamily: fontFamilyMonospace }}inlines are nowmono/monoBlock/monoPreclasses in settingsStyles.ts. The convention: makeStyles for static styling; inlinestylestays only for genuinely data-driven values (a swatch'sbackgroundColor: opt.swatch, a thumbnail's dynamic dimensions, the active-nav inset shadow keyed to selection) — those can't be static classes and are correct as-is. Remaining static inlines migrate opportunistically under this rule. - L6 — Control height mismatch.
Selectis a fixed 32px sitting besidesize="large"(~40px) Input/Buttons in DownloadBar. - L7 —
canceledandpausedshare the 'warning' badge color (QueueItem) — visually ambiguous. - L8 — Index/compound list keys. TerminalView keys log lines by array index; Settings
Diagnostics keys by
id + occurredAtwhile every other list keys byidalone. - L9 — Thumbnail box sizes not shared. Four hand-tuned 16:9 boxes (120×68, 108×64, 72×44, 60×34) with no shared aspect/size constant.
- L10 — Scattered magic numbers. Timeouts/caps spread across modules (probe 60s, indexer
180s, probeMeta 30s, update-idle 60s; MAX_ENTRIES 500/200, MAX_TEMPLATES 100, MAX_ITEMS 20000).
Consider a central constants module. Fixed: a main-process constants.ts now
holds the process-spawn timeouts (
VERSION_TIMEOUT_MS,META_PROBE_TIMEOUT_MS,PROBE_TIMEOUT_MS,INDEX_TIMEOUT_MS,YTDLP_UPDATE_TIMEOUT_MS,FEED_FETCH_TIMEOUT_MS,STALL_TIMEOUT_MS), the execFilemaxBuffersizes (META/PROBE/INDEX_MAX_BUFFER),STDERR_TAIL_BYTES, and the three hand-rolled store caps (ERRORLOG_MAX_ENTRIES,TEMPLATES_MAX,MEDIA_ITEMS_MAX). download/ffmpeg/indexer/probe/ytdlp/sync/ errorlog/templates/sources import from it instead of bare literals. Values already single-sourced in the shared contract (HISTORY_MAX_ENTRIES) and updater.ts's already-named local timeouts were left in place — they aren't scattered literals. - L11 — "Queue (N)" overcounts. DownloadsView's header count is
items.length(includes completed/error/canceled), not the active queue. - L12 — Command palette polish. No scroll-into-view for keyboard selection in the 50vh
list;
role="dialog"withoutaria-modal/focus-trap; Esc handled only on the input. - L13 — Destructive actions lack confirmation. "Clear history", "Clear log", "Remove source" are one-click — inconsistent with the careful confirm on backup import.
- L14 —
base.cssis bare — no globalbox-sizing,:focus-visible, or font-smoothing baseline, so custom native elements (Select, Hint, segmented controls) get inconsistent focus rings;box-sizingis then set ad hoc on the SettingsView swatch. - L15 —
MediaThumbredundant ternarykind === 'audio' ? 'audio' : 'video'(kind is already that union). - L16 —
relTime/formatWhenverbosity.relTimereturns unbounded "N d ago"; History'sformatWhenalways appends the year, even for the current year. - L17 — TemplateManager regex not validated. An invalid
urlPatternis accepted with no feedback and silently never matches (main'smatchesUrlswallows the error). - L18 — Terminal nav item shown when custom commands are off — leads to a gated dead-end view; consider hiding/disabling it.
- L19 —
App.tsxfocus hack —setTimeout(() => …focus(), 60)after a tab switch. - L20 —
youtubePlayerClientis free text with no allowlist/validation; a typo passes straight to yt-dlp's--extractor-args. - L21 — Inconsistent in-component error handling. SettingsView funnels the app-update
not-ok result into a dedicated
appUpdErrorslot but keeps not-ok duals for version/update/export/import — two patterns in one file. Resolved by the H1 decomposition: SettingsView is no longer one file — app-update lives inSoftwareUpdateCard, version/update inAboutCard, backup export/import in its own card, each holding its own result state internally. The "two patterns in one file" no longer exists; each card's error handling is self-contained and consistent within itself. (The remaining{ ok, error }vsnullsignaling styles are the CC6 cross-cutting item, not this per-file one.) - L22 — DownloadOptionsForm shows both audio + video controls regardless of the chosen kind in the per-download override panel (audio format shown for a video download, etc.).
- L23 — Diagnostics shows first 20 errors with no "showing 20 of N" hint (the copied report includes all).
- L24 —
window.open(htmlUrl, '_blank')in SettingsView is the onlywindow.openin the app (relies on the window-open handler); elsewhere external opens go through IPC/shell. - L25 — Taskbar effect over-fires.
App.tsxsubscribes to the whole downloads store and recomputessummarizeQueue+ sends the taskbar IPC on every store change (each progress tick); throttle or dedupe by computed value. - L26 — Third URL-sniffing path. DownloadBar's drag-drop
firstUrlis a separate "first http(s) line" extractor alongsideuseClipboardLink.looksLikeUrland deeplink's scan. - L27 — DownloadBar Enter probes, not downloads. Enter in the URL field triggers
fetchFormats; there's no keyboard path to actually start the download (LibraryView's Enter runs its primary action). Inconsistent. - L28 — Default kind/quality applied once. DownloadBar seeds from settings via a one-shot ref; changing the default in Settings while on the Downloads tab isn't reflected until reload.
- L29 — UI constant in the store.
QUALITY_OPTIONSlives instore/downloads.tsyet is a presentational constant imported by views — couples UI to the store module. - L30 — Redundant
loading="lazy"onMediaThumbimages already inside a virtualized list (the virtualizer only mounts visible rows). - L31 — Two theme switchers. Sidebar shows a 3-way radio when expanded but a cycle button
when collapsed — minor dual-UX for the same setting. Resolved as intentional (documented): the two are
responses to width, not a real inconsistency — the expanded sidebar has room for the 3-segment
SegmentedControl, while the 60px collapsed rail can only fit a single icon button, so it cycles Light→Dark→Auto (with a tooltip naming the current mode). Both drive the exact samethemesetting; a full 3-way control can't fit collapsed. Left as-is by design. - L32 —
paletteActionsrebuilt every render inApp.tsx(not memoized); harmless today, but it's passed straight into a child. - L33 —
idCounterfallback resets per launch. The non-cryptonewIdfallback (item-${++idCounter}) restarts at 1 each run; theDate.now()-based fallbacks elsewhere can collide within a millisecond. Unify on one robust id helper (see M7). Fixed with M7: the singlenewIdfallback is${prefix}-${Date.now()}-${++counter}, so same-ms ids stay distinct.
Round 3 (2026-06-29) — tests, build metadata, error copy, edge cases:
- L34 — Integration test off by default.
real-download.integration.test.tsisdescribe.skipIf(!RUN);npm testnever exercises a real download, so there's no automated regression for the actual yt-dlp argv path (only the pure builder). Fixed: atest:integrationnpm script setsAEROFETCH_REAL_DOWNLOAD=1and runs the suite, making the opt-in real-download regression explicit + discoverable. It stays out of the defaultnpm teston purpose — it needs network + a live yt-dlp, so it can't run in an unattended/CI pass. - L35 —
isValidMediaItemhas no test — the one persisted-JSON validator of six with no spec. - L36 —
compareVersionsdiffering-length components untested (e.g.1.2vs1.2.0). - L37 —
download.tsformatters untested (parseProgress/fmtBytes/fmtEta, incl. theNApaths). - L38 —
buildArgswebm-thumbnail exclusion branch untested (embedThumbnail && container!=='webm'). - L39 —
CROP_SQUARE_PPAexact-string assertion is a change-detector test (breaks on harmless reformat). - L40 —
clipboardLink.test.tsimports the zustand settings store to test two pure helpers (looksLikeUrl/looksLikeSingleVideo) — couples a pure-fn test to store init (see H2). - L41 —
package.jsonhomepagepoints at yt-dlp's GitHub, not AeroFetch — wrong product URL (surfaced by the NSIS installer). - L42 —
package.jsonhas nolicensefield (ships LGPL ffmpeg + GPL aria2c). - L43 —
package.jsonhas norepositoryfield (real repo is the Gitea instance). - L44 — Vestigial lint excludes.
electron-builder.ymlexcludes.eslintrc/.prettierrcbut there's no ESLint/Prettier config, script, or dependency — no enforced style tooling. - L45 — Three self-descriptions drift: pkg
description"A yt-dlp frontend for Windows" vs buildercopyright"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 —
probeMeta30s timeout returns null silently — no surfaced message, unlike every other yt-dlp timeout ("Timed out …"). - L48 —
cleanErrorstrips a leadingerror:for live failures, while rawERROR:text shows elsewhere (errorlog seed / terminal) — inconsistent error normalization. - L49 — Newlines in user-facing error strings. download.ts missing-binary messages embed
\n, which renders awkwardly in inline/Caption error spans. - L50 — "Cookies saved" shown for 0 cookies. Closing the sign-in window without logging in
still writes the file and reports
okwithcookieCount: 0. - L51 — Scheduled daily sync time hardcoded to 09:00 (schedule.ts) — on/off toggle only, no time picker.
Fixed. New
Settings.syncTime(24h 'HH:MM', default '09:00'); a compact native time input appears next to the Library's "Daily sync" switch when it's on.setScheduledSync(enabled, time?)threads the time intoschtasks /ST, guarded by a sharedisValidSyncTime(in @shared/ipc, used by both main's settings validation and schedule.ts — the value reaches the schtasks argv, so malformed input falls back to the default rather than being forwarded; unit-tested incl. injection-shaped values). The picker persists via settings on change and re-registers the task on blur (oneschtasks /Create /Fper commit, not per keystroke — /F overwrite IS the time-change mechanism). Live-verified on this machine: re-registering the real AeroFetchDailySync task with /ST 19:30 moved its start to 7:30 PM and back (the user's original 09:00 registration restored byte-exact afterwards). - L52 — Installer handoff is a fixed
setTimeout(app.quit, 1500)(updater.ts) — a race on slow machines. - 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 noonErrorfallback (inconsistent withMediaThumb's retry). - L55 — QueueItem meta line can show size twice — a probed-format
qualitylabel already contains the size, thensizeLabelis appended again. - L56 — SponsorBlock ON with zero categories silently no-ops (
buildArgsguards onlength > 0) with no UI warning. - L57 — Scheduling a past datetime silently downloads now (
buildItemfuture-only guard), no feedback that the schedule was ignored. - L58 —
chooseFolderpasses the macOS-onlycreateDirectoryproperty — dead option on Windows. - L59 —
THEME_BACKGROUND(index.ts) duplicatespageBackground(theme.ts) — two hand-synced color constants (a "keep in sync" comment guards them). - L60 —
'external-url'channel name lacks thecategory:prefix every other IPC channel uses. - L61 —
taskbarProgressis the loneipcMain.on(fire-and-forget) amid all-invokechannels. - L62 — "Best available" synthetic format hardcodes
ext:'mp4'/hasAudio:true(probe.ts) — mislabels when the chosen container is mkv/webm. - L63 —
audioQuality()unknown label → silent'0'(best) — the audio analog of H5. - L64 —
ARIA2C_ARGSconnection params hardcoded (-x16 -s16 -k1M), no user control. Fixed. NewSettings.aria2cConnections(1–16, default 16 — aria2c's own -x/-s ceiling), exposed as an "aria2c connections" SpinButton in Settings → Network, shown only while the aria2c toggle is on (same pattern as maxConcurrent). The const became a purearia2cArgs(connections?)in buildArgs.ts that clamps/ rounds defensively (settings validation clamps too —applySettingsmirrors the maxConcurrent case);ARIA2C_ARGSremains exported asaria2cArgs()so existing references stay valid. Threaded throughAccessOptions.aria2cConnectionsfrom buildCommand. The-k 1Msplit floor stays fixed — the meaningful user knob is connection count; a free-text field would bypass the customCommandEnabled consent gate. Unit-tested: custom count reaches--downloader-args, clamp floor/ceiling/rounding/non-finite. - L65 — Pause uses
taskkill /Flike cancel — a forced kill risks an unflushed.parttail vs a graceful stop. Verified acceptable in the Batch 15 live pass — no code change needed. Live test (a real 360p download paused via the sametaskkill /T /Fthe app issues, then resumed with identical args): the paused.partwas left byte-intact at 3,913,524 bytes, and yt-dlp's default--continueloggedResuming download at byte 3913524, restarting mid-file (first progress tick 16.7%, not 0%) and producing a valid complete file (ffprobe: full 281.6 s duration, exit 0). The feared "unflushed-tail corruption" does not occur: yt-dlp writes the.partincrementally, a forced kill leaves the already-written bytes intact, and resume re-requests only from the actual byte count — so at worst a few unflushed KB are re-fetched, never corrupted. A graceful Ctrl-C (GenerateConsoleCtrlEvent) would add Windows-specific fragility (windowsHide, no shared console, whole-process- group signalling) for zero correctness gain. Left as the forced kill. (NB: an unrelated observation from the same test — abv*quality-preset selector can re-resolve to a different format on resume and discard the.part; a specific probedformatIdresumes deterministically. Efficiency-only, still produces a valid file.) - L66 — Sidebar caption flips "yt-dlp frontend" → "v" once the version loads (text shift on boot).
- L67 — Onboarding has no Skip and can't be revisited (no "show tips again"). Fixed the revisit half (with UX22): Settings → About has a "Show welcome tips again" button that re-opens the onboarding screen. Onboarding is a single dismissible card (its "Get started" button is the skip — you can proceed without choosing folders, defaults apply), so a separate Skip is redundant.
- L68 — Background-running notification fires once per process (
notifiedBackgroundnever resets) — won't remind on later window closes. - L69 —
settings.tsmixes path helpers with persistence (getDefaultMediaDir/ensureMediaDirs/getDownloadArchivePathalongside the store) — split apaths.ts. Fixed: the three path helpers moved to paths.ts; settings.ts is now just the electron-store persistence layer (its now-unusedpath/fsimports dropped). download.ts and index.ts import the helpers from./paths.applyLaunchAtStartupstays in settings.ts — it's OS login-item integration referenced byapplySettings, not a path helper. - L70 —
crypto.randomUUIDfallback is effectively unreachable in Electron/Node 26 (typeof crypto !== 'undefined'is always true) — dead defensive branch (see M7). Fixed with M7: the branch now lives once innewId(kept so the fn is total in any host) and is covered by a test that stubscryptoto exercise it. - L71 — Settings folder inputs are
readOnlywith only a Browse button — no way to paste a known path. - 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 thetitleFromUrlplaceholder 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.
- L78 —
DownloadsViewempty-state copy ("Paste a URL above to get started") doesn't match the actual two-step Fetch→Download affordance. - L79 — SponsorBlock default categories =
['sponsor']only — enabling SB silently covers just sponsors until the user expands categories (expectation gap).
Round 4 (2026-06-29) — doc/code drift, build artifacts, edge cases:
- L80 — ROADMAP wrong file path. ROADMAP.md Phase A says the flags are
emitted "in
buildArgs(src/main/download.ts)" —buildArgslives insrc/main/buildArgs.ts. - L81 — ROADMAP internal contradiction. Phase A: "
--split-chaptersstill TODO"; Phase L:[x]split-chapters done. Phase A wasn't updated when L landed. - L82 — PINCHFLAT roadmap interface sketches are stale. ROADMAP-PINCHFLAT.md
Phase F's
Source/MediaItemcode blocks omit shipped fields (videoId,itemCount,watched,feedUrl) — design sketches that drifted fromshared/ipc.ts. Fixed: the Phase F sketch now carries the shippedvideoId/itemCount/watched/feedUrlfields (plus the requiredurl/playlistTitle/playlistIndex) and a note that the block reflects the shippedshared/ipc.tsshape. - L83 — Dead build artifacts. electron-builder emits
.blockmapdifferential-update files for each NSIS installer, but the custom updater.ts does a full download and never consumes them. Fixed:nsis.differentialPackage: falsein electron-builder.yml stops the blockmap being generated (NsisTarget gates it ondifferentialPackage !== false), with a comment explaining the custom-updater rationale. - L84 —
dist/accumulates unboundedly. ~620 MB per build with no cleanup; it currently holds 0.1.0–0.5.0 Setup + portable artifacts (~3 GB). Add a clean step or prune. Fixed: aclean:distnpm script (fs.rmSync('dist', …)) now runs at the front ofbuild:win, so each packaged build starts from an emptydist/and only the current version's artifacts remain. (dist/outwere already gitignored, so this is disk hygiene, not a repo change.) - L85 —
.claude/launch.jsonis committed and misplaced. A VS Code launch config lives in.claude/(VS Code reads.vscode/launch.json), so no tool consumes it; it's also tracked (not gitignored). - L86 — History select-all + filter change deletes hidden rows.
selectedpersists across a filter change, so "Delete selected" can remove entries no longer visible (surprising). - L87 — History re-download ignores original options.
redownloadre-queues url/kind/quality only; the post-processingDownloadOptionsused originally are lost (current defaults apply). Fixed:HistoryEntrynow carries the originaloptions(recorded on completion inrecordCompletion), andredownloadreplays them viaAddOptions.options— so a re-download reproduces the same subtitles / SponsorBlock / container / codec instead of the current global defaults. Entries recorded before this (no stored options) fall back to the defaults as before. - L88 —
applyEvent('progress')can promote aqueueditem todownloadingoutsidepump()— a latent concurrency edge if a progress event ever arrives before/without the launch transition. - L89 — Inconsistent release artifacts. Only
0.4.1carries.sha256files (hand-made) and the version sequence skips 0.3.x — symptomatic of the manual release process behind H8. Fixed at the root by H8: scripts/generate-checksums.cjs (electron-builderafterAllArtifactBuildhook) writes a.sha256next to every built.exeand returns them for upload, so future releases are consistent automatically; the stale pre-H8dist/artifacts are pruned by L84. (The historical 0.3.x version gap is immutable.)
Round 5 (2026-06-29) — code-level micro-inconsistencies & polish:
- L90 — Two idioms for "value in const array." settings.ts
sanitizeOptionsusesARR.includes(x as never)(L116–120) whilesetSettingsuses(ARR as readonly string[]).includes(x as string)(L275/323). Pick one. - L91 — Unsafe tuple cast / fragile encoding.
value.split('|') as [MediaKind, string](SettingsView.tsx:411) trusts a'|'-joinedkind|qualitystring; a value without'|'yieldsundefinedquality. Use a typed pair, not a string. - L92 — Preload names diverge from main fn names.
markSourceItemDownloaded↔setMediaItemDownloaded,syncSources↔syncWatchedSources. Align the names across the boundary. Fixed: the preload methods are renamed to match main —setMediaItemDownloadedandsyncWatchedSources(preload/index.ts); the renderer callers (store/sources.ts) and the preview mock (mockApi.ts) updated to suit. TheApitype derives from the preload, so the rename is enforced end-to-end by the compiler. - L93 — SettingsView bypasses the store layer. It calls
window.apidirectly for cookies/ffmpeg/yt-dlp/app-update/backup while using stores for settings/templates/errorlog — mixed data-access in one component. Fixed (Batch 16, the CC13 view-model pass): each card's IPC + state machine moved into a colocated view-model hook on theuseDownloadBarpattern — useAboutCard / useBackupCard (reloads the settings/templates stores via new storereload()actions after a restore) / useCookiesCard / useSoftwareUpdateCard / useNetworkCard (PO-token mint); AppearanceCard's one-liner became a settings-store action (openHighContrastSettings). The sweep also cleared the two non-settings components: TerminalView's run/stream/stop → useTerminalRun, and LibraryView's scheduled-sync IPC → sources-store state/actions (scheduledSyncEnabled/loadScheduledSync/setScheduledSync/commitSyncTime). Nowindow.apiremains in any component (useDownloadBaris itself the view-model layer, per CC13). - L94 —
summarizeQueuerecomputed every render in DownloadsView.tsx (nouseMemo) — and again on every store change in App's taskbar effect. Memoize/share. Fixed: a 1-entry items-ref memoqueueSummaryOfin queueStats.ts — the store hands out a new items array per change, so both App's taskbar subscription and DownloadsView's render summarize it once per change (the second caller hits the cache) instead of twice (also closes PERF4). - L95 — "Sign in" hyphenation varies in the Cookies card ("Sign in…", "Site to sign in to", "Sign-in window", "Signing in…"). Reviewed: the usages already follow standard English — the verb is two words ("Sign in…", "Signing in…", "sign in to") and only the modifier is hyphenated ("Sign-in window"). Consistent by that rule; no change needed.
- L96 — Ellipsis-on-dialog-buttons inconsistent. "Export backup…/Import backup…/Sign in…" use
…(opens a dialog), but "Browse" / "Check for updates" / "Update now" don't. Adopt the…= opens-a-dialog convention. Fixed: the folder "Browse" buttons (they open the OS picker) are now "Browse…". "Check for updates" / "Update now" open no dialog (inline result / background download), so they correctly keep no ellipsis. - L97 — Advanced-field placeholders inconsistent. Adjacent fields use an example (
web_safari), a literal(none), andOptional — Gitea access token. Pick one placeholder style. Fixed: standardized on "an example value where the field has a representative one (web_safari,2M,socks5://…),Optional -- <what>for opaque optional fields" — the PO-token's uninformative(none)is nowOptional -- paste a token, matching the update-token field. - L98 — "Embed chapters" has no hint while every sibling toggle in DownloadOptionsForm does.
- L99 — URL input has an
idbut no label.input={{ id: 'aerofetch-url' }}with no<label for>/aria-label(placeholder only) — see M28. Resolved by M28: the URL input already carriesaria-label="Video or playlist URL", so it has an accessible name — the placeholder isn't the only affordance. - L100 — Inconsistent elevation. DownloadBar (
shadow4) and CommandPalette (shadow28) float; every other card (Settings, QueueItem, History, Library, Templates) is flat (border-only). Define an elevation scale. Fixed:ELEVATIONin tokens.ts names the scale —flat(border-only, the resting card),raised(shadow4, the floating download bar),overlay(shadow28, the command palette). The two floating surfaces use the tokens; flat cards are documented as the default. - L101 — No z-index scale. CommandPalette and Hint both hardcode
zIndex: 1000. Fixed:Z= { overlay: 1000, tooltip: 1100 }. CommandPalette's backdrop isZ.overlay, Hint's bubble isZ.tooltip(above overlays so a tooltip is never clipped by one). - L102 — Two mental models for "format." Settings sets it via one combined Video/Audio dropdown
(
FORMAT_OPTIONS); the DownloadBar uses a Video/Audio segmented control + a separate quality dropdown. Fixed: the Settings "Default format" control (DownloadsCard.tsx) now uses the same sharedSegmentedControl(Video/Audio) + qualitySelectas the DownloadBar, so both screens present one mental model. The combinedFORMAT_OPTIONS/kind|qualitystring encoding is gone — the toggle writesdefaultKindand the dropdown writesdefaultVideoQuality/defaultAudioQualitydirectly. - L103 — Two busy-indicator patterns. Fetch/Index swap the button icon to a
Spinner; the Settings check/update buttons keep their icon and render a separate adjacentSpinner. Resolved by L134 (Batch 8), verified this batch: every busy button swaps its own icon to a<Spinner size="tiny">(Fetch/Index/Check-for-new/yt-dlp update/app-update/PO-mint/version-check); the only 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. Fixed (Batch 17, with L161): HistoryView renders through the sharedVirtualListinside the newfillscreen shell — the list owns an internal scroll region (flex + 160px floor, DownloadsView's contract). The container-level Ctrl+A moved onto a wrapper around the virtualized scroller (key events bubble up from the rows), and per-row Delete stays on each row so it works wherever the row mounts. Verified live in an Electron probe against the vite preview at 1100×720 and 800×520: rows render via[data-index]virtual wrappers, the list scrolls internally, and<main>/body never scroll (probe metrics + screenshots). - L105 —
MediaKinddeclared twice. In both ipc.ts and store/downloads.ts; components import it from both. Single source it. Fixed: the store now importsMediaKindfrom@shared/ipcand re-exports it, so the duplicate localtype MediaKindis gone while existingimport { MediaKind } from '../store/downloads'sites still resolve. - L106 — Refresh icons used interchangeably.
ArrowClockwiseRegular(retry, re-download, check-for-new, update-yt-dlp) vsArrowSyncRegular(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>). - L109 — Versions auto-load with no indicator. yt-dlp/ffmpeg versions in About just appear;
only the manual "Check" button shows a spinner. Fixed (Batch 18, with UX26): the About card shows Fluent
Skeletonlines for the yt-dlp and ffmpeg versions while the mount loads them (checkingnow starts true and settles in afinally; an ffmpeg bridge failure settles to "not found" rather than a permanent skeleton). Verified in the preview probe that the card settles to the loaded state cleanly. - L110 — One muted-text token, many class aliases.
colorNeutralForeground3is re-declared ashint/sub/meta/srcSub/stats/count/emptyHint/previewMetaacross components. Fixed: oneuseTextStyles().mutedin ui/text.ts. Adopted for the pure-color aliases in the list views (QueueItemmeta, HistoryViewmeta, LibraryViewsub/srcSub/rowMeta, Sidebarcaption, DownloadBar/PlaylistPanelpreviewMeta/plItemMeta) — the color is identical, so no visual change. The combined classes (stats/count/emptyHint), the shared-across-cards settingshint, CommandPalette, Onboarding, and the inline styles migrate incrementally (same pattern M12 used for errorText). - L111 — Two helper-text mechanisms — Fluent
Fieldhintvs manual<Caption1 className={hint}>; SettingsView uses both. Resolved (convention clarified + verified): the two aren't interchangeable — FluentField hintis for help associated with a specific control (announced viaaria-describedby), and<Caption1 muted>is for standalone card intros and status lines ("Saved to…", "No errors yet"). Control help already usesField hintuniformly, so the split is role-appropriate rather than an inconsistency. - L112 —
Switchlabeling inconsistent. Settings/DownloadOptionsForm show "On"/"Off" text; Library switches usearia-labelonly (no visible state text). Fixed: dropped the dynamiclabel={x ? 'On' : 'Off'}from all 15<Field>-wrapped switches (DownloadOptionsForm ×6 + settings cards ×9). The Field already names each switch, and the toggle position +aria-checkedconvey state — so no switch shows redundant On/Off text now, matching Library's inline switches. - L113 — Tooltip ≠ aria-label. The Fetch button's Hint says "Fetch formats / playlist" but its
aria-labelsays "Fetch formats or playlist" (DownloadBar). Resolved by UX2: the Fetch button's Hint tooltip andaria-labelare now the same string ("Preview available formats (optional)"), so what a sighted user sees on hover and what a screen reader announces match. - L114 — Row titles use different components —
Text(QueueItem/History/Templates) vsspan(Library) vsCaption1elsewhere. Fixed:<Text>is the one row-title component. Library's three<span>titles (groupTitle,rowTitle,srcTitle) became<Text>— size-identical since Fluent's provider base font is alreadyfontSizeBase300. The duplicated semibold-ellipsis title style (QueueItem/HistoryView/DownloadBarpreviewTitle) is now the shareduseTextStyles().title. - L115 — Equivalent row actions labeled inconsistently — "Re-index" is icon+text; the
comparable "Retry"/"Remove"/"Re-download" are icon-only. Resolved (convention verified + role-appropriate):
the two live in different UI contexts, and each context is internally consistent. List-row item actions
(QueueItem's Pause/Cancel/Retry/Remove/Open, History's Re-download/Open/Show/Remove) are icon-only with a
Hinttooltip + a ≥40px hit target — a compact, repeated-per-row affordance. Card/toolbar actions (the Library source-detail row: Refresh/Remove/Download, the Downloads/History headers) are labeled icon+text — a one-per-card toolbar where the label aids scanning. The former "Re-index" is the Library card toolbar's "Refresh", correctly labeled alongside its siblings. No screen mixes the two within one context. - L116 — Empty-state structure varies — Downloads (icon + line), History (colored badge + line + sub-hint), Library (icon + line, no sub-hint).
Fixed: one shared EmptyState — a centered column with an
optional focal
icon(a hero glyph or History's colored badge), aBody1message at normal foreground, and an optional mutedhint. Adopted by Downloads, History, and Library, so the three now share one structure (History keeps its badge + hint; Downloads/Library pass a hero glyph). The per-viewempty/emptyHintstyles are deleted. - L117 — Terminal/error-log "empty" aren't centered blocks like the other empty states (inline text instead).
Fixed: both now render the shared
EmptyStatein itscompactvariant (smaller inset, for a placeholder inside an already-populated screen). The Terminal log centers "No output yet." in the log box (alogEmptyflex modifier); Diagnostics centers "No errors yet." — so both read as empty states rather than a stray line of inline text. - L118 — Cross-component focus via hardcoded id. App focuses the URL field with
document.getElementById('aerofetch-url')— brittle coupling into DownloadBar's markup. Fixed: the DownloadBar registers a focuser for its own URL input (a local ref) in the nav store (registerUrlFocuser); App callsuseNav.getState().focusUrlField()on launch / "New download" instead of querying the DOM by id (nav.ts, App.tsx, DownloadBar.tsx). Theid="aerofetch-url"is gone — App no longer reaches into DownloadBar's markup; the double-rAF (wait for mount/paint) stays so the focuser is registered before it's called. - L119 — "No items" copy varies — "No … yet" (most) vs "Output will appear here." (Terminal) vs "No errors logged." (Diagnostics).
- L120 — Fluent
Cardvs hand-styleddivcards. Only Settings/Onboarding use<Card>; every other card surface is a bespoke styled<div>. Resolved (convention documented + tokens applied): static content cards (Settings cards, Onboarding) use Fluent<Card>; interactive & list-item surfaces (the download bar, QueueItem, History/Library rows, the Library source card, the command palette) use the Batch-6 tokenized surface recipe —RADIUS.card/surface,ELEVATION,SPACEpadding on a styled<div>— because Fluent'sCardimposes its own internal padding/gap/interaction model that a custom row layout fights. Forcing<Card>onto those would regress layout for no gain; the divergence L120 flags is now a documented tier split, and the styled surfaces share the same radius/elevation tokens. (The larger "wrap every recurring pattern" primitive work stays UI15's incremental track.) - L121 — Raw scrim literal. CommandPalette backdrop is
rgba(0,0,0,0.32)(the only raw rgba in the renderer), identical in light/dark — low contrast over a dark UI. Fixed:SCRIMin tokens.ts aliases Fluent's semanticcolorBackgroundOverlay(the dialog scrim); the raw rgba is gone and the palette separates more cleanly from a dark UI. - L122 — "Checking…" indicated twice — the Settings update button changes its label to "Checking…" and a separate Spinner renders beside it.
- L123 —
Caption1is overloaded for hints, metadata, counts, and timestamps — one size doing four jobs. Resolved: the ui/text.ts header definesCaption1's role as "small secondary text" — hints/metadata/counts/timestamps are all that one role, now stated on purpose rather than by accident (paired with the sharedmutedcolor for L110). - L124 —
Body1vsTextrole overlap —Body1appears only in empty states/onboarding/gate;Textcarries titles; the split isn't principled. Resolved: the type-role convention in text.ts makes the split explicit —Body1= body copy (empty states, onboarding prose, gated notices);Text= row/item titles. Current usage already follows it. - L125 — Icon sizing has two syntaxes — px strings (
fontSize: '20px') on icon containers vs numericfontSize={28}on inline icons. Fixed: both syntaxes now single-source fromICONin tokens.ts (fontSize={ICON.control}on inline icons,fontSize: `${ICON.control}px`on the few makeStyles containers) — no more divergent magic numbers. Glyphs sized to a box rather than to this scale (brand/source tiles, the tinywatchBadgeicon, thumbnail placeholders) are documented exclusions and stay proportional. - L126 — Uneven text truncation — some titles/metas use
nowrap+ellipsis, others wrap; no shared truncation utility. Fixed: a shareduseTextStyles().truncate(text.ts) — single-line clip with ellipsis +minWidth:0— sits alongside the existingtitle(which istruncate+ semibold). Adopted for the ad-hoc copies: LibraryrowTitle, the DownloadBar playlistplItemTitle/plHeaderText(the header is nowtitle), and the sharedBannercontent (UI19).titlealso gainedminWidth:0so it shrinks correctly in a flex row. (Remaining correct-but-inline truncations migrate to the util incrementally.) - L127 —
Notificationsets no icon — completion toasts used the default Electron icon thoughgetAppIconPath()existed. Fixed with W13 (cachedgetAppIconImage()passed asiconat both notification sites). - L128 — Preview seed/mock data ships in production. The
PREVIEWruntime check can't be tree-shaken, so each store's seed arrays + fake ticker are bundled into the Electron renderer. Fixed: the three preview seed arrays (downloadSeed,sourcesseedSources/seedItemsBySource,historyseed) are now gated onimport.meta.env.DEV && PREVIEW. In the packaged renderer buildimport.meta.env.DEVis staticallyfalse, so Rollup constant-foldsfalse && …and dead-code-eliminates the whole seed literal; the browser dev preview (vite, DEVtrue) still gets them. (The smallPREVIEWfake-ticker function stays — it's tied to the runtime flag and is a few lines; the bulky part was the data, now dropped.) - L129 —
Hint alignis silently ignored for left/right placements yet callers still pass it. Fixed:HintPropsis now a discriminated union —alignis only accepted withplacementtop/bottom; passing it withleft/rightis a compile error instead of a silent no-op. No caller needed changing (the left/right hints already omitalign). - L130 — Error-string style is mixed — internal errors are lowercase/no-period ("timed out",
"interrupted"); user-facing ones are capitalized with periods. Fine when interpolated, jarring if shown raw.
Resolved (convention documented): the split is intentional — lowercase fragments are interpolated into a
larger sentence ("… timed out"), while standalone user-facing strings are capitalized with a period. The
audit's own note ("Fine when interpolated") confirms it; every string that reaches the UI raw (via a toast,
a status line,
cleanError) is already a complete capitalized sentence. Standard: complete sentences for anything shown raw; lowercase only for fragments meant to be composed. - L131 — Taskbar
errorstate is ephemeral — it's driven by liveerroritems, so "Clear finished" flips the red taskbar bar back to normal even though failures occurred (out of sync with the persisted error log). Resolved as by-design (Batch 20): the taskbar mirrors the live queue, and "Clear finished" is the user acknowledging those results — a red bar lingering after the queue is cleared would point at nothing the window can explain and would need its own dismiss affordance. Attention at failure time is already covered by the notification +flashFrame(W10), and the durable record lives in Diagnostics/errorlog. Judged on the state semantics rather than pixels; the running production instance on this machine was left undisturbed. - L132 —
clearProbe+ form-reset logic is duplicated in DownloadBar (onUrlChange,download,addPlaylist). Fixed: a sharedresetForm()in useDownloadBar.ts now owns the post-enqueue reset (URL + probe + the one-shot trim/schedule/advanced/incognito/options), called by bothdownload()andaddPlaylist().clearProbe()stays the lighter probe-only reset foronUrlChange. Also fixes a latent inconsistency —addPlaylistpreviously left the trim/schedule/advanced panels open after enqueuing. - L133 —
Fieldcontrols mixaria-label+ visiblelabel. ManyField label="X"wrap aSelect aria-label="X", so AT announces the name twice (extends M20 across DownloadOptionsForm). Fixed: the custom Select is now<Field>-aware (useFieldControlProps_unstable,supportsLabelFor: true), so a Field's visible label names the native<select>viahtmlForand its hint associates viaaria-describedby. Removed the now-duplicatearia-labelfrom every Field-wrapped select (DownloadOptionsForm ×4, AboutCard, AppearanceCard, CookiesCard ×2, DownloadsCard, CustomCommandsCard). The two standalone selects — the download-bar quality picker and the history type filter — keep theiraria-label, since there's no Field to name them. - L134 — Inconsistent disabled affordance for busy buttons. Some stay enabled-looking with a
spinner icon (Fetch/Index) while others are
disabledwith adjacent text (Settings) — no single "busy" convention. Fixed: one busy convention —disabledwhile working + the button's own icon swaps to a<Spinner size="tiny">. Fetch, Index, Check-for-new and the yt-dlp update button already did this; converted AboutCard's version-check button off its separate adjacent spinner to match. - L135 —
TextareavsInputfor similar fields. Trim uses aTextarea; the visually-similar single-line fields useInput— fine, but the trim/schedule panels and the template form mix both with no shared field style. Resolved: verified role-appropriate — the trim field is genuinely multiline (time ranges "one per line",resize="vertical"), soTextareais correct there andInputfor the single-line fields. Kept as-is; the shared field styling is covered by the Fluent<Field>wrapper they all use.
Round 6 (2026-06-29) — behavioral/logic edge cases:
- L136 — Incognito leaks outside history. Even if the (unreachable, M6) private flag were set,
download.tsstill writes failures toerrorlog.json(title + URL) and shows the title in completion notifications — the "private" promise covers only history. Fixed:incognitonow flows throughStartDownloadOptionsto main (the renderer's launch path passesitem.incognito), where main enforces the full "no logging, no history, no cookies" promise:logFailureskips the errorlog write (and the pre-spawn errorlog in ipc.ts too),notifyuses a generic outcome title and drops the detail body so no title/URL lands in the Windows Action Center, and both cookie sources (--cookies/--cookies-from-browser) are withheld so a private download can't be tied to the signed-in identity. (M6 wired the toggle; this makes the promise true in main, not just the renderer.) - L137 — Stuck 0% for unknown-size downloads.
parseProgressreturnsprogress: 0whentotal_bytes/_estimateare absent (livestreams, some sites), so the bar reads 0% the whole time instead of an indeterminate state. Fixed:parseProgressnow setssizeUnknown: !totalBytesonDownloadProgress; the store threads it onto the item andQueueItemrenders an indeterminate bar + "Downloading…" (instead of a frozen 0%) when it's set, reusing the existing SR7 indeterminate path. - L138 — Clipboard read on every window focus. useClipboardLink.ts
reads the full clipboard on each
focus(and once on mount, possibly beforeclipboardWatchhas loaded — FALLBACK istrue). Reading all clipboard text whenever focused may surprise privacy-conscious users. Fixed: the check now gates on the settings store'sloadedflag as well asclipboardWatch, so the clipboard is never read using the pre-load fallback (the mount-before-settings-load window); the effect re-runs whenloadedflips true, so a link copied before launch is still offered once the real setting is known. (Per-focus reads were already gated onclipboardWatch; the privacy default itself is SR4.) - L139 — Auto-update vs concurrent spawn race.
runStartupYtdlpAutoUpdatecan overwriteyt-dlp.exeon launch while a deep-link/queued download spawns the same binary — a Windows file-in-use/corrupt-spawn window. Fixed with a spawn↔update interlock in new ytdlpLock.ts (pure, unit-tested in test/ytdlpLock.test.ts). Each yt-dlp spawn registers viaacquireSpawn/releaseSpawn(download.ts + terminal.ts, released on every settle path incl. the stall watchdog);updateYtdlptakes the lock withbeginYtdlpUpdate, which REFUSES when any spawn is live so the best-effort daily update simply defers a cycle rather than rewrite the exe under a running process. The reverse direction is a HOLD, not a failure (per the audit's wording): thedownloadStart/terminalRunIPC handlersawait whenYtdlpUpdateSettled()before spawning, so an auto-resumed persisted-queue download that lands during the ~2–5 s rewrite waits it out and then proceeds (bounded by the update timeout —endYtdlpUpdatealways runs in afinally).runStartupYtdlpAutoUpdatealso skips (without recording the daily check) if a spawn is already live. The real driver was confirmed live via queuePersist.test.ts: adownloadingitem is persisted and restored asqueued, so persisted downloads DO auto-resume at launch — exactly the collision this closes. The Windows exe-swap race itself is a milliseconds-wide, non-deterministic window that can't be reproduced on demand, but the interlock provably serialises the update against every spawn path (302 tests green). - L140 — Retry-during-teardown race.
retryreuses the same id; if the previous process'sclosehandler hasn't removed it fromactiveyet,startDownloadreturns "already running" → markError. Fixed in download.ts:cancelDownload/pauseDownloadnow drop the item fromactivesynchronously (so a retry/resume's same-id spawn is never rejected by the stale entry), and the doomed child'sclose/error/watchdog release the slot through a newreleaseActive(id, rec)that only deletes when that rec still owns the id — so a late teardown can't evict the fresh spawn. The transient cookie jar is now keyed per spawn (-${spawnSeq}) so the old download's cleanup can't unlink the new one's file. (with L148) - L141 — Renderer history grows unbounded in memory.
useHistory.addprepends without a cap while main capshistory.jsonat 500; in a long session the in-memory list exceeds the persisted cap (resets on reload). Fixed in store/history.ts:addnow.slice(0, MAX_ENTRIES)(500, the same cap as main'shistory.ts) and de-dupes byurlas well asid, so the optimistic in-memory list matches what a reload would show (also aligns with main's M35 url de-dup). - L142 — IPC mutation return values are discarded everywhere. history/templates/sources/settings
IPC calls return the authoritative (validated/capped/sanitized) state, but every renderer caller does
optimistic-only updates and ignores it — client and persisted state can silently diverge until reload (generalizes M34).
Fixed (Batch 16, the focused reconciliation PR): a keyed latest-wins reconciler
(lib/reconcile.ts, unit-tested in
test/reconcile.test.ts) applies main's authoritative return on every mutation —
history add/remove/removeMany/clear, template save/remove, and sources setWatched/removeSource/
setMediaItemDownloaded (keyed per source so overlapping completions don't suppress each other) — extending
the M34 settings pattern to all four stores. The seq guard drops a stale response when a newer call was
issued on the same collection, so an overlapped pair can't transiently roll back the newer optimistic
update;
loadSourcesshares the mutations' key so a slow list read can't clobber a newer authoritative result either. - L143 — No in-window "quit anyway." With a download active (or tray mode), closing only hides;
quitting requires the tray menu. If the tray ever fails to create, the only exit is Task Manager
(mitigated today by the embedded fallback tray icon).
Fixed in index.ts: when the window is closed while a download is running AND the user is
NOT in minimize-to-tray mode (i.e. the app is only staying alive because of the download), the close handler
now shows a native, tray-INDEPENDENT
dialog.showMessageBox— "Keep downloading" (hide to background) vs "Quit anyway" (which callsmarkQuitting()+app.quit(), the same path the tray Quit uses). AquitPromptOpenguard stops the dialog stacking if the X is hammered. This replaces the old passive once-per-process "still running" notification (whose own body told users to "use the tray icon to … quit" — the very dependency this removes), sonotifyBackgroundOnce+ itsnotifiedBackgroundlatch /show-reset (L68) are removed as dead code. A deliberate minimize-to-tray close still hides silently (the user opted in). Verified the modified main process launches cleanly — the removedNotification/getAppIconImageimports and the new dialog wiring load and create the window with no error; typecheck + lint + 302 tests + production build green. The dialog is a standardshowMessageBox(same family as the app's folder picker) and "Quit anyway" reuses the proven quit path. - L144 — Duplicate guard checks the queue only, not history. Re-adding a URL downloaded earlier
(already cleared from the queue) gives no "already downloaded" hint (roadmap notes this as a possible extension).
Fixed: the DownloadBar dup guard (useDownloadBar.ts)
now checks history when a URL isn't in the live queue — if it was downloaded before, the warning banner reads
"Already downloaded: …" (vs "Already in your queue: …"), with the same "Download anyway" confirm. A new
dupInHistoryflag drives the wording. - L145 —
useClipboardLinkre-offers a dismissed link after a tab switch.lastSeenis per-hook-instance; switching Downloads↔Library remounts it, so a previously-dismissed clipboard link is offered again. Fixed:lastSeenis now a module-level variable (not a per-instanceuseRef), so a dismissed/accepted clipboard link stays suppressed across hook remounts. Only one field is mounted at a time (tab switch), so there's no cross-field contention. - L146 —
parseTrimSectionsaccepts malformed multi-colon times. The\d+(?::\d{1,2})*pattern passes tokens like1:2:3:4-5:6:7:8, which then reach yt-dlp's--download-sectionsand fail there rather than being rejected up front. - L147 — macOS branches in a Windows-only app.
app.on('activate')and theprocess.platform !== 'darwin'guard inwindow-all-closedare dead on Windows — boilerplate that implies multi-platform support the app doesn't ship. - L148 —
clearFinished/remove can free a slot before the killed process exits. Removing a just-canceled item letspump()launch into the "freed" slot whiletaskkillis still tearing down the prior tree — a brief window over the concurrency cap. Fixed with L140:cancelDownload/pauseDownloadrelease theactiveslot the instant they issue the kill (rather than on the asyncclose), so main's concurrency accounting matches the renderer immediately — the just-promoted next item is neither falsely rejected by the maxConcurrent guard nor run over the cap while the prior tree dies.
Round 7 (2026-06-29) — copy, a11y patterns & micro-behavior:
- L149 — Hint length wildly inconsistent. Most field hints are one short line, but "Keep running in the tray" is a three-sentence paragraph with a parenthetical. Normalize hint length/voice. Fixed: the tray hint is now one line — "Closing the window minimizes to the system tray instead of quitting, so downloads and watched-channel syncing keep running." — matching the one-line voice of its sibling hints.
- L150 — "PO token" capitalization varies —
PO Token(ipc.ts/ROADMAP) vsPO token/Proof-of-Origin token(SettingsView). Pick one. - L151 — Mixed range dashes. En-dash in prose ("2–3 is a good balance") vs hyphen in time-range placeholders ("1:30-2:00"). Choose one convention.
- L152 — Search-field sizing differs per screen. History
minWidth 180 / maxWidth 320, Settings inlinewidth: 100%, DownloadBarflexGrow. No shared search-input width. Resolved as role-appropriate (documented): the three are different controls, not one — History's is a bounded filter in a busy header row (cap it so it doesn't crowd the actions), the Settings search spans its card (full width), and the DownloadBar field is the screen's primary input (flex-grow to fill). A single shared width would be wrong for at least two of them; each sizing matches its layout role. - L153 — Generic "Dismiss" aria-labels. Three close buttons (DownloadBar suggestion + dup, Library suggestion) all read just "Dismiss" — ambiguous to a screen reader. Name what's dismissed.
- L154 — Instructional text in an aria-label. Sidebar theme-cycle button is
aria-label="Theme: Dark. Click to change."— "Click to change" is UI instruction, not a name. - L155 — Accent swatches aren't a
radiogroup. They're a single-select set rendered as plain buttons witharia-pressed, while the Theme controls (also single-select) userole="radiogroup"/radio. Inconsistent single-select a11y pattern. Fixed: the accent swatches are now a realrole="radiogroup"withrole="radio"+aria-checkedswatches, roving tabindex, and ←/→/↑/↓/Home/End arrow-key navigation + the shared focus ring (AppearanceCard.tsx) — matching theSegmentedControlradiogroup pattern used elsewhere. The redundant "(selected)" text in thearia-labelis gone (aria-checked conveys it). - L156 —
datetime-localschedule has nomin. Past times are selectable, then silently download now (extends L57). Addmin={now}. - L157 —
removeSourcedoesn't cancel in-flight downloads. Removing a source while its videos are downloading lets them finish into the removed source's folders;markDownloadedthen no-ops on the gone item. Fixed:removeSourcenow emits asourceRemovedevent on the coordinator bus (C2); the downloads store subscribes and cancels every active (downloading/queued) item whosemediaItemIdstarts with${sourceId}:— MediaItem ids are${sourceId}:${videoId}(indexerCore), so the prefix reliably identifies the source's downloads. Reuses the provencancel()path (process kill + pump). Live-smoke pending (per the mark-done convention): implemented + type/lint/test-green, but the actual process-kill-on-remove is worth a watched confirm in the Batch 15 live pass. - L158 — Drag highlight flickers.
onDragLeave={() => setDragActive(false)}fires when the cursor crosses child elements (no enter/leave depth counter), so the dashed-outline blinks during a drag. Fixed: useDownloadBar.ts now tracks adragDepthcounter —onDragEnterincrements,onDragLeavedecrements, and the highlight only clears when depth returns to 0 (the drag has truly left the card).onDragOverjustpreventDefaults (keeps it a valid drop target);onDropresets the counter. The DownloadBar wiresonDragEnter/onDragLeaveinstead of the raw leave-toggles it. - L159 — Dead fallback branch.
copyErrorReportfalls back to "No errors logged." but its button is disabled when there are no entries, so the fallback is unreachable. - L160 — One-off inline margins.
marginTopliterals (Onboarding 2px, QueueItem 4px, SettingsView 8px) instead of style classes (extends L5). Fixed: the margin literals now draw fromSPACE(hairline/xtight/tight) — same values, one source. (The broader inline-stylevs makeStyles split is still L5.) - L161 —
height: 100%screens inside a padded scroll container. Downloads/Terminal setheight: 100%while App's<main>isoverflowY: auto+ 24/28px padding — a fragile coupling that can double-scroll or misalign the virtualized list's viewport. Fixed (Batch 17):<main>no longer scrolls or pads — it's a plain flex slot, and each screen owns its scrolling through two shared shells in ui/Screen.tsx:page(document screens — Library, Settings, empty History: the shell scrolls) andfill(internally-scrolling screens — Downloads, Terminal, History: the screen's list/log is the scroller, with the shell scrolling only as the short-window fallback). Theheight:100%couplings are gone (flexGrow:1; minHeight:0inside the shell instead). The rework surfaced a real pre-existing edge: on a very short window the flexed list collapsed to 0px — now floored at 160px with the shell absorbing the overflow. Verified with an Electron probe at 1100×720 and 800×520 per screen: exactly one active scroller per screen in the normal case,<main>/body never scroll, floors hold (probe metrics + screenshots). - L162 — Per-thumbnail store subscription. Every
MediaThumbcallsuseResolvedDark()(two store subscriptions) instead of receiving a resolved theme prop — N subscriptions per list. Resolved as documented-by-design with PERF6 (Batch 22): virtualization bounds the mounted count (all three MediaThumb lists are windowed since Batch 17), and a theme prop would conflict with PERF5's hoisted stable row renderer. See PERF6 for the full rationale. - L163 — QueueItem makes 9 separate
useDownloadsselector calls instead of one destructured selection — repeated boilerplate per row. Fixed: oneuseShallowselection replaces the 10 individual subscriptions in QueueItem.tsx (the actions are stable, so the shallow compare never re-renders the row).
Round 8 (2026-06-29) — formatting micro-inconsistencies:
- L164 — Metadata separator glyph/spacing is inconsistent. Meta lines join with
' • '(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). Fixed: oneMETA_SEP = ' • 'in tokens.ts, adopted at every meta-line join (QueueItem/History/DownloadBar/DownloadsView/Library/PlaylistPanel). The' · 'inside compound quality labels ("720p · mp4 · 184 MB") is a data format parsed by HistoryView and is intentionally left as-is. - L165 — The two byte/speed formatters round differently.
download.ts fmtBytesusestoFixed(v >= 100 ? 0 : 1)("512 MB") whilequeueStats formatSpeedusestoFixed(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) andformatEta(queueStats) emitM:SSonly, so a 2-hour ETA renders as "120:00", whereasfmtDuration(indexerCore) correctly rolls toH:MM:SS. Unify on the hour-aware format. - L167 —
PROGRESS_TEMPLATEis exported but used only insidebuildArgs.ts— a superfluous public export (download.ts parses theprog|lines independently). Make it module-private.
Round 9 (2026-06-29) — type-safety & build config:
- L168 —
noUncheckedIndexedAccessis off. Array/index/tuple access is typed as always-defined, so the exact edge cases already filed compile clean: thesplit('|') as [MediaKind, string]cast (L91/CL2) andparseProgress's positional destructure of a possibly-shortsplit('|'). Enabling it would surface them at build time. - L169 —
noFallthroughCasesInSwitchis off. The largesetSettingsswitch andapplyEventswitch aren't fallthrough-guarded; a missingbreak/returnwouldn't be caught. - L170 — No source maps in production.
sourceMap: false(toolkit) and nobuild.sourcemapinelectron.vite.config.ts, so field crash stack traces are unmapped — combined with no logging (CC8/M29), diagnosing a user-reported crash is very hard. Consider hidden/external source maps. Fixed:build.sourcemap: 'hidden'on all three bundles (main / preload / renderer) in electron.vite.config.ts —.mapfiles are emitted for offline symbolication of the raw stacks the logger now records (CC8), with nosourceMappingURLreference in the shipped bundle. - L171 —
main.tsxmock drift. The preview mock's version strings disagree (getAppVersion→'0.4.0-preview' vscheckForAppUpdate.currentVersion→'0.4.0'), and itssetSettingsskips the real validation/sanitization — so the browser preview can accept settings the app rejects, masking M34-class issues during design. Fixed the version drift: oneMOCK_CURRENT_VERSIONconstant in mockApi.ts now backs bothgetAppVersionandcheckForAppUpdate.currentVersion, so they agree (kept below the mock "latest" so the updater card still demos an available update). ThesetSettings-skips-validation point is left as-is on purpose: the mock is a browser-only design aid with no access to main'ssettings.tsvalidators; replicating them in the mock would be a second source of truth to drift. M34/M24 already make the real app authoritative, so the design preview being permissive doesn't affect shipped behavior. - L172 —
skipLibCheck: true(toolkit) hides type errors in dependency.d.tsfiles — standard practice, low risk, noted for awareness (a dep type regression won't fail typecheck). Acknowledged — kept as-is (it IS standard practice and speeds typecheck); recorded so the trade-off (a dependency.d.tsregression can pass typecheck silently) is a known, accepted decision rather than an oversight.
UI consistency
Cross-screen review (Downloads, Library, History, Terminal, Settings, Onboarding, Command palette,
Sidebar, cookie sign-in window). Each item: what's inconsistent → Standard: the single rule to
adopt. Items prefixed UI; where a dimension was already filed, the existing ID is referenced
instead of re-filing. The app uses Fluent tokens well (theme-awareness is mostly solid); the gaps
are an undefined spacing/radius/type scale and a drift between Fluent components and hand-rolled ones.
Layout, spacing & width
- UI1 — No shared content width. SettingsView is
maxWidth: 640px(SettingsView.tsx), but Downloads/Library/History/Terminal are full-width; Onboarding card is 460px, CommandPalette 560px. On a wide window Settings is a narrow column while siblings stretch edge-to-edge. Standard: one reading-width token (e.g. 720px) applied by a sharedScreenwrapper, or commit to full-width everywhere. - UI2 — Per-screen vertical rhythm differs. Root section gap is 16 (Settings/Terminal), 20
(Downloads), 18 (Library), 12 (History). Standard: a single section-gap (16px) via a shared screen wrapper.
Fixed: all five view roots now use
SPACE.section(16) — Downloads 20→16, Library 18→16, History 12→16; Settings/Terminal were already 16 and are now tokenized. - UI3 — Card padding has no scale. 14 (QueueItem) / 16 (DownloadBar) / 20 (Settings card) /
32 (Onboarding) /
10px 12px(History row) /12px 14px(Library head) /8px 10px(template row). Standard: a padding scale (e.g. control 8–10, list-row 12, card 16, hero 24) keyed to surface tier. Done (Batch 18): the remaining list-row/control paddings migrated to theSPACEscale — History row and Downloads summary →snug/cozy, Library head/detail and Terminal gate/log and the template form →cozy(12, the list-row tier; the odd 14px horizontals snapped down), template row →tight/snug. The earlier card-tier deltas (20→16 settings cards, 14→16 queue rows) plus these were eyeballed via the Electron-probe screenshot pass (light + dark, Downloads/History/Library/Settings): nothing reads cramped or loose; kept. - UI4 — Asymmetric app content padding.
padding: '24px 28px'(App.tsx) — 28px horizontal appears nowhere else. Standard: symmetric or scale-based padding. Fixed: the<main>content padding is now the symmetricSPACE.page(24px). - UI5 — Action-cluster gaps split 4px vs 8px. Icon-button rows use 4px (QueueItem/History/
TemplateManager) but DownloadBar/Library action rows use 8px. Standard: 4px for icon-button groups, 8px for labelled-control rows. (Good baseline already: every empty-state uses
56px 16px— keep it.) Resolved: the existing split already matched the standard; it's now named inSPACE(xtight= 4 for icon-button clusters,tight= 8 for control rows) and adopted where touched, so the rule is explicit rather than incidental.
Corner radius
- UI6 — Thumbnail radius varies for the same element. QueueItem
Large, HistoryMedium, Library rowThumbSmall, DownloadBar previewThumbMedium. Standard: one thumbnail radius (Medium/10px). (Distinct from L9, which is thumbnail pixel dimensions.) Fixed: every thumbnail usesRADIUS.control(Medium) — QueueItem Large→Medium and Library rowThumb Small→Medium; History/DownloadBar were already Medium and are now tokenized. - UI7 — Card radius (XLarge vs Large) has no stated rule. Top-level cards use XLarge (16),
list rows Large (12), but it's applied by feel. Standard: document surface tiers — page-card XLarge, list-item Large, control Medium — and apply uniformly.
Fixed:
RADIUSdocuments the tiers (cardXLarge /surfaceLarge /controlMedium) and they're applied on the surfaces touched this batch (page cards →card, the QueueItem list-item →surface, thumbnails/controls →control); remaining correct-but-untokenizedborderRadius*calls migrate incrementally. - UI8 — Circular shapes mix token and literal.
borderRadiusCircular(Library pill/watchBadge) vs literal'50%'(SettingsView swatch, History emptyBadge). Standard: always the token.
Typography
- UI9 — Screen titles are inconsistent. Most screens use
<Subtitle2>(Downloads "Queue", Library, Terminal, Settings cards); Onboarding uses<Title2>; History has no title at all (just a count). Standard: one page-title style on every screen. - UI10 — Only Library has a screen subtitle/description. Others jump straight to content. Standard: a consistent header block (title + optional one-line description).
- UI11 — Semantic type ramp vs ad-hoc px. Sidebar
brandName/navItemand LibrarywatchBadge/pillset rawfontSizeBaseXXX;sectionIcon/markuse literal'20px'/'26px'. Standard: Fluent type components/tokens; no literal px font sizes. Fixed: no literal px font sizes remain — the last tile/badge glyphs snap toICONtiers (Sidebar mark 20, Onboarding mark 24, Library srcIcon 20, History emptyBadge 24). ThefontSizeBase*usages already met the standard (they're tokens, on interactive<button>/pill elements that can't be Fluent type components). The four ≤2px tile-glyph deltas join UI12's watched visual pass.
Icons
- UI12 — No icon-size scale. Inline
fontSizeof 11/14/16/18/20/22/26/28/40 is scattered across components. Standard: a small set (16 inline / 20 control / 24 section / 40 empty-state). Done (Batch 18): theICON= 16/20/24/40 tiers were already in place; the deferred eyeball of the shifted glyphs (18→20 sidebar nav/theme toggles, 20→24 settings card headers, 14→16 private badge) happened in the Electron-probe screenshot pass — the nav icons sit balanced in their 20px tier, the card-header glyphs read as proper section markers at 24, nothing looks oversized. Box-proportional glyphs (brand/source tiles,watchBadge, thumbnail placeholders) remain documented exclusions. - UI13 — Regular vs Filled weight mixed for one concept.
ArrowDownloadRegular(nav, buttons) vsArrowDownloadFilled(brand mark). Standard: one weight per concept (Filled only for brand/emphasis). Verified:ArrowDownloadFilledappears only on the two brand marks (Sidebar, Onboarding); every other download arrow (nav, buttons,contentBefore) isArrowDownloadRegular. The convention already held — the only Filled glyphs elsewhere are the status Checkmark/Error circles, which are deliberate state emphasis.
Buttons & controls
- UI14 — Two hand-rolled segmented controls. DownloadBar kind toggle (
segment, padding7px 16px) and Sidebar theme toggle (themeSeg, padding7px 4px) are separate implementations with different padding/markup. Standard: one sharedSegmentedControl. - UI15 — Raw
<button>s alongside Fluent<Button>. Sidebar navItem/iconBtn/themeSeg, DownloadBar segment/plKindBtn, CommandPalette item, and LibrarygroupHead(arole="button"div) are bespoke. Standard: wrap recurring patterns so hover/focus/disabled are uniform (ties to L5/M15). - UI16 — Button size hierarchy isn't applied uniformly. DownloadBar
large; Library "Index"largebut its toolbarsmall; History allsmall; Settings mostly default. Standard: size-by-role (screen primary = large; secondary = medium; row actions = small). Done (Batch 18): the usage the item lists already matches the size-by-role rule (each screen's one primary islarge— Download/Add-to-queue, Library Index; row/toolbar actionssmall; settings cards default) — confirmed screen-by-screen in the screenshot pass, no tweaks needed. The convention is now codified in ui/tokens.ts alongside the other scales so it's a rule, not an accident. - UI17 —
appearance="secondary"used in only two spots (Library "Check for new", Terminal "Stop") while every other non-primary button issubtle. Standard: pick subtle or secondary as the standard non-primary appearance. Fixed: both switched tosubtle(the app's standard non-primary appearance — used by every other non-primary button). - UI18 — Two status-chip systems. QueueItem uses Fluent
Badge; LibraryView uses custom colorpillspans for the same item-status concept (and labels differ — see M8). Standard: one shared status-chip component + label map. - UI19 — Suggestion-banner CSS duplicated. Identical
suggestion/suggestionTextstyles in DownloadBar and LibraryView. Standard: a sharedLinkSuggestioncomponent (also kills drift). Fixed: a shared Banner (leading icon + truncating/wrapcontent + trailing actions,tone= brand/warning) and a LinkSuggestion built on it. The DownloadBar's copied-link suggestion and the Library's are now one<LinkSuggestion>; the DownloadBar's channel/playlist nudge (wrap) and duplicate warning (tone="warning") reuse the baseBanner. Five hand-rolled style blocks (suggestion/suggestionText/channelHint/channelHintText/dupRow) are deleted — the tint/padding/radius are identical, so no visual change.
Colors & selection
- UI20 — "Selected/active" styling is split. Solid brand (
colorBrandBackground+ on-brand text) for DownloadBar/Sidebar segments, but brand-tint (colorBrandBackground2+colorBrandForeground2) for Sidebar nav, CommandPalette item, Library pill. Standard: one active treatment per control class. Resolved (rule named + verified): one active treatment per control class — toggles (the sharedSegmentedControl, used by both the DownloadBar kind toggle and the Sidebar theme switch after UI14) use solid brand; selection within a list (Sidebar nav item, CommandPalette active option) uses brand-tint. Each class is now internally consistent, and the rule is documented on SegmentedControl. The "Library pill" the item cited is gone — UI18 replaced it with the sharedStatusChip, so there's no third treatment left to reconcile. - UI21 — Sidebar active-nav rail is expanded-only. The inset brand box-shadow shows only when
expanded; collapsed relies on tint alone. Standard: a consistent active indicator in both states.
Fixed (Batch 18): the inset rail moved from a conditional inline style into
navItemActiveitself, so it shows in both states. Screenshot-verified collapsed: the 3px brand rail hugs the row's left edge and reads clearly beside the active-downloadCounterBadgepinned at the icon's top-right — no collision. - UI22 — Brand-icon tiles differ.
mark(on-brand on solid brand) vssrcIcon(brandForeground2 on brand-tint) vssectionIcon(compoundBrand, no tile). Standard: a defined icon-emphasis set. Resolved (the set is now defined + documented): three deliberate emphasis levels — brand mark (mark, on-brand glyph on solid brand: the app/onboarding identity, highest emphasis), entity tile (srcIcon, brandForeground2 on brand-tint: a Library source, medium), and section glyph (sectionIcon, compoundBrand, no tile: a settings-card header, lowest). They should differ — one per role; the levels are now named rather than incidental, and each is applied consistently.
Dialogs, menus, status, navigation
- UI23 — Fragmented overlay/dialog system. Native OS dialogs (pickers, backup save/open,
backup warning
messageBox), one custom React overlay (CommandPalette), a full-screen view (Onboarding), and an unstyled separateBrowserWindow(cookie sign-in) that shows none of the app's theme/chrome. Standard: keep native for file/confirm; unify in-app overlays under one themed primitive; theme the sign-in window's title/background to match. Fixed (Batch 20, the actionable remainder): the cookie sign-in window — and the PO-token window, its sibling on the same login partition — now open withbackgroundColorset to the app's resolved theme (PAGE_BACKGROUND[resolveBackgroundMode(...)], supplied by the IPC layer to avoid an import cycle), so neither flashes default-white chrome before the remote page paints; their title bars already follow the app theme via the W3nativeTheme.themeSourcebridge. In-app overlays were already unified on the non-portalZ/SCRIM/ELEVATIONtokens; native file/confirm dialogs stay native by design. - UI24 — No context menus anywhere. Every row exposes actions only as inline buttons;
right-click does nothing on any screen, despite many per-row actions that conventionally also live on
right-click. Standard: add consistent right-click menus mirroring row actions, or note "none by design."
Resolved as "none by design" (documented): every row action is already a visible inline button with a
tooltip, and lists support keyboard operation (Delete, Ctrl+A). Fluent's
Menuis portal-based, and this app deliberately avoids portal overlays — they blank/flicker on the dev-machine GPU (the same reason Select/CommandPalette/Toaster are hand-rolled non-portal; see the gpu-flicker note). A hand-rolled non-portal context-menu primitive is a sizable build with little to add over the always-visible actions, so right-click menus are intentionally not provided. Revisit if the GPU/portal issue is resolved. - UI25 — No in-app global status surface. Queue progress shows only on the Downloads tab's
summary strip; on any other tab there's no in-app sign downloads are running (only the OS taskbar).
Standard: a persistent affordance (e.g. a sidebar "Downloads" badge with the active count).
Fixed (with UX9): a
CounterBadgeon the sidebar Downloads nav item shows the active (downloading + queued) count from every tab, and a new in-app toast surface covers transient status. The exact suggested affordance.
Animations & transitions
- UI26 — Lone animation. Only the sidebar width transitions (
0.15s ease); tab switches, panel expand/collapse, banners, and Library card expansion are all instant. Standard: one motion policy — either add subtle, consistent transitions (gated onprefers-reduced-motion) or drop the sidebar one. Fixed (policy established): the sidebar transition now uses the sharedMOTIONtiming (durationFast + curveEasyEase), and a global@media (prefers-reduced-motion: reduce)rule in base.css disables every transition/animation for users who ask for less. New transitions adoptMOTION; none were added speculatively (compositing overlays flicker on the dev GPU — see the gpu-flicker note).
Hover, focus, disabled, keyboard & accessibility
- UI27 — Command palette uses JS hover, not CSS. Items highlight via
onMouseEntersettingitemActive(no:hover), unlike every other list, and expose noaria-selected/active-descendant, so the highlight is invisible to screen readers. Standard: CSS:hover+ listbox/option semantics. Fixed in CommandPalette.tsx: the input is now arole="combobox"witharia-controls/aria-activedescendant/aria-autocomplete="list", the list is arole="listbox", and each row is arole="option"witharia-selected— so the active row is announced to Narrator via active-descendant (the standard combobox pattern; focus stays on the input, so the rows became non-focusabledivs rather thanbuttons).onMouseEnteris kept deliberately: it unifies the pointer and keyboard highlight onto oneselstate, which — now thataria-activedescendanttracks it — is more correct than a pure CSS:hover(which wouldn't move the announced selection). - UI28 — Command palette input has no focus ring. The search
<input>setsoutline: 'none'with no replacement. Standard: a visible focus ring (Fluent stroke token). (a11y) - UI29 — Fragmented focus treatment. Fluent ring (Fluent controls) vs custom border (Select)
vs removed (palette input) vs UA default (Sidebar buttons, segments, and Library
cardHead/groupHeadwhich arerole="button" tabIndex=0with no:focus-visible→ invisible keyboard focus). Standard: one focus-ring style on all interactive elements, custom and native. (a11y; extends L14) - UI30 — Hand-rolled radiogroups aren't arrow-navigable. DownloadBar kind and Sidebar theme
use
role="radiogroup"/radiobut implement click only — no ←/→ roving focus a radiogroup implies. Standard: roving-tabindex arrow keys, or Fluent's RadioGroup. (a11y) - UI31 —
Selectcan't be disabled. The native-<select>wrapper exposes nodisabledprop, so that control can't show a disabled state while Fluent controls can. Standard: adddisabled+ styling. Fixed in Select.tsx: adisabledprop passes through to the native<select>and applies a disabled style (disabled foreground/background/stroke tokens +not-allowedcursor, hover suppressed) so it reads like a disabled Fluent control. - UI32 — Native-control dark mode is uneven. The DownloadBar
datetime-localsetscolorScheme: 'light dark'explicitly; theSelectrelies on the rootcolorScheme. Standard: setcolorSchemeconsistently on both. Fixed in downloadBar/styles.ts: dropped the explicitcolorScheme: 'light dark'on thedatetime-localso it inherits the resolved in-app scheme set on the app root (App.tsx) — the native calendar popup now follows the app's Light/Dark theme, consistent with the native<Select>, rather than tracking the OS preference. - UI33 — No semantic headings. Titles render as
Subtitle2/Title2(styled spans), so there's no h1–h6 hierarchy for screen-reader heading navigation (landmarks<nav>/<main>exist; headings don't). Standard: render titles as real headings (as="h1"/"h2"orrole="heading" aria-level). (a11y) Fixed by rendering the Fluent typography titles as real headings (theasprop keeps the visual ramp): the shared ScreenHeader title is now<Subtitle2 as="h1">, giving every screen (Downloads, History, Library, Settings, Terminal) oneh1in one place; the DownloadsView "Queue (N)" and all 11 settings cards are<Subtitle2 as="h2">sections beneath it; and the Onboarding "Welcome" title is<Title2 as="h1">. A minimalh1–h6margin reset in base.css zeroes the UA heading margin (Fluent's typography classes out-specify the element selector, so the visual ramp is unchanged) so the layout matches the former spans.
Top cohesion-breakers (the "not one product" shortlist)
- Status chips — Fluent
Badge(Downloads) vs custom color pills (Library) for the same concept (UI18, M8). - Segmented controls — two separate hand-rolled versions (UI14).
- Buttons — Fluent
<Button>vs many bespoke<button>s with divergent hover/focus (UI15, UI29). - Content width — Settings is a 640px column; every other screen is full-width (UI1).
- Focus rings — four different focus treatments incl. one removed and several invisible (UI28–UI29).
- Screen headers — Subtitle2 / Title2 / none (UI9–UI10).
UX review (first-run perspective)
Walking the app as a new user. Ranked by severity. UX IDs; existing IDs referenced where the
root cause is already filed.
High — core-flow friction
- UX1 — Per-download options are unreachable; you must change global Settings for one video.
The DownloadBar exposes only URL · Video/Audio · quality · Trim · Schedule. There is no
per-download post-processing panel, custom-command panel, command preview, or incognito toggle —
so
DownloadItem.options/extraArgsand the whole Phase A/C per-download-override plumbing are unreachable; every download usessettings.downloadOptions. To grab one video as MP3-with-subs a user must edit global Settings, download, then revert. (Supersedes/expands M5, M6.) Fix: add a collapsible per-download Options panel (reuseDownloadOptionsForm) — the plumbing already exists. Fixed: a collapsible "Advanced" panel in DownloadBar.tsx reusesDownloadOptionsFormfor per-download overrides (audio format, container, codec, format-sort, subtitles, SponsorBlock), passed viaAddOptions.options→startDownload. The panel also carries the M6 incognito checkbox and sits beside the M5 command preview. Overrides are one-shot: they reset to the global defaults after each download (matching Trim/Schedule). - UX2 — "Fetch" vs "Download" is ambiguous, and Enter does the wrong thing. Two unlabeled
icon buttons (magnifier = "Fetch", clipboard = "Paste") sit next to "Download"; a first-timer can't
tell that Download works without Fetch. Pressing Enter in the URL field runs a probe, not the
download (L27) — violating "type URL + Enter = go". Fix: label the action, make Enter download
(or fetch-then-download), and present Fetch as optional ("Preview formats").
Fixed in DownloadBar.tsx: Enter is now "go" — it
starts the download (or adds the fetched playlist when the playlist panel is open) instead of probing;
probing stays an explicit, optional click. The magnifier button is relabeled from "Check URL" to
"Preview available formats (optional)" (both its tooltip and
aria-label), and the URL placeholder now reads "Paste a video or playlist URL, then press Enter to download" so it's clear Download works without Fetch.download()already enqueues with the presets when nothing has been probed, so the Enter-downloads path needs no probe. (resolves L27's Enter inconsistency too.) - UX3 — Library "Index" is jargon and a hidden two-stage workflow. Pasting a channel URL in
the Downloads bar tries to treat it as one item; whole channels belong in the Library tab, but
nothing signposts that. "Index" reads like "Download" but only catalogs — you must then expand,
select, and Download. Fix: rename to "Add channel/playlist," and after indexing surface a clear
"Download N videos" next step; detect channel URLs in the Downloads bar and suggest the Library.
Fixed across three seams. (1) De-jargoned the Library (LibraryView.tsx):
the primary button is now "Add" (was "Index"), "Re-index" → "Refresh", and the header
description / empty state / placeholder / error copy drop "index" for "add". The post-add next step
already exists —
indexSourceauto-selects the new source so its card expands with a prominent "Download N pending" primary button. (2) Channel detection in the Downloads bar — a new purelooksLikeChannelOrPlaylist(conservative: only clear YouTube channel/dedicated-playlist shapes; a/watchvideo, even with alist=, is never flagged and keeps working in the bar) drives a dismissible brand-tinted nudge in DownloadBar.tsx: "This looks like a channel or playlist. Add it in the Library…" with an Open in Library action. (3) The handoff goes through a small new store/nav.ts (App's tab state moved into it) that carries a one-shotpendingLibraryUrl; the Library consumes it on mount to pre-fill its add field. Unit-tested (looksLikeChannelOrPlaylistintest/clipboardLink.test.ts) and verified end-to-end in the browser preview (channel URL → nudge → Open in Library → Library tab with the URL pre-filled). - UX4 — Destructive actions have no confirmation and no undo (L13). "Clear history," "Clear log," "Remove source" (deletes an entire indexed channel + items), and "Delete selected" are all one click. A user exploring can wipe data irreversibly. Fix: confirm destructive/bulk deletes (or offer an undo toast).
- UX5 — First run can't choose the download folder. Onboarding.tsx
only describes Documents\Video/Audio — there's no picker (the ROADMAP claims one). The user must
later discover Settings → Downloads. Fix: put the folder picker in onboarding as documented.
Fixed: the "Where downloads go" block in Onboarding.tsx
now has interactive Video and Audio rows — each shows the current folder (or the
Documents\Video/Documents\Audiodefault) with a Choose… button that opens the OS picker via the settings store'schooseDir(the same proven path DownloadsCard uses, seeded at the current value — W5). Picking a folder persists immediately, so the choice is already saved when the user clicks "Get started"; leaving it untouched keeps the Documents defaults. Verified in the browser preview with onboarding forced on.
Medium — confusion, feedback & organization
- UX6 — Silent failures on file actions.
openFile/showInFolderin the stores callwindow.api.openPath(...)and ignore the returned error string; reveal.ts refuses missing/moved files or disallowed types and returns an error nobody surfaces. Clicking "Open file" on a moved download does nothing, with no message. Fix: surface open/reveal errors. Fixed: a shared renderer helper reveal.tsrevealFile('open'|'folder', path)awaits the IPC result and raises the main-process reason ("File not found — it may have been moved…", "Refusing to open this file type.") as an error toast — the new transient-notification surface (see UX9). Both the downloads and history stores route through it.safeShowInFolderwas upgraded to return an error string likesafeOpenPath(it previously no-op'd silently), so reveal failures surface too. - UX7 — Settings is one long unsegmented scroll of ~11 cards with no section nav/anchors; the
only finder is the hide-cards search (M14). Order is arbitrary and related items are split.
Fix: group into sub-sections (or a left rail) with a stable, logical order.
Fixed (Batch 19): the 11 cards are grouped into five stable sections — Downloads (defaults +
post-processing + filenames), Appearance, Network & accounts, Advanced, App & maintenance — each with a
section heading, plus a jump nav under the search box that smooth-scrolls to a section (anchors work inside
the screen's own scroll shell from L161;
scrollMarginToplands the heading cleanly). The search filter still works per-card and now hides a whole section when all its cards are filtered out; the jump nav hides while searching. Heading hierarchy corrected (page h1 → section h2 → card h3). Verified in the preview probe: nav renders, jumping lands on "App & maintenance", searching "proxy" leaves exactly the matching cards + their section headings visible (screenshots). - UX8 — "Default format" vs "Format & post-processing" are two places for one concept. Kind + quality live under Downloads; container/codec/subs/SponsorBlock live in a separate card. Fix: co-locate, or clearly label one "defaults" and the other "advanced post-processing." Fixed (Batch 19, both halves of the fix): the cards are now co-located — the post-processing card sits directly under the Downloads card in the new Downloads section (UX7) — and clearly labeled: the card is renamed "Post-processing" with a hint stating it's the advanced conversion half and that the default format itself is set in the Downloads card above.
- UX9 — No global completion feedback off the Downloads tab (UI25). If OS notifications are
off/suppressed (focus assist) and you're on another tab, a finished download gives no in-app sign.
Fix: a sidebar Downloads badge / lightweight in-app toast.
Fixed (both halves of the suggested fix): a sidebar Downloads badge — a Fluent
CounterBadgeon the Downloads nav item shows the active (downloading + queued) count from any tab (Sidebar.tsx), driven by a count-only selector in App so it updates without re-rendering on every progress tick; and a lightweight in-app toast surface (Toaster.tsx + store/toasts.ts), a non-portal bottom-center stack that any store/component can raise (now used for UX6's file-action errors). Together this closes UI25. - UX10 — Re-download silently changes quality (H5) — a confusing "I asked for 720p, got Best." Resolved by H5 (re-download stores
formatId/formatHasAudioand strips compound quality labels). - UX11 — Scheduled downloads silently never fire if the app is quit (M4). The hint is easy to
miss; the user returns to find nothing happened. Fix: warn at schedule time that it requires the
app running (or persist + relaunch). Resolved by M4 (the "persist + relaunch" option the fix names): the
queue is now persisted, so a
saved+scheduledForitem survives a quit and the App promoter tick fires it on the next launch once its time has passed (the store normalizes/promotes due items). So a scheduled download no longer "never fires" if quit — it fires when due, or on the next launch if that time already passed. (Live-smoke of the quit→relaunch cycle noted in M4.) - UX12 — Terminal is a dead-end when custom commands are off (L18). The nav item is always
visible; clicking it shows a gate pointing to a setting on another screen. Fix: hide/disable the
nav item, or let the gate enable the setting inline. Fixed (both fixes): the nav item is already hidden
when custom commands are off (Sidebar's
showTerminalfilter, L18), and the gate — still reachable via the command palette — now carries an inline "Enable custom commands" button that flips the setting on the spot (TerminalView.tsx), so it's no longer a dead-end that sends you to another screen. - UX13 — Backup export warns of nothing; it contains secrets (M22). A user "backs up settings" and ships proxy creds / tokens in cleartext. Fix: warn, or mask/omit secrets. Resolved by M22 (backup strips proxy / PO-token / update-token; the caption states credentials aren't included).
- UX14 — Duplicate warning can show a placeholder title (L76): 'Already in your queue: "YouTube video (abc123)"' before metadata resolves looks broken. Fix: fall back to the URL. Resolved by L76 (the dup warning falls back to the URL when the title is still the placeholder).
- UX15 — No bulk actions in the Downloads queue. History has a select-mode + bulk delete;
Downloads has only per-row remove + "Clear finished." Inconsistent. Fix: mirror select/bulk, or
add "Cancel all." Fixed (the "Cancel all" option): a
cancelAllstore action + a "Cancel all (N)" header button (DownloadsView.tsx) that cancels every active (downloading + queued) item at once — killing the live processes and freeing the queue — shown only when there's something cancelable. (A full History-style select-mode is the heavier alternative; "Cancel all" covers the common bulk need.)
Low — polish & smaller friction
- UX16 — URL field isn't focused on launch.
aerofetch-urlis only focused via the command palette; opening the app or the Downloads tab requires a click to start typing. Fix: autofocus it. Fixed: anAppmount effect focuses#aerofetch-urlon launch when the Downloads tab is active (double-rAF for paint, reusing the command palette's focus pattern). - UX17 — Icon-only Fetch/Paste buttons rely on hover tooltips for meaning (UI a11y; new users on touch/keyboard miss them). Fix: labels or
aria-described affordances. Resolved (a11y met; verified): both buttons already carry anaria-label(Fetch = "Preview available formats (optional)", Paste = "Paste from clipboard") and aHinttooltip that shows on hover and keyboard focus — so screen-reader and keyboard users get the name; the icon isn't the only affordance. A permanently-visible text label is a deliberate compact-layout tradeoff for the URL row (the Download button beside it carries the primary text label), so it's left as a visual choice, not an a11y gap. - UX18 — The quality control morphs after Fetch (preset list → real formats; label "Quality" → "Quality / format") in place — a surprising transform. Fix: keep a stable control with a clear "formats loaded" state. Fixed (Batch 18): the label stays "Quality" in both states; a small brand-colored "• fetched" tag appears beside it once real formats load, so the state change is signalled without the control or its label morphing. Verified in the preview probe: Fetch on a URL loads the mock formats and the tag + format list render as designed (screenshot).
- UX19 — Window size/position isn't remembered. index.ts opens a fixed 920×700 every launch (no bounds persistence). Fix: persist + restore window bounds. Fixed with W2 — windowState.ts persists + restores bounds/maximized with an off-screen guard.
- UX20 — "App is still running" surprise. Closing with a download active hides to tray and
notifies once per process (L68); later closes give no hint, so the window "won't close" reads as a
bug. Fix: a persistent tray hint / first-close explainer. Resolved by L68 (the "first-close explainer"
it now is on every close): the once-per-process guard was the whole bug —
notifiedBackgroundnow resets when the window is shown again, so each close-to-tray with a download active (or tray mode) re-fires the "AeroFetch is still running in the tray" notification, not just the first. So a later close no longer looks like the window is stuck. (The tray behavior itself is worth a watched confirm in the Batch 15 live pass.) - UX21 — No keyboard-shortcut discoverability. Ctrl+K (palette), Enter, Ctrl+Enter are undocumented; there's no "?"/shortcuts screen. Fix: a shortcuts hint or help affordance. Fixed (discoverability): the first-run Onboarding now includes a tip — "Press Ctrl+K anytime to jump to any screen or start a new download." — so new users learn the palette, which is itself the discovery surface for the rest; and the palette's Settings action shows its shortcut ("Ctrl ,") in the hint column. (A dedicated full shortcuts/"?" screen is the heavier alternative, left for later.)
- UX22 — Onboarding is shallow and one-shot (L67): three tips, no folder choice, not
revisitable; the clipboard tip requires a focus event a first-timer won't trigger. Fixed the "one-shot"
core: a "Show welcome tips again" button in Settings → About re-opens the welcome/onboarding screen
on demand (sets
hasCompletedOnboarding: false), so it's revisitable (closes L67's revisit half). Onboarding does offer folder choice already (Video/Audio "Choose…"), and it gained the Ctrl+K tip (UX21). The remaining "shallow" (only a few tips) is a content judgment left as-is. - UX23 — Settings folder paths are read-only (L71) — no paste; Browse-dialog only.
Resolved (Batch 19) — stale duplicate of L71, which was already fixed: the Video/Audio folder inputs are
editable (typed or pasted paths commit through the settings store), main validates via
isSafeOutputDir(unit-tested in validation.test.ts), and the M34 reconcile means a rejected path never sticks in the UI. Verified editable in the Batch 19 preview probe. - UX24 — "Check N watched for new" is disabled with no explanation when no source is watched —
a user with indexed (but unwatched) sources sees a dead button. Fix: tooltip / enable with a hint to watch a source.
Fixed: the button uses
disabledFocusable(notdisabled) when nothing is watched, so it stays hover/focus-reachable, and aHintexplains why — "Turn on 'Watch' for a channel or playlist below first, then this checks them for new uploads." (LibraryView.tsx). - UX25 — Empty-state copy mismatches the flow (L78): Downloads says "Paste a URL above to get started," but the actual path is Fetch/Download. Resolved by L78 (DownloadsView empty-state copy rewritten to match the Fetch/Download flow).
- UX26 — yt-dlp/ffmpeg version + initial settings load have no skeleton/spinner — brief blank
states on boot (plus the documented one-frame theme flash). Fix: lightweight loading states.
Fixed (Batch 18, with L109): the About versions show skeleton lines during the boot load, and SettingsView
renders three skeleton cards until the settings store's
loadedflag flips — no more flash of fallback values. (The one-frame theme flash remains separately documented under SR1.)
Windows platform conventions
Review against Windows 11 / Fluent desktop conventions. W IDs are new; existing IDs are referenced
where a dimension is already filed. The app gets the big things right (native dialogs, per-user
no-admin install, AppUserModelID, tray, taskbar progress, jump list, nativeTheme dark-mode follow,
DPI handled by Chromium). The deviations:
Window behavior & resizing
- W1 — No minimum window size.
createWindowsetswidth/heightbut nominWidth/minHeight(index.ts), so the window can be dragged down to a few pixels and the layout (212px sidebar + content) breaks. Standard: set a sensible min (e.g. 640×480). - W2 — Window placement isn't persisted (extends UX19): size, position, maximized state, and
which monitor are all forgotten — it reopens 920×700 on the primary display every launch.
Standard: persist & restore window placement per Windows app convention (e.g.
electron-window-state). Fixed (no new dep): windowState.ts persists the window's normal bounds + maximized flag to<userData>/window-state.json;createWindowopens from the saved state (debounced save on resize/move + a save on close, which may only hide to tray). Ascreen.getAllDisplays()visibility check falls back to the default centered size when the saved monitor is gone, so the window can never restore off-screen. (The restore cycle itself wants a live quit→relaunch confirm.) - W3 — Title bar doesn't follow the in-app theme.
nativeTheme.themeSourceis deliberately never set (index.ts:97), so the OS-drawn caption follows the OS theme. Choosing an explicit in-app dark theme on a light OS leaves a light title bar on a dark app (and vice-versa). Standard: setnativeTheme.themeSourceto the resolved mode, or usetitleBarOverlaywith themed colors.
Dialogs & file pickers
- W4 — Text fields have no Cut/Copy/Paste context menu. Electron adds no default editing menu and
the app registers no
context-menuhandler, so right-clicking any input/textarea (URL, search, terminal args, template fields) shows nothing — a basic Windows text-editing affordance is missing. Standard: wire a standard editing context menu (and spellcheck suggestions for textareas). - W5 — File pickers don't open at the current value.
chooseFoldersets nodefaultPath, so the folder dialog opens at a default location instead of the currently-configured Video/Audio folder. Standard: seed the picker with the current path. (Native dialogs themselves are correct — good.) - W6 — Cookie sign-in is a separate taskbar window.
openCookieLoginWindowcreates aBrowserWindowwith noparent/modal(cookies.ts), so it appears as a second AeroFetch taskbar button rather than a child/modal dialog. Standard:parent: mainWindow(+modalif appropriate) so it groups under the app.
Keyboard, context menus & focus
- W7 — Lists have no keyboard navigation. The queue, history, and library lists can't be arrowed through, Delete doesn't remove the focused/selected item, and there's no Ctrl+A select-all — all standard Windows list behaviors. Standard: roving focus + Delete/Ctrl+A on list surfaces. (a11y; see also UI30)
- W8 — No standard accelerators. No Ctrl+, (Settings), F1 (help), or F5 (refresh); Ctrl+K (palette) is the only global shortcut and is undiscoverable (UX21). Standard: add the conventional accelerators + a discoverable shortcut list.
- W9 — Focus isn't restored or advanced. Closing the command palette doesn't return focus to the trigger; finishing onboarding and switching tabs don't move focus into the new content. Standard: restore focus on overlay close; move focus to the activated panel. (extends UI28–UI29)
- (ref) Context menus absent app-wide — UI24; the text-field case is W4. Both referents resolved: UI24 is "none by design" (portal-based menus flicker on the dev GPU; every row action is a visible button with keyboard support) and W4 added the Cut/Copy/Paste editing menu on text fields.
System tray, taskbar & icons
- W10 — No taskbar attention flash. When a background download completes while the window is
minimized/hidden, the app doesn't
flashFramethe taskbar button. Standard: flash for attention on background completion (pairs with the existing notification). Fixed:notify()in download.ts callswin.flashFrame(true)when the completion/failure event lands while the window is unfocused/minimized/hidden (Windows stops the flash on focus). Gated on the samenotifyOnCompleteas the toast, so it pairs with it. (The flash itself wants a quick live confirm.) - W11 — No taskbar overlay badge (deferred in roadmap): a download manager conventionally shows an
active-count / error overlay icon via
setOverlayIcon. Standard: add it (the progress bar's error mode is not a substitute). Done: badge.ts draws a green (active) / red (error) dotNativeImagewith pure Node/zlib (no dep), and thetaskbarProgresshandler in ipc.ts callswin.setOverlayIcon(badge, label)with an accessible count label ("N downloads active" / "Download error"), cleared tonullwhen the queue goes idle. (Numeric count lives in the tooltip label — a 16px overlay can't legibly render digits.) - 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
.icois absent. Standard: a multi-size fallback (or rely only on the.ico). Fixed (Batch 20): the fallback is now a multi-representationNativeImage— 16/24/32/48 px renders of build/icon.svg (ImageMagick, like W14) embedded in trayFallbackIcons.ts and added viaaddRepresentationat scale factors 1/1.5/2/3, so Windows picks the crisp size for the DPI. Verified in an Electron probe: the image decodes (non-empty) and reports all four scale factors. As before, it only renders if the bundled icon.ico is missing. - W13 — Notifications set no icon.
new Notification({title, body})omittedicon; on the portable build (no installed AUMID shortcut) Windows toasts showed a generic icon. Fixed: both notification sites (download.ts completion/failure toast + index.ts background-running toast) now passicon: getAppIconImage()— a new cachedNativeImagehelper in binaries.ts that loads the app.icoonce and falls back to an empty image (OS default) if it's missing. (refines L127) - W14 — App/notification icon is a placeholder (M3-orig) — Fixed: redesigned
icon.svg into a proper mark (teal brand square with a top-lit gradient + soft sheen and a
bold rounded download glyph over a landing shelf), legible down to 16px; regenerated the multi-size
build/icon.ico(256/128/64/48/32/16) via ImageMagick. The notification-icon wiring (passing the asset tonew Notification) is also done — see W13/L127.
High DPI, multiple monitors, touch
- 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).
Fixed (Batch 20) as a user setting rather than a blind re-enable (the dev-machine GPU bug still can't be
validated here): new
Settings.hardwareAcceleration(default false = today's safe software rendering), surfaced as Settings → Appearance → "Hardware acceleration" with a restart note. index.ts reads it before ready (after the portable userData redirect) and only then callsdisableHardwareAcceleration(); the window-occlusion feature stays disabled in both modes (a separate documented flicker cause). Users on modern GPUs can now opt in without a build change. Verified the module-top boot path runs cleanly (exit 0, empty stderr) with the new pre-ready settings read. - 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: the icon-only row-action clusters (QueueItem + HistoryViewactionscontainers) now enforce a ≥40×40px hit target via a& buttonmin-size rule — the glyph is unchanged, the subtle button just carries more padding. The label half was already covered: the shared Hint shows on:focus-within, not only:hover, so keyboard/AT users get the name (and every action also has anaria-label). (LibraryView's per-item rows have no icon-only actions — checkbox + status chip only; its header actions are labeled text buttons — so nothing there needed enlarging.)
Dark mode & accessibility (Windows-specific)
- W17 — No
aria-livefor status changes. Narrator doesn't announce download progress, completion, or errors — there are no live regions. Standard: polite live regions for queue/status updates. - W18 — Custom colors unverified under High Contrast. The status pills, segmented controls, accent
swatches, and thumbnail tints use explicit background colors that Windows forced-colors mode may not
adapt (nothing sets
forced-color-adjust). Standard: test under each Contrast theme; let system colors win. Fixed the two surfaces that convey meaning through color and would otherwise break underforced-colors: the accent swatches (settingsStyles.ts) now setforced-color-adjust: noneso the accents stay distinguishable (they're color previews — otherwise all flatten to one system color), and the SegmentedControl active segment paints with the systemHighlight/HighlightTextpair under@media (forced-colors: active)so the checked state stays visible once the brand background is flattened. The status chips are FluentBadges (Fluent supplies its own forced-colors handling) and the thumbnail tints are purely decorative placeholders, so both correctly let the system palette win. Live verification under each Contrast theme is still worthwhile but the color-meaning surfaces are now defended in code. - (ref) No semantic headings / radiogroup arrow-nav / focus rings — UI33, UI30, UI28–29 (all bear on Narrator + keyboard users). All four resolved (see UI33/UI30/UI28/UI29).
Settings & standard conventions (mostly OK)
- W19 — Window/taskbar don't reflect state. No window title at all (L108), so the taskbar button never shows "3 downloading" or similar. Standard: reflect activity in the title/tooltip.
- (OK) Per-user no-admin install, NSIS uninstaller,
aerofetch://registration, AppUserModelID, tray tooltip + left-click restore + right-click menu, taskbar progress, and jump list all follow Windows conventions. (Non-goal) unsigned binary → SmartScreen prompt — accepted; no certificate will be purchased (the build stays signing-ready via env vars if that ever changes — see SIGNING.md). (Acknowledged — verified-OK / accepted non-goal, no action.)
Code consistency & conventions
A cross-cutting consistency pass over the dimensions requested. Many specifics are already filed; this
section consolidates them by theme and — the point of the exercise — recommends one standard each.
CC IDs are the consolidated items; existing IDs are referenced, not re-filed. The codebase is actually
stylistically consistent (no-semicolon, single-quote, 2-space) and has a genuinely good pure/impure
split — but that style is unenforced and several "do the same thing two ways" seams have crept in.
- CC1 — Naming conventions. Boolean settings have no convention:
useAria2c(verb-prefix),autoUpdateYtdlp(auto-prefix),customCommandEnabled(suffix),downloadArchive/restrictFilenames(bare). Keys sayvideoDir/audioDirwhile the UI says "folder." Preload names diverge from main (L92);MediaKindis declared twice (L105). Standard: booleans asis/has/should/<verb>consistently; one term ("folder") across keys + UI; align preload↔main names; single-source shared types in@shared. Fixed (Batch 23) — the breaking rename shipped WITH its migration:customCommandEnabled→enableCustomCommands,downloadArchive→useDownloadArchive,clipboardWatch→watchClipboard,sidebarCollapsed→isSidebarCollapsed,hardwareAcceleration→useHardwareAcceleration, andvideoDir/audioDir→videoFolder/audioFolder(plus the store actionschooseDir/clearDir→chooseFolder/clearFolder); already-conforming verbs (useAria2c,notifyOnComplete,minimizeToTray,launchAtStartup,restrictFilenames,autoUpdateYtdlp,autoDownloadNew,hasCompletedOnboarding) kept. The rename map lives in settingsMigration.ts (pure, unit-tested in test/settingsMigration.test.ts incl. a full pre-rename backup fixture) and is applied in two places: a one-time idempotent on-disk shim when the electron-store opens, and on backup import so old backup files still restore. Preload↔main names (L92) and single-sourced types (L105) were already done. - CC2 — Coding style is consistent but unenforced. No ESLint/Prettier config, script, or dep
(L44), so the (good) house style drifts only by discipline;
??vs||is occasionally misused for null checks. Standard: add Prettier + typescript-eslint withlint/formatscripts in CI; codify the existing style. - CC3 — Different patterns for the same problem. JSON persistence (M1), status chips (UI18),
segmented controls (UI14), buttons (UI15), overlays/dialogs (UI23), three yt-dlp probe spawners
(
probeMedia/probeMeta/probeFlat), and duplicated stdout line-buffering indownload.tsandterminal.ts. Standard: one helper per concern (ajsonStore, aStatusChip, aSegmentedControl, one spawn-and-stream helper). Fixed: one helper now backs each concern —createJsonStore(M4/R1),StatusChip(UI18),SegmentedControl(UI14),execFileAsync(lib/exec.ts) for the three probe spawners, and — this batch — the last duplication, the stdout newline-split loop, is now lib/lineBuffer.ts (createLineBuffer), adopted by bothdownload.tsandterminal.tsand unit-tested (test/lineBuffer.test.ts). (UI15 button-wrapping and UI23 overlay unification stay their own incremental tracks.) - CC4 — Duplicate utilities. Formatting (M9),
youtubeId(H2),newId(M7), JSON I/O (M1),MediaKind(L105) — plusexecFileis hand-wrapped in anew Promisein five places (probe/ytdlp/ffmpeg/download.probeMeta/indexer.probeFlat) instead of onepromisifyd helper, and the stdout newline-split loop is copied in two. Standard:src/main/lib/+src/renderer/src/lib/with one each of:format,youtube,jsonStore,ytdlpExec(promisified spawn + line stream). Fixed: every listed duplicate now has one home — rendererlib/formatters/urlHelpers/id+@shared/format,MediaKindsingle-sourced (L105); mainlib/exec.ts(the fiveexecFilewrappers) and nowlib/lineBuffer.ts(the two stdout-split loops). Thelib/dirs exist on both sides. - CC5 — Async patterns are mixed. Three I/O idioms: callback-wrapped
new Promise(execFile ×5),async/await+fetch(updater check, sync), and event-emitter-wrapped Promise (net.requestin updater). Renderer mixes.then().catch()(stores) withasync/await+try/catch (components). Standard:async/awaiteverywhere; a sharedexecFileAsync; wrap event-emitter APIs once in a helper. Closed with CL4 (Batch 21): theexecFile ×5idiom was already gone (oneexecFileAsync, CC4), and the last event-emitter-wrapped Promise — the updater'snet.requeststreaming download — is now wrapped once instreamInstallerToFile, extracted under 14 behavior-pinning tests.async/await+ single wrappers hold across the app code. - CC6 — Five failure-signaling conventions.
{ ok, error }result objects (download/updater/ ytdlp/backup/IPC),throw(assertHttpUrl),null(indexerCore),[](history/sources read errors), and a bare error string (reveal.safeOpenPath).cleanErroris applied to some spawn stderr but not ytdlp/ffmpeg. Standard: aResult<T>({ ok, value?/error? }) across every service/IPC boundary;throwonly inside pure helpers, caught at the boundary; always run yt-dlp stderr throughcleanError. Fixed (the concrete divergence): yt-dlp stderr now runs throughcleanErroreverywhere —getYtdlpVersionandupdateYtdlp(ytdlp.ts) adopted the exactcleanError(r.stderr) || r.error?.message || ''idiom already used by download/probe/indexer, so a failed version/update check shows the trailing error line, not raw noise. (ffmpeg surfaces no stderr — its version probe returnsnullby design.) The broader "oneResult<T>across every boundary" holds already for the service/IPC layer (they return{ ok, error }); collapsing the remainingnull/[]/bare-string internal signals into it is a low-value sweep left to the incremental track — the specificcleanErrorinconsistency this item names is closed. - CC7 — Dependency-injection styles. Pure modules take deps as params (good —
binDir,now); the impure shell uses module singletons (getSettings(),getYtdlpPath(), lazygetStore()); progress is callback-injected; theWebContentssender is a param in some handlers and a closure in others. Standard: keep pure-core param injection; in the shell, passwc/onProgressconsistently as the leading argument and keep singleton access for config/binaries. Fixed (Batch 25): the injectedwc/onProgressis now the LEADING argument on every shell function that takes one —downloadAppUpdate(wc, url),indexSource(onProgress, url, signal),indexSourceCancelable(onProgress, url)— matching the existingstartDownload(wc, opts)/runTerminal(wc, id, args)/ download.tssend(wc, ev)/notify(wc, …). Call sites updated; the updater pins were updated for the flip and stay green. Singleton access for config/binaries is kept as the recommended split. - CC8 — No logging strategy. Effectively no diagnostics — a single stray
console.errorin preload, and ~29 swallowed catches (M29);errorlog.tsis domain data, not logging. Standard: one small leveled logger (e.g.electron-log) written to userData, called at every catch; keeperrorlog.tsfor user-facing download failures only. Fixed: a small in-house leveled logger logger.ts (no new dependency — theelectron-logsuggestion was an example) appends to<userData>/logs/aerofetch.logwith size-capped rotation; allapp/fs access is lazy+guarded so it's a safe no-op under unit tests. The main-process catch sites route through it, and a fire-and-forgetlog:writeIPC + preloadlogError()forward the renderer'slogError(M29) failures to the same file sink — closing the "sink" half M29/R6 deferred here.errorlog.tsstays as user-facing download-failure data. The user-facing toast half ties to the global status surface (UI25/UX9) and lands there. - CC9 — Four validation styles. Type-guard predicates (
isValid*in validation.ts), coerce-with-fallback (sanitizeOptions,templates.sanitize), an imperative per-keyswitch(setSettings, M24), andthrow(assertHttpUrl). Standard: one schema layer (e.g.zod) that both validates and coerces;setSettingsruns values through the same schema instead of a bespoke switch. Fixed (Batch 24):zodadopted; every Settings key is a schema in settingsSchema.ts whose parsed output is exactly what gets persisted — numbers coerce+clamp, strings trim, enums allowlist, folders/templates run the shared validation.ts refinements,downloadOptionssanitizes (via the extracted settingsOptions.ts).setSettings's 130-line switch is now a 15-line loop: parse → skip-on-fail → store, with the two genuine side effects (secret encryption at rest, the launch-at-startup registry sync) kept beside their module state. Backup import flows through the same schema (it calls setSettings). Parity with the old switch is pinned field-by-field in test/settingsSchema.test.ts; the whole suite passes unchanged. The jsonStore row guards deliberately stay type-guard predicates (hot read path filtering untrusted disk rows — no coercion wanted), documented in the schema header. - CC10 — Mixed serialization/persistence.
electron-store(settings) and hand-rolled pretty-JSON files (history/errorlog/templates/sources/media-items, M1) for the same job, plus yt-dlp-mandated formats (Netscape cookies, plaintext archive) and base64enc:v1:secrets. Standard: onejsonStore<T>()abstraction for the app's own records; pick either electron-store or the JSON stores for everything, not both; leave the yt-dlp-format files alone. Resolved as deliberate-by-design (Batch 25): thejsonStore<T>()abstraction (createJsonStore, M4/R1) already backs every app record (history/errorlog/templates/sources/media-items/queue), so the "hand-rolled per file" divergence is gone. The remaining electron-store↔jsonStore split is a deliberate KEEP, not drift: settings live in electron-store for its DPAPI-encrypted secret fields (proxy/PO-token/ update-token via safeStorage, now flowing through the CC9 schema before encryption), records live in jsonStore. Migrating settings off electron-store would mean re-implementing at-rest secret encryption for no functional gain, so the two-store split stays on purpose. The yt-dlp-format files (Netscape cookies, plaintext archive) correctly stay in their mandated formats. - CC11 — Configuration is scattered.
electron-store+localStorage(sidebar, M19) + env vars (PORTABLE_EXECUTABLE_DIR,CSC_LINK,AEROFETCH_REAL_DOWNLOAD) + hardcoded module consts (update host/owner/repo, timeouts, caps,09:00,ARIA2C_ARGS, L10). Standard: aconfig.tsfor build/host constants; foldlocalStorageUI prefs into the settings store so there's one persisted-prefs source. Fixed (Batch 23, the remaining move): build/host constants (update host/owner/repo + the release API URL) now live in config.ts, imported by the updater — deploy identity in one file, runtime tunables in constants.ts (L10), user prefs in settings (localStorage was folded in by M19). The env vars are genuinely environmental (portable mode, CI signing, integration-test opt-in) and stay env vars. - CC12 — Project organization. Renderer
components/mixes screens (views) with reusable widgets; helpers (theme/thumb/useClipboardLink) sit at src root; the purequeueStatslives instore/; main mixes pure (buildArgs/validation/indexerCore/ytdlpPolicy) and impure modules in one flat dir. Standard: rendererviews/+components/+lib/; maincore/(pure) + services; movequeueStatstolib. Fixed (Batch 25): the reorg landed. Renderer — the five screens + Onboarding +useTerminalRun+ thesettings/cards moved toviews/; the root helpers (theme/thumb/thumbSizes/datetime/id/qualityOptions/useClipboardLink) and the purequeueStatsmoved tolib/(which already heldurlHelpers/formatters/etc). Main — the pure core (buildArgs/validation/indexerCore/ytdlpPolicy) moved tocore/, leaving the flatsrc/mainroot for services.git mvpreserved history; every import (src + test) updated. Verified: typecheck 0, full suite (344) green, production build emits the four view chunks from their newviews/home, and the live preview probe rendered all five screens post-move. - CC13 — View/state boundary (the project's "MVVM"). It's React+Zustand, but the container/
presentational split is inconsistent: orchestration lives in stores for downloads/sources yet inside the
component for DownloadBar, and SettingsView calls
window.apidirectly (L93, UX1). Standard: stores/ hooks are the view-models (own orchestration + all IPC); components stay presentational; nowindow.apiin components. Closed with L93 (Batch 16): the settings cards, TerminalView, and LibraryView now reach IPC only through colocated view-model hooks or store actions;grep window.api src/renderer/src/componentsmatches only hooks (use*.ts). The standard holds codebase-wide: stores/hooks own orchestration + IPC, components render. - CC14 — State ownership is unprincipled. State lives in Zustand stores, component
useState,localStorage, the mainelectron-store(mirrored into a renderer store), and module-level vars (idCounter,notifiedBackground,active). Persisted state is optimistic-only and never reconciled (M34/L142); two stores form a cycle (C2). Standard: main owns persisted state; renderer stores mirror it and apply the authoritative value the IPC call returns; UI-only state in stores; break store cycles via a coordinator. Closed with L142 (Batch 16): every persisted-state mutation now applies main's authoritative return (history/templates/sources via the keyed reconciler; settings via M34). Main owns persisted state, the renderer mirrors + reconciles, the store cycle is gone (C2),localStorageis folded in (M19). The module-level vars called out (idCounter/notifiedBackground/active) are legitimately module-scoped singletons, not misplaced UI state.
Recommended single style (apply project-wide)
- Errors: one
Result<T> = { ok: true; value } | { ok: false; error }across every service/IPC boundary;throwonly in pure helpers; onelogger(leveled, file-backed) invoked at every catch. - Async:
async/awaitonly; oneexecFileAsync+ one spawn/stream helper; wrap event-emitter APIs once. - Validation: one schema lib (zod) that validates and coerces; reuse it in
setSettings, backup import, and JSON-store reads. - Persistence/serialization: one
jsonStore<T>()(or electron-store) for app records — not both; one config module for host/build constants; one persisted-prefs store (nolocalStorage). - Shared code:
lib/for pure utils (format, youtube, ids, jsonStore);@sharedis the only home for cross-process types (no re-declaredMediaKind). - UI: design tokens (spacing/radius/type/icon scales) + shared primitives (
Screen,StatusChip,SegmentedControl,LinkSuggestion, button/focus); stores are view-models, components presentational. - Naming: booleans
is/has/should/<verb>; "folder" not "dir"; preload methods match main names;verbNounfor store actions,onXfor component handlers. - Tooling: Prettier + typescript-eslint with
lint/format/CI gates so all of the above stays enforced rather than aspirational.
Code cleanliness
A dead-code / clutter review. First, the good news — the codebase is genuinely clean on most axes:
no TODO/FIXME/HACK/XXX in source (only in roadmap docs), no commented-out code, no
debugger/debug logging (the lone @ts-ignore ×2 in preload is the legitimate browser-fallback
window assignment), every source file is imported, and the dead IPC surface is exactly the two already
filed. Several listed categories are N/A to this stack and worth recording so they're not re-asked:
- Unused XAML / WPF — N/A (Electron + React/HTML, no XAML).
- Unused strings / localization — N/A (no i18n/string-resource files; strings are hard-coded, i18n deferred), so nothing to be "unused."
- Unused images / icons — none: the only assets are
build/icon.{ico,svg}(used) and the inline base64 fallback tray PNG (used); thumbnails are remote. (Clutter, not shipped:dist/~3 GB of old installers — L84;.frames/19 gitignored captures.) - Unused files / resources — none in
srcorresources/bin(yt-dlp/ffmpeg/ffprobe all used; aria2c optional).
Dead / unreachable code (the real list):
- Safe to remove now —
getDefaultFolder(M2): channel + preload method + main handler + mock, zero callers. Pure deletion, no behavior change. - Decide "wire or remove" — command preview (M5): channel + preload
previewCommand+ mainpreviewCommand()+formatCommandLine/quoteForDisplay(buildArgs, unit-tested) +CommandPreviewResult+ mock. Decided: wired (M5/UX1) — the DownloadBar Advanced panel has a Show/Hide command toggle callingwindow.api.previewCommand; the whole chain is now reachable. - Decide "wire or remove" — incognito/private (M6):
DownloadItem.incognito+AddOptions.incognitobuildItemhandling + two history-skip checks + the QueueItem "Private" badge. Decided: wired (M6/UX1) — the DownloadBar Advanced panel has an "Incognito mode" checkbox that sets the flag (reset after each download).
- Decide "wire or remove" — per-download options/extraArgs (UX1):
DownloadItem.options/extraArgs,StartDownloadOptions.options/extraArgs,AddOptions.options/extraArgsare plumbed end-to-end but never set by any UI. Decided: wired (UX1) — the collapsible Advanced panel reusesDownloadOptionsFormfor per-download overrides passed viaAddOptions.options; overrides reset to the global defaults after each download. - Comment, not code —
MAX_ENQUEUE_BATCH(M33): referenced by ipc.ts:629 + the roadmap but never implemented; fix the comment (nothing to delete). Resolved by M33: the falseMAX_ENQUEUE_BATCHreference is gone; ipc.ts:730 now honestly states all selected entries are enqueued at once (with a note to add a cap if large — the actual cap is deferred to PERF3).
New cleanliness findings:
- CL1 — Magic-string protocol markers duplicated across the emit/parse boundary.
'prog|'and'path|'are hard-coded inbuildArgs.ts(PROGRESS_TEMPLATE/--print after_move:path|…) and again indownload.ts(line.startsWith('prog|')/'path|'). Change one and the other breaks silently. Fix: export shared marker constants from one module. - CL2 — Long positional parameter list.
buildArgs(opts, outputTemplate, o, binDir, access, extraArgs)takes six positional args (andovsoptsis easy to swap). Fix: pass a single options object. Fixed:buildArgs(input: BuildArgsInput)now takes one named object (opts/outputTemplate/options/binDir/access/extraArgs?) destructured at the top, so callers can't transpose the two path-like strings oropts/options. download.ts'sbuildCommandand both test harnesses updated. - CL3 — Large method:
startDownload(~130 lines) bundles spawn + dual metadata path + four inlinechildevent handlers. Fix: extract the stdout-parse + close/error wiring (pairs with CC3's spawn-stream helper). Fixed: the stdout/stderr parse, the close/error handlers, and the B1 idle watchdog moved into awireChildProcess({ wc, opts, rec, cleanup, getTitle })helper in download.ts.startDownloadis now a linear pre-flight (binary checks → URL normalise → concurrency guard → cookies → spawn → metadata probe → wire).getTitleis a getter because the parallel metadata probe may fill the resolved title in after wiring is set up. Behaviour unchanged. - CL4 — Deep nesting:
updater.downloadAppUpdate— Promise →net.request→response→datawith nested conditionals/teardown is the hardest-to-follow block. Fix: extract astreamToFilehelper. Fixed (Batch 21, pin-then-refactor): the security-critical invariants were first locked with 14 behavior-pinning tests at the publicdownloadAppUpdateboundary (test/updaterDownload.test.ts, scriptednet.requestfake) — untrusted initial URL, per-hop redirect re-validation on both the checksum fetch and the installer, missing/malformed checksum refusal, SHA-256 mismatch + partial cleanup, truncation detection, idle-timeout, checksum-phase and mid-download cancel, HTTP-error status, combined-checksum line matching, and progress events. Then the nested block was extracted verbatim intostreamInstallerToFile(redirects/hashing/timeout/truncation/ teardown in one once-wrapped helper);downloadAppUpdateis now a linear pre-flight (trust-check → checksum → stream), with the B6 cancel-slot ownership kept beside the module state it guards viaonCancelReady/onSettled. All pins passed before AND after the extraction — no behavior change. - CL5 — Superfluous exports.
PROGRESS_TEMPLATE(L167) andgetManagedBinDirareexported but used only within their own module. Fix: make them module-private. - CL6 — Redundant wrappers (minor):
MediaThumb'skind === 'audio' ? 'audio' : 'video'(L15),clearDir=update({[t]:''}), and the storeopenFile/showInFolderthin wrappers aroundwindow.api. Harmless; collapse opportunistically. Resolved: the genuinely-redundant one is gone —MediaThumb's no-op ternary was collapsed in L15. The other two are deliberate, not redundant:clearDirand the storeopenFile/showInFolderare the view-model layer (CC13) — the store is the single place components reachwindow.api, so keeping components free of direct IPC is the point, not incidental wrapping. Left as-is on purpose.
Magic numbers / strings (consolidated, see L10): timeouts 15_000/30_000/60_000/180_000,
maxBuffers 4/64/128/256×1024×1024, caps 500/200/100/20000/80, VIRTUALIZE_AT 100,
installer handoff 1500, tickers 650/15_000, stderr tail 4000; magic strings 'enc:v1:',
'persist:aerofetch-login', 'AeroFetchDailySync', '--sync', 'prog|'/'path|' (CL1).
Fix: a constants.ts (CC11) — names over literals.
What can safely be removed (recommendation)
- Now (zero risk): the
getDefaultFolderslice (M2); un-exportPROGRESS_TEMPLATE+getManagedBinDir(CL5). - Now (housekeeping): prune
dist/to the current release (L84); the macOS-only branches if Windows-only is committed (L147). - Decision made — wired, not deleted (all resolved): command preview (M5), incognito (M6), and per-download options/extraArgs (UX1) are now surfaced in the DownloadBar Advanced panel, so the previously-unreachable plumbing is reachable. Nothing to delete.
- Not removable (intentional): preview seed/mock data +
PREVIEWbranches (C1/L128) — they power the browser-preview dev workflow; consolidate (C1) rather than delete.
Simplification opportunities
Reusable abstractions that shrink the code and read better — no premature optimization, just de-duplication. Organized by the requested categories; each lists the repetition (with refs), the abstraction, and a rough LOC delta (net, after the helper's own cost). Estimates are deliberately conservative.
| # | Category | Repetition (refs) | Reusable abstraction | ~Net LOC |
|---|---|---|---|---|
| SIMP1 | Repeated file handling | JSON read/save+cap+try/catch in history/errorlog/templates; sources already has it (M1, CC10) | createJsonStore<T>(file, isValid, cap) |
−40 |
| SIMP2 | Repeated networking | execFile-in-Promise ×5 (probe/ytdlp×2/ffmpeg/probeMeta/indexer) (CC4/CC5) |
execFileAsync + spawnYtdlpJson() |
−40 |
| SIMP3 | Repeated parsing | byte/speed/eta/duration formatters ×4 (M9, L165, L166) | lib/format.ts (hour-aware, one precision rule) |
−40 |
| SIMP4 | Repeated parsing | YouTube id/URL parsing ×4 (H2) | lib/youtube.ts |
−30 |
| SIMP5 | Repeated logic | stdout line-buffering ×2 + taskkill tree-kill ×2 (download.ts/terminal.ts) (CC3, new) |
lineStream(stream, onLine) + shared killTree(pid) |
−25 |
| SIMP6 | Repeated commands | 7 identical preload on* subscribe/unsubscribe wrappers (new) |
subscribe(channel, cb) helper in preload |
−20 |
| SIMP7 | Repeated event handlers | Set<string> add/remove/toggle/toggle-all in DownloadBar/Library/History (new) |
useSelection() hook |
−30 |
| SIMP8 | Repeated view models | async-action setBusy/try/catch/finally + result + error ×~7 in SettingsView (new) |
useAsyncAction() hook |
−50 |
| SIMP9 | Repeated UI (cards) | hand-styled <div> card surfaces ×6 (UI7/UI20/L120) |
<Surface tier> (or use Fluent Card) |
−30 |
| SIMP10 | Repeated UI (settings) | sectionHeader + Card ×11 and Switch+Field+"On/Off" ×~12 in SettingsView |
<SettingsCard icon title> + <ToggleField> |
−90 |
| SIMP11 | Repeated UI (row actions) | Hint+Button icon-action ×~16 (QueueItem/History/Templates) |
<IconAction label icon onClick> |
−55 |
| SIMP12 | Repeated UI (empty states) | centered icon+text empty block ×3 (L116) |
<EmptyState icon title hint?> |
−25 |
| SIMP13 | Repeated dialogs/banners | suggestion banner ×2 (UI19); status chips Badge-vs-pill (UI18, M8) | <LinkSuggestion> + <StatusChip> |
−40 |
| SIMP14 | Repeated UI (controls) | two hand-rolled segmented controls (UI14) | <SegmentedControl> |
−30 |
| SIMP15 | Repeated validation | type-guards + sanitizeOptions + setSettings switch (CC9, M24) |
one schema (zod) validate+coerce | −60 |
| SIMP16 | Repeated networking | net.request + redirect-revalidate + timeout ×2 in updater (fetchTrustedText/downloadAppUpdate) (new) |
trustedRequest(url, {sink}) |
−30 |
| SIMP17 | Repeated logic (misc) | newId ×3 (M7), PREVIEW ×8 (C1), (ARR as readonly string[]).includes ×N (L90) |
lib: newId, isPreview, isMember |
−15 |
| SIMP18 | Repeated converters | MediaKind ×2 (L105), kind|quality string encode/decode (CL2/L91) |
single-source type + typed pair | −10 |
Highest-value (do these first)
- SettingsView decomposition (SIMP10 + SIMP8 + SIMP11). A
<SettingsCard>+<ToggleField>+useAsyncAction+<IconAction>would take the 1104-line file to roughly ~600, and the per-card split (H1) falls out for free. Biggest single readability win. createJsonStore(SIMP1) collapses three near-identical persistence modules to thin configs and removes the M1/CC10 "two ways to persist" seam.lib/format.ts+lib/youtube.ts(SIMP3/SIMP4) remove the most-copied pure utilities and fix the formatter divergences (L165/L166) at the same time.execFileAsync/spawnYtdlpJson(SIMP2/SIMP5) unify five spawn sites + the duplicated kill/line-buffer.
Estimate
Gross duplicated lines removable ≈ 650–750; the new helpers/primitives add back ≈ 150–200, for a
net reduction of ~550–800 lines (~5–7% of the ~12.5K total). Concentration: SettingsView ~−450
(1104 → ~600), the JSON stores ~−40, main spawn/format/util helpers ~−120, shared UI primitives
~−150. The real payoff is readability and killing the "two-ways-to-do-X" seams (M1/M8/M9/UI14/UI18/CC*),
not the line count. Caveat: stop short of over-abstracting — a createMirroredStore factory or a
generic form engine would cost more clarity than it saves; keep abstractions at the "one obvious helper" level.
Bug hunt (execution-path review)
Tracing real code paths for latent bugs by category. New bugs get B IDs; the genuine bugs found in
earlier passes are listed so this is complete; and the categories that came back clean are recorded
(bug-hunting that finds nothing is a result too).
New bugs
- B1 — No stall/idle timeout on downloads (resource leak + stuck slot).
spawn(ytdlp, …, { windowsHide: true })(download.ts:306) sets notimeout, andbuildArgsemits no--socket-timeout. A hung connection means yt-dlp never exits → theactivemap entry and the renderer'sdownloadingitem persist forever, permanently consuming a concurrency slot with no recovery or feedback. (The app-updater download does haveDOWNLOAD_IDLE_TIMEOUT_MS; downloads don't.) Fix: pass--socket-timeoutand/or an app-side idle watchdog that kills + errors a stalled child. - B2 — Source indexing can't be cancelled.
indexSourcewalks a channel's playlists with sequential probes (each up to the 180 sprobeFlattimeout); the UI shows "indexing…" with the Index button disabled and no cancel. A large channel locks the add-source flow for minutes with no abort. Fix: thread anAbortSignal/cancel token and a Cancel button. Fixed end-to-end. Main: indexer.ts threads anAbortSignalthroughprobeFlat/probeTab(→ execFileAsync'ssignal, which kills the in-flight yt-dlp child) and checkssignal.abortedat every walk boundary, returning acanceledresult that pushes no error progress; a per-walkAbortControlleris registered so a newsources:index-cancelIPC (cancelIndexing()) aborts it. Renderer: a "Cancel" button in the LibraryView add-source progress row callsuseSources.cancelIndexing(), which optimistically clears the indexing UI and flags the pending index/reindex so its result is treated as canceled (no error toast) rather than a failure. Background sync (sync.ts) stays non-cancelable by design (unattended). Verified live that the abort actually stops the walk: aborting a real@LinusTechTips/videos-J --flat-playlistprobe fired the callback 2 ms later withAbortErrorandtasklistconfirmed the yt-dlp process was gone — Node's AbortSignal terminates the native probe on Windows. typecheck + 302 tests + production build green. - B3 —
extractSha256ignores the filename (wrong-hash risk). It returns the first 64-hex token in the file (updater.ts); a combined multi-file checksum uploaded as<asset>.sha256would verify the installer against the wrong line's hash. Safe only by the one-hash- per-asset convention. Fix: match the hash on the asset's filename line. - B4 —
probeMetaassumes one line per--printfield. It doesstdout.split('\n')→[title, uploader, duration](download.ts); a title containing a newline shifts channel and duration by a line. Fix: use a single--printwith an unlikely delimiter, or-J. - B5 — Missing status guard on the
metaevent. Betweencancel()and the child'sclose,active.has(id)is briefly true, soprobeMetacansendametaevent for a just-canceled item;applyEvent'smetacase (unlikeprogress/done/error) has no canceled guard, so it updates a canceled item's title. Harmless today, but the asymmetry is a latent bug. Fix: guardmetalike the others. - B6 — App-update download has no cancel. Once
downloadAppUpdatestarts there's no user abort — only completion or the idle timeout stops it; the progress UI offers no Cancel. Fix: expose cancel (abort the request). Fixed. updater.ts now stashes the in-flight download's canceller (a closure over the existingfinish({ ok:false })) in a module var, set when the download starts and cleared on settle; a newcancelAppUpdate()+app:update-cancelIPC invokes it, routing through the SAME teardown the audit's failure-path review already verified —request.abort(), destroy the write stream, and unlink the partial. Renderer: a "Cancel download" button under the progress bar in SoftwareUpdateCard.tsx calls it and sets acanceledRefso the resulting not-ok download reads as canceled (no error toast), not a failure. Verified the teardown on the real Electron runtime: anet.requeststreaming to a temp file was aborted mid-download via that exactabort()+destroy()+unlink()sequence and the partial was confirmed gone from temp afterwards (no lingering Windows file handle). CL4 (the updater's deep-nesting refactor) stays deferred as a separate cosmetic cleanup. typecheck + 302 tests + production build green. - B7 — Cookie-login promise can never resolve if the window is destroyed without
close.openCookieLoginWindowresolves only via theclose→exportAndResolvepath; adestroy()(or aclosedwithoutclose) would leavependingResolverspending forever. Not triggered by current code, but a latent never-resolve. Fix: also resolve/reject onclosed.
Real bugs already filed (confirmed in this pass)
- M32 playlist/channel batches download in reverse order · M35 History re-download duplicates rows
· M34 optimistic settings never reconcile (UI shows unsaved values) · L88 a
progressevent can flip aqueueditem todownloadingoutsidepump()· L139 startup yt-dlp auto-update vs a concurrent download spawning the same exe (Windows file-in-use) · L140 retry-during-teardown hits the "already running" guard · L148clearFinishedcan free a slot before the killed tree exits (brief over-cap) · L157markDownloadedno-ops afterremoveSource(orphan downloads).
Came back clean (no bug found)
- IPC-after-destroy: every
wc.send/e.sender.send/mainWindow.*is guarded byisDestroyed(). - Threading / UI threading / synchronization: single JS thread in main and renderer; React state
updates are always on-thread; the
activemap + module vars are main-thread-only — no data races. - Infinite loops: the stdout line-buffer advances each iteration;
parseExtraArgs/parseRssVideoIdsregexes can't zero-width-match; no self-recursivepump. - Stream disposal (updater):
downloadAppUpdatehas a singlefinish()that aborts the request, destroys the file stream, and unlinks the partial — correct on every failure path. - Event-listener cleanup: component
useEffectsubscriptions return their unsubscribe; module-level subscriptions are app-lifetime by design. - Off-by-one / boundary:
parseProgressguardsparts.length < 6; playlist/collection indexing is 1-based throughout;MAX_ENTRIESslices are correct;fmtBytesclamps the unit index.
Resilience & failure-mode review
A fresh lens: what happens when the environment fails (disk full, permission denied, corrupt file,
killed mid-write, huge inputs) rather than when the logic is wrong. These are robustness findings, not
cosmetics. R IDs.
- R1 — JSON stores are not written atomically (corruption on crash/power-loss). The hand-rolled
stores (history/errorlog/templates/sources/media-items) persist via
writeFileSync(file, JSON.stringify…)— not write-temp-then-rename. A crash/power-cut mid-write truncates the file; the next read hits a parse error and returns[].electron-store(settings) is atomic, so durability is inconsistent across the app. Fix: an atomic write (temp + rename, orwrite-file-atomic) in the sharedjsonStore(SIMP1). - R2 — A corrupt/invalid store silently loses all data.
readJsonArray/listHistoryetc. catch a parse error and return[]with no warning, backup, or recovery — one bad byte wipes the user's history/ sources/templates from their perspective. Fix: on parse failure, back up the bad file (.corrupt) and surface a notice. - R3 — O(n) full-file rewrite per download completion (≈O(n²) per channel), synchronously on the
main thread.
setMediaItemDownloaded(sources.ts:126) reads + parses the entiremedia-items.json(up toMAX_ITEMS= 20,000) and rewrites the whole file on every completion, andaddHistoryrewrites all ofhistory.jsonlikewise. Downloading a 1,000-video channel ⇒ ~1,000 full read-parse-write cycles of a multi-MB file, blocking the IPC/event loop each time. Fix: in-memory cache + debounced/batched atomic writes (or the deferred better-sqlite3). - R4 — Canceled downloads orphan
.partfiles.cancelDownloadtree-kills yt-dlp but never deletes the partial; canceled items vanish from the UI while.part/fragment files accumulate in the output folder. (App crash mid-download likewise — the queue isn't persisted to resume, M4.) Fix: clean up the partial on cancel, or surface/offer-resume for orphans. Fixed. The engine now captures the resolved FINAL output path during the run via a new--print before_dl:dest|%(filename)s(empirically:%(filepath)sis unresolved that early,%(filename)sgives the full path), stored on the download in download.ts. On CANCEL (not pause — pause keeps the.partfor resume, L65) the child's close/error handler runscleanupPartials, which uses the pure, unit-tested orphanPartials to delete only this download's unambiguous intermediates:.part,.part-Frag*,.ytdl, and finished per-stream<stem>.fNNN.<ext>files — never a bare<stem>.<ext>completed file (so a pre-existing same-title download is safe) or another download's files. Verified live: a real 1080p download killed with the app's exacttaskkill /T /Fleft…f399.mp4.part, and on close the cleanup deleted ONLY that orphan while a planted same-title.webmand an unrelated download's.partboth survived — and the handle was released by close time (no lock error). The predicate is covered across every partial type by test/partials.test.ts. Cleanup runs from the settle handler, so the process (and its file handles) are already gone. - R5 — Settings write failure is unhandled.
setSettingscallsstore.set(...)with no try/catch; on disk-full/read-onlyelectron-storethrows → the IPC rejects → the renderer'ssetSettings().catch(() => {})swallows it while the optimistic UI keeps the change (the disk-full form of M34). Fix: catch and report. - R6 — Silent data loss on any store write failure. A completed download that can't be recorded
(disk full) shows "Completed" in-session but is gone on restart, with no error (the write
catchis empty, M29). Fix: detect write failure and warn. Fixed (log half) in jsonStore.ts:writeJsonAtomicnow returns whether the write landed and logs the failure (with the path) instead of an empty catch, and removes the leftover.tmp;createJsonStore'sflushonly clears the dirty flag on success, so a failed write stays pending and is retried on the nextwrite()or the quit-timeflushAllStores()rather than dropped. Unit-tested intest/jsonStore.test.ts. Scope note: like M29 this delivers the "no longer silent" half (logged + retried); the user-facing toast needs the leveled file logger / toast sink deferred to CC8. - R7 —
safeStorageunavailable ⇒ secrets silently stored as plaintext.encryptSecretfalls back to plaintext when DPAPI/safeStorage isn't available, with no indication — compounds H7 (cookies) and the backup-cleartext issue (M22). Fix: warn (or refuse to store) when encryption is unavailable. Fixed in settings.ts:encryptSecretnowconsole.warns before returning a non-empty secret as plaintext, so the cleartext fallback is no longer silent. Chose warn-not-refuse to preserve the pre-encryption behaviour (storing still works); DPAPI is effectively always present on Windows, so this should never fire in practice. (refusing/at-rest enforcement remains an option if ever wanted.) - R8 — Pretty-printed JSON for large stores.
JSON.stringify(value, null, 2)inflates size/write time for the potentially-20k-item media store (ties R3). Fix: compact JSON for the large stores. - R9 — Clock-skew sensitivity.
shouldAutoCheckYtdlpand the scheduled-download promoter compareDate.now(); a backward system-clock correction can skip or double-fire a check/schedule. Minor; note for awareness. Fixed the concrete case:shouldAutoCheckYtdlpnow treats alastCheckin the future (clock ran backward → negative interval) as due, so the daily update check can't be skipped forever (unit-tested). The scheduled-download promoter is inherently fire-once — each promotion flips the item out ofsaved, so a backward jump only delays it and can't double-fire; a forward jump firing early is documented and accepted as minor (a monotonic scheduler would be overkill for a park-until-time feature).
Ship-readiness (perceived quality)
Obsessive "does it feel finished" pass. The biggest untapped vein is defaults (first-run impressions),
plus a few stuck-status / progress glitches. SR IDs; already-filed perceived-quality items are listed at
the end so the picture is complete. Verified-clean first: card/label/button text is consistently
sentence case, and a careful read found no spelling typos — the polish gaps are defaults, jargon, and a couple of stuck states, not sloppy text.
Poor / risky defaults (first-run impressions)
- SR1 — Theme defaults to
'light', not'system'. A dark-mode Windows user gets a bright white app on first launch despite the app fully supporting "follow system." Single most-noticeable first-run miss. Fix: defaulttheme: 'system'. - SR2 —
ytdlpChanneldefaults to'nightly'. The app ships auto-pulling yt-dlp's daily, less-tested build (auto-update also on by default); one nightly regression breaks downloads for everyone. Defensible for chasing YouTube changes, but a risky default for a commercial release. Fix: considerstableas the shipped default. - SR3 —
autoDownloadNewdefaults totrue. 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 —
clipboardWatchdefaults totrue. 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. Fixed:DEFAULT_SETTINGS.clipboardWatchis nowfalse(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. - 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). 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-qualityhandling 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
- SR6 — "Resolving…" can stick forever. A queued item's channel shows the
'Resolving…'placeholder until ametaevent arrives; if metadata never resolves (error before meta, or no channel in the probe), the row reads "Resolving… • 1080p • Video" permanently — looks hung. Fix: clear the placeholder on error / when meta fails. - SR7 — Progress bar visibly resets mid-download. A video+audio merge downloads as two streams, so the bar fills 0→100%, then resets to 0→100% again before muxing — looks like a glitch/restart to the user. Fix: weight the two phases (e.g. 0–50% / 50–100%) or show a "merging…" state.
- SR8 — Window title never reflects state (L108/W19): always the static "AeroFetch," so the taskbar can't show "3 downloading." A finished-feeling app updates its title/tooltip with activity.
Wording / jargon polish
- SR9 — Dev jargon in user-facing option hints (extends M37): "mux them into the video," "Tiebreaker,
not a hard filter," "Endcards / credits" ("End cards"), "Proof-of-Origin token," "
--extractor-args." Reads like engineer notes. Fix: plain-language hints. - SR10 — Inconsistent product self-description / casing surfaced to users: "yt-dlp frontend" caption flips to "v0.5.0" on load (L66); "Sign in" vs "Sign-in" (L95); "PO token" vs "PO Token" (L150).
Also reduces perceived quality (already filed — listed for completeness)
- Onboarding has no folder picker / feels thin (UX5/UX22); no boot loading state + one-frame theme flash
(UX26); refresh-icon mismatch ArrowClockwise vs ArrowSync (L106) and Regular/Filled brand icon (UI13); only
the sidebar animates, everything else snaps (UI26); separator glyph/spacing inconsistency
•/·(L164); the command palette is undiscoverable (UX21). (The unsigned-build SmartScreen prompt is accepted as a non-goal — no certificate.)
Ship-blockers (highest perceived-quality impact): SR1 (light-on-dark first launch), SR7 (progress visibly restarting), SR6 ("Resolving…" stuck), and W14 (placeholder icon) — all now resolved. The unsigned-build SmartScreen prompt is accepted as a non-goal (no certificate). Together these were most of the "this feels finished" gap.
Performance (no premature optimization — real hot paths only)
A fresh lens: redundant work on hot paths and scaling cliffs, not micro-tuning. PERF IDs. Verified
acceptable first: memoized QueueItems don't re-render when their item is unchanged (the applyEvent
map preserves references for untouched items); the queue + large library lists are virtualized;
electron-store is constructed lazily. The issues:
- PERF1 —
getSettings()is heavy and on every hot path. It runs per download spawn (twice — inbuildCommandand themaxConcurrentcheck), per completion (notify), and on the system-theme bridge; each call re-reads the whole store and, for users who've configured a proxy/PO-token/update-token, runs up to 3safeStorage.decryptString(DPAPI) calls. Fix: cache the decrypted settings, invalidate on write. - PERF2 —
templates.jsonis read+parsed on every download.resolveExtraArgsevaluatestemplates: listTemplates()eagerly as an argument (download.ts:182) beforeselectExtraArgscheckscustomCommandEnabled, so the file is read even when custom commands are off. Fix: pass a lazy getter, or short-circuit on the gate first. - PERF3 —
pump()is O(n) per queue event. Itfilter+reverse+slices the entire items array 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. Resolved with benchmark evidence, no rewrite (Batch 22): test/pumpBench.test.ts measures pump()'s exact array pipeline on the real item shape — median 0.43 ms for a full scan+promote over 5,000 items and 1.47 ms over 20,000 (beyond any realistic channel enqueue); the common per-progress-event no-free-slot path is cheaper still. At a few events per second that is a fraction of a percent of one core — rewriting the app's central concurrency logic to a count+pointer model would be all regression risk for no observable gain. The benchmark stays in the suite as a regression tripwire (5 ms budget). - PERF4 —
summarizeQueueruns 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 (nouseMemo, L94) — each O(items), every ~second per active download. Fix: compute once, memoize, throttle the taskbar push. Fixed with L94:queueSummaryOfcomputes the aggregate once per items change (shared ref cache), so App's subscription and DownloadsView's render no longer each run it; the taskbar IPC push was already deduped by computed value (L25). - PERF5 —
VirtualListgets new function identities each render.estimateSize/getKey/renderItemare passed as inline arrows, so the virtualizer can't rely on stable references (risking re-measures/cache churn). Fix:useCallback/hoist them. Fixed: the three callbacks are hoisted to stable module-level functions in DownloadsView.tsx. - PERF6 — Per-thumbnail store subscriptions. Each
MediaThumbcallsuseResolvedDark()(two store subscriptions) (L162); a long list creates N×2 subscriptions. Fix: resolve once, pass as a prop. Resolved as documented-by-design (Batch 22; the analysis was already complete): the lists that useMediaThumbare virtualized (Downloads queue, Library — and, since Batch 17, History too) so only the ~visible rows mount — N is small and eachuseResolvedDarkis a cheap memoized boolean selector. ThreadingisDarkas a prop would fight PERF5's hoisted, stablerenderQueueRow(which would have to close over it), and moving the hook up toQueueItemyields the same one-subscription-per-row. Low value, mild conflict — the current shape is the deliberate choice. - 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
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.lazythe heavy tabs. Fixed: Library/History/Terminal/Settings areReact.lazychunks behind aSuspensein App.tsx (Downloads, the launch tab, stays eager). The production build now emits separateSettingsView(~90 kB),LibraryView(~29 kB),HistoryView(~16 kB), andTerminalView(~8 kB) chunks — ~140 kB out of the initial bundle, loaded on first visit. The stores these views use are imported eagerly elsewhere (App'suseDownloads+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 bugs (B1, M32, M35) — listed for completeness, not urgency.
Completed
Security & correctness audit (2026-06-23):
- S1 — Backup import: confirm before enabling imported custom-command templates.
- S2 — Cookie login window: restrict popups to http(s)/about: schemes.
- S3 — Enforce
maxConcurrentin main (startDownload), not just the renderer. - S4 — Path-traversal sanitization for
filenameTemplate+outputDir. - S5 — Per-row validation on persisted JSON reads.
- C1 (orig) — Bundle the missing
ffprobe.exe; assert ffmpeg+ffprobe presence up front. - P1 — Stop
getSettings()writing to disk on every read. - P2 — Forward the renderer's pre-probed metadata to main; skip the redundant probe.
- P3 — Document "lower maxConcurrent mid-flight doesn't pause overflow".
- M1 (orig) — Extract duplicated
cleanErrortolog.ts. - M2 (orig) — Document
parseExtraArgsquoting limitations. - M3 (orig) — App icon placeholder (designed asset still wanted pre-v1.0).
Deferred
- Designed app icon before v1.0 (placeholder shipped).
- Migrate JSON stores to better-sqlite3 if a user indexes many large channels.
Metadata editing (— done:--parse-metadata/--replace-in-metadata)--replace-in-metadatainDownloadOptionsForm.tsx(ROADMAP.md "Metadata editing before download").PO-token WebView auto-minting— done: poToken.ts opens a hardened BrowserWindow on the shared login session and mints the token automatically (ROADMAP.md "Reliability").Taskbar overlay badge (needs a drawn— done (W11): badge.ts.NativeImage)- i18n — deliberately deferred (note: UI strings are hard-coded throughout).
- Code signing — non-goal: no certificate will be purchased, so builds ship unsigned and the SmartScreen
prompt is accepted. The build remains signing-ready via
CSC_LINK/CSC_KEY_PASSWORDenv vars if this ever changes — see SIGNING.md. - Live smoke-test of OS-level wiring (tray/jump list/scheduled sync/
aerofetch://).