Renderer-side polish bundle. - updateQueueItemProgress now looks up items by [data-id] selector instead of array index. Resilient against queue/DOM divergence between renders. Determinate vs indeterminate progress logic tightened. - Active tab persisted to localStorage on every showTab; restored on init via loadPersistedActiveTab (whitelisted to known tab IDs so a future rename cannot strand the user on a missing tab). Page title now only shows the streamer name on the VODs tab — it no longer leaks into Settings / Cutter / Merge. - Escape closes the topmost open modal regardless of focus (clip dialog, template guide, update modal — in that priority order). - Ctrl+1..5 (Cmd+1..5 on macOS) jumps directly to a tab. The existing Del (delete selected) and S (start/pause) shortcuts still work and remain blocked while typing in inputs. Adds docs/IMPROVEMENT_LOG.md (new, single dated section for this cycle). Build: tsc clean. Full smoke suite green (failures: [], runtimeIssues: []). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
46 lines
5.2 KiB
Markdown
46 lines
5.2 KiB
Markdown
# Improvement Log
|
|
|
|
Dated entries from improvement cycles. Newest at top.
|
|
|
|
## 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.
|