Twitch-VOD-Manager/docs/IMPROVEMENT_LOG.md

Improvement Log

Dated entries from improvement cycles. Newest at top.

2026-05-03 — Cycle 4: GQL retry + VOD sort + shutdown consolidation

Three independent improvements landed this cycle.

1. Public Twitch GQL fallback retries on transient failures (defensive error handling)

• File: src/main.ts — new isTransientAxiosError + retry loop in fetchPublicTwitchGql.
• Problem: fetchPublicTwitchGql swallowed every network error with catch (e) { console.error(...); return null; }. The public-API fallback path is what users without a Twitch client_id/secret hit on every VOD list load — a single TCP RST or a transient 503 from gql.twitch.tv produced an empty list and the user had to click refresh.
• Fix: Up to 3 attempts with exponential backoff (400ms × 2^(attempt-1) + jitter, capped by attempt count). Retries cover transient HTTP (408, 429, 5xx) and pure network failures (no response). GraphQL errors in errors[] are still returned without retry — those are application-level rejections of the query itself. Recovery is logged via appendDebugLog('public-gql-recovered', ...) so we can later see in logs whether the retries actually pay off.

2. VOD list sort dropdown with persistence (client feature: VM/state + UI + persistence)

• Files: src/renderer-streamers.ts, src/renderer.ts, src/renderer-texts.ts, src/index.html, src/renderer-locale-de.ts, src/renderer-locale-en.ts.
• Problem: VODs always rendered in the order Twitch returned them (sort:TIME desc). With long archives users had no way to find the longest stream, the most-watched, or the oldest.
• Fix: vodSortSelect dropdown next to the filter input. Five sort modes: newest first, oldest first, most viewed, longest first, shortest first. State (vodSortKey) persisted to localStorage under twitch-vod-manager:vod-sort and validated against an enum on load — an unknown stored value falls back to date_desc so a future rename can't strand the user. renderVodGridFromCurrentState now applies sortVods before filterVodsByQuery so the filter sees the sort order and the match-counter is consistent. Sort labels and the "Sort:" prefix label are localized (DE + EN), and refreshVodSortSelectLabels re-runs on language switch so the option labels stay in the active language. Browser-default keyboard nav on the select (arrow keys, type-ahead) covers keyboard access.

3. shutdownCleanup() consolidates window-all-closed + before-quit (cleanup of meaningful size)

• File: src/main.ts.
• Problem: Both lifecycle handlers ran nearly identical cleanup blocks but had drifted: window-all-closed killed children and was platform-aware (app.quit() on non-darwin), before-quit only stopped timers and saved state. There was no single place to add a new "must run on exit" step — every future addition had to be pasted into both handlers and inevitably one would diverge.
• Fix: Single shutdownCleanup(reason) helper, gated by an idempotent shutdownCleanupDone flag so a before-quit immediately following a window-all-closed is a no-op. The helper kills activeDownloads, activeClipProcesses, and currentEditorProcess (with try/catch so an already-exited proc doesn't throw), persists config + queue, then stops timers. Debug-log flush is reordered to run AFTER saveConfig / flushQueueSave so any error in those persistence calls actually reaches the log file before the flush timer is gone. Both app.on(...) handlers shrank to one line each.

Regression

• npm run build — clean (TypeScript strict, 0 errors).
• npm run test:e2e:update-logic — passed.
• npm run test:merge-split — passed.
• npm run test:e2e — passed (issues: []).
• npm run test:e2e:guide — passed (failures: []).
• npm run test:e2e:full — passed (failures: [], runtimeIssues: []).

2026-05-03 — Cycle 3: clip hardening + VOD filter + cancel-cross-talk fix

Three independent improvements landed this cycle.

1. download-clip IPC: integrity, cancellation, sanitization (server defensive)

• File: src/main.ts — download-clip IPC handler, new activeClipProcesses map.
• Problem: The handler reported success: true on streamlink exit code 0 even when the resulting file was empty / a few hundred bytes (Twitch occasionally returns a manifest with no segments). The path passed clipInfo.broadcaster_name straight to path.join — Twitch returns the broadcaster's display name, which can carry unicode, spaces, or punctuation that produced surprising directory layouts on Windows. The spawned streamlink process was tracked nowhere, so window-all-closed left it orphaned.
• Fix: safeBroadcaster runs through sanitizeFilenamePart. safeTitle falls back to clip when the title sanitises to empty. The output filename now goes through ensureUniqueFilename(path, clipId) so retrying a clip with the same title doesn't overwrite the previous download. After streamlink exits, the file is rejected if smaller than 16 KiB or if validateDownloadedFileIntegrity fails (no video stream / unreadable). The proc is tracked in a new activeClipProcesses map and killed by window-all-closed.

2. VOD list filter / search (client feature: VM/state + UI + persistence + keyboard)

• Files: src/renderer-streamers.ts, src/renderer.ts, src/renderer-texts.ts, src/index.html, src/renderer-locale-de.ts, src/renderer-locale-en.ts.
• Problem: A streamer can have hundreds of VODs (the test fixture alone has 37 cards). There was no way to find a specific VOD by title — only scroll. With a long archive this is genuinely painful.
• Fix: Filter row above the VOD grid (vodFilterInput, clear button, match counter). State (vodFilterQuery) is persisted to localStorage via loadPersistedVodFilter / persistVodFilter, so the search bar survives an app restart. The render path was split: renderVODs now stores lastLoadedVods + lastLoadedStreamer and delegates to renderVodGridFromCurrentState, which applies filterVodsByQuery on every input event without re-fetching. Empty-state DOM is built via setVodGridEmptyState using createElement + textContent (no innerHTML for locale strings — defense-in-depth even though the strings are trusted). Keyboard: Ctrl+F / Cmd+F focuses the filter (only when the VODs tab is active and Electron's no-op default is suppressed); Esc clears the filter when the input has focus and content; Esc still closes modals first if any are open.

3. Decouple currentProcess from queue downloads (server cleanup + race fix)

• File: src/main.ts — global rename and assignment removal.
• Problem: A single currentProcess: ChildProcess | null was shared by cutVideo, mergeVideos, splitMergedFile, AND downloadVODPart. With parallel downloads the global was constantly overwritten between siblings, but the cross-talk that mattered was different: if a queue download was running and the user kicked off a video cut, the cutter ffmpeg ran into the same global. Pressing the queue's cancel-download button then iterated activeDownloads (correct) AND called currentProcess.kill() (incorrect — that was the cutter ffmpeg by then), killing the unrelated cut.
• Fix: currentProcess renamed to currentEditorProcess and confined to the editor pipeline (cutter / merger / splitter). downloadVODPart no longer assigns to it — activeDownloads is the sole source of truth for queue children. The fallback if (currentProcess) currentProcess.kill() was removed from remove-from-queue, pause-download, and cancel-download. window-all-closed still kills it (so a cutter ffmpeg gets cleaned up on app exit) and now also kills activeClipProcesses introduced by Pick 1.

Regression

• npm run build — clean (TypeScript strict, 0 errors).
• npm run test:e2e:update-logic — passed.
• npm run test:e2e — passed (issues: []).
• npm run test:e2e:guide — passed (failures: []).
• npm run test:merge-split — passed.
• npm run test:e2e:full — passed (failures: [], runtimeIssues: []; flows: language switch, queue add, duplicate prevention, runtime metrics, clip queue, pause/resume, retry, reorder, media cut/merge, update check).

2026-05-03 — Cycle 2: release pipeline + defensive parsing

Three independent improvements landed this cycle.

1. scripts/release_gitea.mjs skips rebuild when artifacts exist (release pipeline)

• File: scripts/release_gitea.mjs.
• Problem: The script unconditionally ran npm run dist:win (full test suite + electron-builder) even when the version's artifacts were already on disk under release/. When npm run test:e2e was broken (cycle 1 follow-up), the release path was unusable — the previous cycle had to bypass the script with direct API uploads via PowerShell. Every future agent would hit the same wall.
• Fix: New --skip-build flag. The script now also auto-detects whether all 3 required artifacts (Setup-<v>.exe, Setup-<v>.exe.blockmap, latest.yml) exist for the requested version and skips dist:win accordingly. The auto-skip is the safe default — explicit --skip-build documents intent. Help text updated to describe the new flag and the auto-skip behaviour.

2. playwright in devDependencies + simplified test scripts (release pipeline)

• Files: package.json (+ package-lock.json).
• Problem: npm exec --yes --package=playwright -- node scripts/smoke-test*.js failed with MODULE_NOT_FOUND in environments where npm exec couldn't resolve playwright on the fly (clean caches, locked CI runners). Cycle 1 worked around it with npm install --no-save playwright. Result: the documented test path was unreliable.
• Fix: playwright ^1.59.1 added to devDependencies. test:e2e, test:e2e:guide, test:e2e:full now invoke node scripts/smoke-test*.js directly — require('playwright') resolves locally. No browser binary install needed because the smoke tests drive Electron via _electron, not a browser.

3. Defensive parsing in loadConfig and loadQueue (server-side correctness)

• File: src/main.ts — new isPlainObject / isValidQueueStatus / sanitizeCustomClip / sanitizeMergeGroup / sanitizeQueueItem helpers; rewritten loadConfig and loadQueue.
• Problem: loadConfig blindly spread JSON.parse(data) over the defaults. If the config file ever held a non-object (corrupt, manually edited to an array, partial write before Cycle 1's fsync landed), the spread either dropped values silently (primitives) or polluted the config object (arrays became numeric keys). loadQueue only validated id, url, status are strings — it accepted customClip / mergeGroup of any shape, never validated progress was a finite number, and notably never normalized stale status: 'downloading' items. After a hard kill mid-download, those items came back marked as still downloading with no actual download running, and start-download only resurrected paused items, leaving them stuck.
• Fix: loadConfig checks isPlainObject(parsed) before spread; non-objects are logged and ignored, defaults used. loadQueue runs every entry through sanitizeQueueItem which validates the status enum, normalizes progress to [0, 100], validates and normalizes customClip / mergeGroup shapes, and demotes stale status: 'downloading' to pending with progress = 0 so the user can actually resume the queue. Invalid items are dropped with a count logged. As a bonus, the previously-unused CustomClip and MergeGroupItem type imports now have call sites.

Regression

• npm run build — clean (TypeScript strict, 0 errors).
• npm run test:e2e:update-logic — passed.
• npm run test:e2e — passed via the new direct script path (no npm exec workaround), issues: [].
• npm run test:e2e:guide — passed.
• npm run test:merge-split — passed.
• npm run test:e2e:full — passed (failures: [], runtimeIssues: []; flows: language switch, queue, duplicate prevention, runtime metrics, clip queue, pause/resume, retry, reorder, media cut/merge, update check).

2026-05-03 — Cycle 1: stability & UX polish

Three independent improvements landed this cycle.

1. Atomic file writes survive power loss / crash mid-write (correctness)

• Files: src/main.ts — new writeFileAtomicSync helper, saveConfig, writeQueueToDisk.
• Problem: saveConfig and writeQueueToDisk used writeFileSync + renameSync. Node's writeFileSync does NOT call fsync — the OS may report the rename complete while the file content still sits in the write cache. A power loss / kernel panic between writeFileSync and renameSync could leave the renamed file empty or truncated. On next launch, JSON.parse throws and the app silently falls back to defaults (config) or [] (queue). Users would see "settings reset" / "queue lost" with no diagnostic in the debug log beyond a console.error.
• Fix: openSync(tmp, 'w') → writeSync(fd, buffer, 0, len, 0) → fsyncSync(fd) → closeSync(fd) → renameSync. The fsyncSync is wrapped in an inner try (some filesystems reject it, e.g. network shares); failure there is non-fatal but the close + rename order is always preserved. The Windows copy/unlink fallback for "rename failed because target locked" is kept.

2. Per-item filename claims fix parallel-download race (race condition + dead-code cleanup)

• Files: src/main.ts — ensureUniqueFilename, new releaseClaimedFilenamesForItem, every download call site, splitMergedFile signature.
• Problem: claimedFilenames was a global Set<string> and processOneQueueItem did claimedFilenames.clear() in its finally. With parallel downloads enabled (max 2), when item A finished, the clear() wiped item B's reservations too. In the narrow window between B claiming a filename via ensureUniqueFilename and streamlink actually writing the first bytes to disk, a third item entering the freed slot could compute the SAME filename (claim set empty, file not yet on disk) → both downloads would race writing the same path. The dead releaseClaimedFilename(filePath) function was defined at line 722 but never called from anywhere.
• Fix: New Map<itemId, Set<filename>> tracks which item claimed which filenames. ensureUniqueFilename(filePath, itemId) registers per-item; releaseClaimedFilenamesForItem(itemId) removes only that item's claims. splitMergedFile gained an itemId parameter so split-phase claims register correctly. The dead releaseClaimedFilename is gone, replaced by the per-item variant.

3. Renderer UX polish — robust progress lookup, persisted active tab, keyboard shortcuts (client-side feature)

• Files: src/renderer-queue.ts, src/renderer.ts.
• Problem(s) (small wins bundled as one coherent UX improvement):
  • updateQueueItemProgress indexed byId('queueList').children[idx] by array position — fragile if the queue array and DOM ever diverged for a frame (queue mutated after render-fingerprint shortcut, or during the throttled queue-sync window).
  • The active tab always reset to vods on app launch — annoying for users who live in settings, cutter, or merge.
  • No way to dismiss any of the three modals (clipModal, templateGuideModal, updateModal) without clicking the close button.
  • No keyboard navigation between tabs (only Del and S were wired).
  • The page title used to show the streamer name even when the user was on Settings or Cutter, because showTab always preferred currentStreamer over the tab title.
• Fix:
  • Look up queue items by [data-id="..."] selector instead of array index. Resilient to mutation between renders. Determinate / indeterminate progress class logic tightened (isDeterminate = progress > 0 && progress <= 100).
  • Active tab persisted to localStorage on every showTab; restored on init via loadPersistedActiveTab, whitelisted to known tab IDs (vods | clips | cutter | merge | settings) so a future rename can't strand users on a missing tab. Title logic fixed: streamer name only appears in the page title when the VODs tab is active.
  • Escape closes the topmost open modal regardless of focus (priority order: clip dialog → template guide → update modal). Works while typing in a modal input.
  • Ctrl+1..5 (or Cmd+1..5 on macOS) jumps directly to a tab. Existing Del (delete selected) and S (start/pause) shortcuts continue to work and remain blocked while typing in inputs.

Regression

• npm run build — clean (TypeScript strict, 0 errors, 0 new warnings).
• node scripts/smoke-test-update-version-logic.js — passed.
• node scripts/smoke-test-merge-split-logic.js — passed.
• node scripts/smoke-test.js — passed (37 VODs listed, queue add OK, preflight green, issues: []).
• node scripts/smoke-test-template-guide.js — passed (17 variable rows, live preview reactive, failures: []).
• node scripts/smoke-test-full.js — passed (failures: [], runtimeIssues: []; flows verified: language switch, queue add, duplicate prevention, runtime metrics, clip queue, pause/resume, retry, reorder, media cut/merge, update check).

ESLint reports 36 pre-existing warnings and 1 pre-existing error (control-character regex in sanitizeFilenamePart); none new from this cycle.