Architectural & perf rewrites: push IPC, React Compiler, virtualization, worker pool

kleb commented

2026-06-24 00:53:56 +02:00

Owner

Larger architectural/perf rewrites, stacked on #3 (audit-fixes). 7 commits;
all typecheck + lint + test (22) + build green, and the renderer was
boot-validated after each change via the SPOTIFLAC_CAPTURE dev hook.

Changes

Push-based queue/progress — coalesced main→renderer queue:changed push from
the single bumpQueueVersion() chokepoint, a preload onQueueChanged
subscription, and hooks that refetch on event. Additive: the poll stays as a
backstop, so behavior can't regress below today even if a push is missed.
Visibility-gated polling — the 200ms loop drops to 2s while the window is
hidden (was burning CPU minimized with backgroundThrottling off).
Relaxed active poll to 1s — push now drives immediacy, so the fast poll is
just a backstop.
React Compiler enabled — auto-memoizes the inline callbacks/objects across
App/MetadataView/TrackList.
Download-queue virtualization — @tanstack/react-virtual (dynamic measure),
pairs with the earlier React.memo.
Cache schema versioning — PRAGMA user_version-gated migration runner; fixes
a real bug where the album/artist UPDATE ... expiry_days = 90 WHERE = 30/14 ran
every launch and silently reset a user-chosen 30/14. Unit-tested.
FFT worker pooled — one long-lived spectrum worker instead of spawn/terminate
per track; any worker error falls back to inline computation (correctness-safe).

NEEDS LIVE VALIDATION (Electron)

These change runtime/dynamic behavior that a static screenshot can't confirm:

A real download → push updates the queue/progress; the virtualized list
scrolls/measures with many items.
Minimize mid-download → progress still updates (slower), CPU drops.
Click-through after React Compiler → no interaction regressions.
Run an audio analysis → pooled worker still produces correct spectra.

Intentionally NOT included (need a paired/eyes-on loop)

Download state-machine rewrite — the state module is the most race-fixed file
in history; a blind rewrite risks reintroducing those races, only catchable with
live concurrent download + pause/stop/cancel testing.
Full type-contract / Wails-model collapse — large mechanical refactor better
reviewed with eyes on the diff (classes are still instantiated via new X({...})).
Partial-file .part restructure / double-ISRC-rescan removal / request
coalescing — download-path changes with subtle cancellation/dedup semantics that
need live validation for marginal benefit.

Larger architectural/perf rewrites, stacked on #3 (`audit-fixes`). 7 commits; all `typecheck` + `lint` + `test` (22) + `build` green, and the renderer was boot-validated after each change via the `SPOTIFLAC_CAPTURE` dev hook. ## Changes - **Push-based queue/progress** — coalesced main→renderer `queue:changed` push from the single `bumpQueueVersion()` chokepoint, a preload `onQueueChanged` subscription, and hooks that refetch on event. Additive: the poll stays as a backstop, so behavior can't regress below today even if a push is missed. - **Visibility-gated polling** — the 200ms loop drops to 2s while the window is hidden (was burning CPU minimized with `backgroundThrottling` off). - **Relaxed active poll to 1s** — push now drives immediacy, so the fast poll is just a backstop. - **React Compiler enabled** — auto-memoizes the inline callbacks/objects across App/MetadataView/TrackList. - **Download-queue virtualization** — `@tanstack/react-virtual` (dynamic measure), pairs with the earlier `React.memo`. - **Cache schema versioning** — `PRAGMA user_version`-gated migration runner; fixes a real bug where the album/artist `UPDATE ... expiry_days = 90 WHERE = 30/14` ran every launch and silently reset a user-chosen 30/14. Unit-tested. - **FFT worker pooled** — one long-lived spectrum worker instead of spawn/terminate per track; any worker error falls back to inline computation (correctness-safe). ## NEEDS LIVE VALIDATION (Electron) These change runtime/dynamic behavior that a static screenshot can't confirm: - A real download → push updates the queue/progress; the virtualized list scrolls/measures with many items. - Minimize mid-download → progress still updates (slower), CPU drops. - Click-through after React Compiler → no interaction regressions. - Run an audio analysis → pooled worker still produces correct spectra. ## Intentionally NOT included (need a paired/eyes-on loop) - **Download state-machine rewrite** — the state module is the most race-fixed file in history; a blind rewrite risks reintroducing those races, only catchable with live concurrent download + pause/stop/cancel testing. - **Full type-contract / Wails-model collapse** — large mechanical refactor better reviewed with eyes on the diff (classes are still instantiated via `new X({...})`). - **Partial-file `.part` restructure / double-ISRC-rescan removal / request coalescing** — download-path changes with subtle cancellation/dedup semantics that need live validation for marginal benefit.

kleb added 7 commits

2026-06-24 00:53:56 +02:00

perf(renderer): slow polling when the window is hidden 25d9f9ab7b

The progress/queue hooks polled at 200ms during downloads with
backgroundThrottling disabled, so a minimized app mid-batch kept hammering
IPC and pinning a core. Drop to a 2s cadence while document.hidden and
re-evaluate on visibilitychange. Visible behavior is unchanged.

feat(arch): push queue/progress changes to the renderer 29a48980ad

Add a coalesced main->renderer 'queue:changed' push from the single
bumpQueueVersion() chokepoint (throttled ~150ms), a preload onQueueChanged
subscription, and renderer hooks that refetch immediately on the event. This
is additive: the existing poll stays as a correctness backstop, so behavior
cannot regress below today even if a push is missed. Lays the groundwork for
removing the fast renderer poll once validated against a live download.

NEEDS LIVE VALIDATION: the dynamic push path (updates during an actual
download) can only be confirmed by running a real download.

perf(renderer): enable React Compiler 8f676a97ca

Auto-memoizes components/values (React 19 target) so the many inline
callbacks/objects across App/MetadataView/TrackList stop causing needless
re-renders without hand-written useMemo/useCallback. Build is clean and the
home page renders pixel-identical; broader interactive validation is best done
in the running app.

perf(renderer): virtualize the download queue list b6d0ed8715

Render only the visible queue rows via @tanstack/react-virtual (dynamic
measurement, since QueueItem height varies). Pairs with the earlier
React.memo so large queues no longer mount or reconcile every row.

NEEDS DATA VALIDATION: empty-state path is unchanged and the app boots, but
the virtualized layout (scroll/measurement) can only be confirmed with a
populated queue in the running app.

perf(renderer): relax active poll to 1s now that push drives updates 2ea8411acd

With the queue:changed push in place the renderer no longer needs the 200ms
fast poll for real-time progress; relax it to a 1s backstop. Cuts active-
download IPC ~5x while push keeps updates sub-second.

fix(cache): version-gate one-time migrations (PRAGMA user_version) ade7517d0a

The album/artist schema ran 'UPDATE ... SET expiry_days = 90 WHERE
expiry_days = 30/14' on EVERY open, silently resetting a user who
deliberately chose 30 or 14. Add a user_version-gated migration runner to
BaseCache and move those one-off updates (and the album tracks-column ALTER)
into it so they apply exactly once. Tolerates already-applied steps on
pre-existing unversioned DBs. Unit-tested.

perf(audio): pool the FFT spectrum worker

CI / check (pull_request) Waiting to run

Details

791c3543ab

The spectrum Worker was created and terminated per track (a 50-track batch
analysis spawned and destroyed 50 workers). Reuse a single long-lived worker
with serialized jobs; any worker error drops it and falls back to inline
computation, so pooling can only cost speed, never correctness. Also memoize
the worker-path probe.

kAgent commented

2026-06-24 00:53:57 +02:00

Collaborator

kReview review

Verdict: 2 Medium

Overall risk is medium; the most important issue is that the PR still removes the automated CI gate for pull requests.

Medium (2)

Restore the pull request CI workflow — .github/workflows/ci.yml
Pull requests will no longer run the repository's typecheck, lint, test, or placeholder-owner guard automatically. The diff deletes the workflow that was triggered by pull_request:, so regressions in this PR's changed runtime paths can merge without the existing quality gate.
Suggested fix: Restore .github/workflows/ci.yml or replace it with an equivalent workflow that runs the same checks on pull requests.
Restore the release workflow — .github/workflows/release.yml
Version tags and manual releases will no longer build or publish installers through GitHub Actions. The release workflow containing the v* tag trigger and packaging job is deleted, so the existing release path disappears without a replacement in the diff.
Suggested fix: Restore .github/workflows/release.yml or add an equivalent release workflow that preserves tag/manual publishing and the existing pre-release checks.

Excluded as generated or vendored (not reviewed): bun.lock.

Reviewed by kReview at 39cfc3ebcf. This comment is conservative and based only on the PR diff, metadata, and supplied repository context.

Est. cost ~$2.40 total (346.2k in / 24.3k out) · this run ~$0.39 (68.3k in / 1.8k out) / gpt-5.5.

## kReview review **Verdict:** 2 Medium Overall risk is medium; the most important issue is that the PR still removes the automated CI gate for pull requests. ### Medium (2) - **Restore the pull request CI workflow** — [`.github/workflows/ci.yml`](http://git.kleb.sh/kleb/SpotifFLAC/src/commit/39cfc3ebcf4b12955b16cd316b030786d15b5577/.github/workflows/ci.yml) Pull requests will no longer run the repository's typecheck, lint, test, or placeholder-owner guard automatically. The diff deletes the workflow that was triggered by `pull_request:`, so regressions in this PR's changed runtime paths can merge without the existing quality gate. Suggested fix: Restore `.github/workflows/ci.yml` or replace it with an equivalent workflow that runs the same checks on pull requests. - **Restore the release workflow** — [`.github/workflows/release.yml`](http://git.kleb.sh/kleb/SpotifFLAC/src/commit/39cfc3ebcf4b12955b16cd316b030786d15b5577/.github/workflows/release.yml) Version tags and manual releases will no longer build or publish installers through GitHub Actions. The release workflow containing the `v*` tag trigger and packaging job is deleted, so the existing release path disappears without a replacement in the diff. Suggested fix: Restore `.github/workflows/release.yml` or add an equivalent release workflow that preserves tag/manual publishing and the existing pre-release checks. _Excluded as generated or vendored (not reviewed):_ `bun.lock`. _Reviewed by kReview at `39cfc3ebcf`. This comment is conservative and based only on the PR diff, metadata, and supplied repository context._ _Est. cost ~$2.40 total (346.2k in / 24.3k out) · this run ~$0.39 (68.3k in / 1.8k out) / gpt-5.5._

kleb added 1 commit

2026-06-24 00:55:44 +02:00

fix(audio): don't transfer sample buffer to the spectrum worker

CI / check (pull_request) Waiting to run

Details

1ea7c7bbb4

Address PR review: postMessage transferred samples.buffer, detaching it on
the main thread, so the onError/failed-message inline fallback would compute
on an empty buffer. Drop the transfer list (structured clone copies to the
worker) so the fallback keeps a usable buffer — restores the correctness-safe
guarantee the pooling relies on.

kleb added 3 commits

2026-06-24 01:14:52 +02:00

refactor(arch): encapsulate download state in a DownloadSession 2908097ecb

Replace the scattered module-global flags (isDownloading/activeDownloadCount/
paused/stopped/stopController/currentItemID/queue) with a single
DownloadSession class exposing an explicit lifecycle phase
(idle/running/paused/stopping) derived from a concurrency refcount + the
flags, and centralize the stop-controller recreation in ONE place
(ensureFreshController) instead of three duplicated snippets. Logic is ported
verbatim; the exported functions are thin delegates so every caller is
unchanged. Adds 8 lifecycle unit tests incl. the invariant that a prior stop
cannot poison a fresh download's abort signal.

NEEDS LIVE VALIDATION: concurrent downloads with pause/stop/cancel/skip.

refactor(types): collapse Wails model classes to plain interfaces de491723da

The backend/main model namespaces were ~1140 lines of generated Wails class
scaffolding (createFrom/constructor/convertValues) that was dead at runtime:
IPC returns structured-cloned JSON, so the renderer already treated these as
plain data, and createFrom/convertValues had zero call sites. Convert both
namespaces to plain interfaces, switch the 13 importers to 'import type'
(verbatimModuleSyntax), and replace the 5 'new X({...})' sites with object
literals. Net ~760 lines removed; typecheck/lint/tests/build all green.

fix(download): write to .part and rename into place atomically

CI / check (pull_request) Waiting to run

Details

819e154cc5

The Qobuz/Tidal-direct/Amazon downloaders streamed straight to the final
path and removed the partial only in a catch, so an abnormal termination
(crash/power loss) left a truncated final file that could be mistaken for a
complete download. Stream to <file>.part and rename on success (fs.rename
replaces an existing dest on POSIX and Windows), matching the Tidal manifest
paths that already used a temp. A crash now leaves a harmless .part that a
retry overwrites.

kleb added 2 commits

2026-06-24 06:19:25 +02:00

perf(download): skip the provider's redundant ISRC rescan f832e5a6bb

downloadTrack already scans the output dir for a matching-ISRC file (and the
per-ISRC TOCTOU guard prevents one appearing meanwhile), yet the Qobuz/Tidal
providers re-ran the same full-directory tag scan — 2*O(files) tag reads per
download, trending O(n^2) over a large single-folder batch. Pass
skipExistenceCheck from the manager so the provider does it at most once.

perf(songlink): single-flight + negative-cache URL resolution

CI / check (pull_request) Has been cancelled

Details

084004fdb7

Two related improvements to getAllURLsFromSpotify: (1) collapse concurrent
resolutions of the same track id into one request via an in-flight map (the
common single-caller case keeps the caller's signal, so cancellation is
unchanged); (2) negative-cache a 'no Tidal/Amazon link' result (the same
both-empty row the availability path already writes) so repeat lookups and
the prefetcher stop re-resolving unavailable tracks — fixing the prefetcher's
non-convergence. Cache-read behavior is unchanged.

kleb added 1 commit

2026-06-24 06:28:21 +02:00

ci: remove GitHub/Forgejo Actions workflows 4f5f48d57c

Remove .github/workflows/ci.yml and release.yml at the maintainer's request.
(Dropping these also removes the CI typecheck/lint/test gate and the
PLACEHOLDER_OWNER guard that release.yml/ci.yml carried.)

kleb added 1 commit

2026-06-24 06:30:54 +02:00

fix(songlink): decouple single-flight from any one caller's signal 71834f7a4b

Address PR review: the shared in-flight resolution was created with the first
caller's AbortSignal, so that caller cancelling rejected the shared work for
other still-active callers. Run the shared resolution signal-free (it must
complete and cache for everyone) and apply each caller's cancellation
independently via awaitWithSignal — a cancelled caller rejects with
StoppedError without disturbing the shared work or other awaiters.

kleb added 1 commit

2026-06-24 06:33:45 +02:00

fix(songlink): throw on negative cache hit for consistency 39cfc3ebcf

Address PR review: a both-empty (negative) cache row was returned as empty
URLs on a cache hit while the network path throws 'no streaming URLs found'
for the same case. Treat a both-empty cached entry as a negative hit and throw
the same error, so a not-found track is signalled consistently. (GetStreamingURLs
has no renderer caller; the prefetcher catches the throw and now converges.)

kleb closed this pull request

2026-06-24 08:41:56 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Rows
Columns

Architectural & perf rewrites: push IPC, React Compiler, virtualization, worker pool #4

Changes

NEEDS LIVE VALIDATION (Electron)

Intentionally NOT included (need a paired/eyes-on loop)

kReview review

Medium (2)

Pull request closed