kleb/SpotifFLAC
Watch
1
0
Fork
You've already forked SpotifFLAC
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Harden download pipeline and fix audit findings (stability, UX, perf) #9

Merged
kleb merged 6 commits from fix/audit-stability-ux-perf into main 2026-07-19 08:19:09 +02:00
Conversation 1 Commits 6 Files changed 64 +1758 -377
kleb commented 2026-07-19 01:32:17 +02:00
Owner
Copy link

Implements the fixes from the 2026-07-18 stability/UX/performance audit, on top of the in-flight media-transfer/atomic-write extraction, ESM migration, and parallel DASH fetching that this work builds on and gates.

Data safety

  • Metadata "repair" of a same-named existing file is now gated on a duration match (+/-3s) and distinguishes unreadable metadata from a missing ISRC tag; on any doubt the file is left untouched (src/main/core/download-manager.ts).
  • ISRC scans no longer delete files whose tags fail to parse (tidal.ts, qobuz.ts, fs/filemanager.ts) - a parse failure is not proof of corruption.
  • settings.json is written atomically (temp+fsync+rename, 0600) and validated on load with a {} fallback.
  • before-quit now aborts in-flight downloads with a bounded 3s drain; startup sweeps orphaned .part/.tmp/.repair-cover temp files from the download folder (new src/main/fs/temp-sweep.ts).

Stability / performance

  • Aggregate download progress/speed is derived by summing active queue items instead of last-writer-wins globals under concurrency.
  • queue:changed pushes carry the queue snapshot; both renderer hooks consume it directly instead of each re-invoking getters per event.
  • Renderer crash/hang recovery (render-process-gone -> reload) and window bounds persistence with off-screen protection.
  • Audio analysis kills ffmpeg once the sample cap is reached instead of buffering whole hi-res files in memory.
  • Amazon gateway polling uses a wall-clock deadline instead of summed intervals.
  • Progress meters animate transform: scaleX instead of width (no layout work per tick).

UX

  • Long artist-discography fetches are cancellable (new CancelSpotifyMetadata channel) with a Cancel button and loading skeleton.
  • Tidal auth failures show an actionable "Open Settings" toast instead of a generic dead end.
  • Offline banner via online/offline listeners.
  • Track-list pagination is windowed with ellipses; artwork lazy-loads with a clean error fallback.
  • Batch stop marks items "Stopped (partial)", keeps their counters, and resumes on Start.
  • First-run "Getting started" card; download-path existence validation (new ValidateDirectory channel); Spotify credential Verify button.

Tests

97 vitest tests pass, including new coverage: repair gating (4 cases), settings round-trip/corruption, temp sweep, atomic-write failure cleanup. oxfmt/oxlint/tsgo/knip clean; dev boot smoke-tested.

Implements the fixes from the 2026-07-18 stability/UX/performance audit, on top of the in-flight media-transfer/atomic-write extraction, ESM migration, and parallel DASH fetching that this work builds on and gates. ## Data safety - Metadata "repair" of a same-named existing file is now gated on a duration match (+/-3s) and distinguishes unreadable metadata from a missing ISRC tag; on any doubt the file is left untouched (src/main/core/download-manager.ts). - ISRC scans no longer delete files whose tags fail to parse (tidal.ts, qobuz.ts, fs/filemanager.ts) - a parse failure is not proof of corruption. - settings.json is written atomically (temp+fsync+rename, 0600) and validated on load with a {} fallback. - before-quit now aborts in-flight downloads with a bounded 3s drain; startup sweeps orphaned .part/.tmp/.repair-cover temp files from the download folder (new src/main/fs/temp-sweep.ts). ## Stability / performance - Aggregate download progress/speed is derived by summing active queue items instead of last-writer-wins globals under concurrency. - queue:changed pushes carry the queue snapshot; both renderer hooks consume it directly instead of each re-invoking getters per event. - Renderer crash/hang recovery (render-process-gone -> reload) and window bounds persistence with off-screen protection. - Audio analysis kills ffmpeg once the sample cap is reached instead of buffering whole hi-res files in memory. - Amazon gateway polling uses a wall-clock deadline instead of summed intervals. - Progress meters animate transform: scaleX instead of width (no layout work per tick). ## UX - Long artist-discography fetches are cancellable (new CancelSpotifyMetadata channel) with a Cancel button and loading skeleton. - Tidal auth failures show an actionable "Open Settings" toast instead of a generic dead end. - Offline banner via online/offline listeners. - Track-list pagination is windowed with ellipses; artwork lazy-loads with a clean error fallback. - Batch stop marks items "Stopped (partial)", keeps their counters, and resumes on Start. - First-run "Getting started" card; download-path existence validation (new ValidateDirectory channel); Spotify credential Verify button. ## Tests 97 vitest tests pass, including new coverage: repair gating (4 cases), settings round-trip/corruption, temp sweep, atomic-write failure cleanup. oxfmt/oxlint/tsgo/knip clean; dev boot smoke-tested. 
Data safety: duration-gated metadata repair, no delete on tag-parse
failure, atomic settings writes, quit drain + stale temp sweep.
Stability/perf: per-item aggregate progress, snapshot-carrying queue
pushes, renderer crash recovery, window-state persistence, capped PCM
decode, wall-clock Amazon poll budget.
UX: cancellable metadata fetch, actionable Tidal auth errors, offline
banner, windowed pagination, lazy artwork, batch stopped status,
first-run hints, settings validation, scaleX progress meters.
Includes the in-flight media-transfer/atomic-write extraction, ESM
migration, and parallel DASH fetching this work builds on.
kAgent commented 2026-07-19 01:32:19 +02:00
Collaborator
Copy link

kReview review

Verdict: 1 Medium

Medium risk: the packaging change breaks the configured macOS auto-update path by removing its required update artifact.

Medium (1)

  • Preserve the existing release artifact targets — electron-builder.yml
    Packaged macOS users will no longer receive updates because the app still initializes electron-updater, while this change removes the ZIP artifact required to generate latest-mac.yml; electron-builder explicitly documents that disabling ZIP breaks auto-update for DMG packages. ([electron.build]​(https://www.electron.build/docs/features/auto-update/?utm_source=openai)) The same configuration also silently removes the established Windows portable and Linux DEB artifacts despite the PR not describing a distribution-policy change.
    Suggested fix: Restore zip alongside dmg so macOS update metadata is generated, and retain the portable and deb targets unless their removal is an explicit release-policy decision.

Reviewed by kReview at 85ae5c1215. This comment is conservative and based only on the PR diff and metadata.

Est. cost ~$2.51 total (421.4k in / 23.6k out) · this run ~$0.42 (76.9k in / 4.0k out) / gpt-5.6-sol.

<!-- codex-forgejo-review --> <!-- codex-forgejo-review-head:85ae5c1215a16b86e3761fcae66ac7dc6944945e --> ## kReview review **Verdict:** 1 Medium Medium risk: the packaging change breaks the configured macOS auto-update path by removing its required update artifact. ### Medium (1) - **Preserve the existing release artifact targets** — [`electron-builder.yml`](https://git.kleb.sh/kleb/SpotifFLAC/src/commit/85ae5c1215a16b86e3761fcae66ac7dc6944945e/electron-builder.yml) Packaged macOS users will no longer receive updates because the app still initializes `electron-updater`, while this change removes the ZIP artifact required to generate `latest-mac.yml`; electron-builder explicitly documents that disabling ZIP breaks auto-update for DMG packages. ([electron.build]​(https://www.electron.build/docs/features/auto-update/?utm_source=openai)) The same configuration also silently removes the established Windows portable and Linux DEB artifacts despite the PR not describing a distribution-policy change. Suggested fix: Restore `zip` alongside `dmg` so macOS update metadata is generated, and retain the `portable` and `deb` targets unless their removal is an explicit release-policy decision. _Reviewed by kReview at `85ae5c1215`. This comment is conservative and based only on the PR diff and metadata._ _Est. cost ~$2.51 total (421.4k in / 23.6k out) · this run ~$0.42 (76.9k in / 4.0k out) / gpt-5.6-sol._ <!-- codex-forgejo-review-state:eyJoZWFkU2hhIjoiODVhZTVjMTIxNWExNmI4NmUzNzYxZmNhZTY2YWM3ZGM2OTQ0OTQ1ZSIsInN1bW1hcnkiOiJNZWRpdW0gcmlzazogdGhlIHBhY2thZ2luZyBjaGFuZ2UgYnJlYWtzIHRoZSBjb25maWd1cmVkIG1hY09TIGF1dG8tdXBkYXRlIHBhdGggYnkgcmVtb3ZpbmcgaXRzIHJlcXVpcmVkIHVwZGF0ZSBhcnRpZmFjdC4iLCJmaW5kaW5ncyI6W3sic2V2ZXJpdHkiOiJtZWRpdW0iLCJ0aXRsZSI6IlByZXNlcnZlIHRoZSBleGlzdGluZyByZWxlYXNlIGFydGlmYWN0IHRhcmdldHMiLCJmaWxlIjoiZWxlY3Ryb24tYnVpbGRlci55bWwiLCJsaW5lIjpudWxsLCJxdW90ZWRfc25pcHBldCI6Ii0gemlwIn1dLCJjdW11bGF0aXZlQ29zdCI6eyJ1c2QiOjIuNTA3Mjg1OTk5OTk5OTk5NywiaW5wdXRUb2tlbnMiOjQyMTQ0MCwib3V0cHV0VG9rZW5zIjoyMzU4OSwibW9kZWwiOiJncHQtNS42LXNvbCJ9fQ== -->
Sweep only temp names carrying the app's media-filename marker so foreign
.part/.tmp files in a shared download folder survive. Metadata repair now
embeds and verifies on a work copy and renames over the original only
after the ISRC verifies, so a failed repair leaves the file untouched.
Provider temp files now carry a .spotiflac. ownership marker and the
startup sweep only matches it, plus skips anything modified within a 10s
grace window so a download started during the background sweep is never
touched. Amazon status polling re-checks the wall-clock deadline after
sleeping and bounds each request by the remaining budget.
ValidateDirectory now uses fs.promises so a slow or unavailable network
path cannot block the main thread, and the settings input debounces
validation instead of checking on every keystroke.
Atomic-write temp names now carry the .spotiflac. ownership marker and
the sweep requires it, so unmarked foreign temp files are never deleted.
The download-path validator ignores responses for superseded inputs.
kleb merged commit 704d08eb20 into main 2026-07-19 08:19:09 +02:00
kleb deleted branch fix/audit-stability-ux-perf 2026-07-19 08:19:09 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kleb/SpotifFLAC!9
No description provided.