From 005c6ad73aef34c166b7b576a3a103f3599202ac Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Thu, 23 Apr 2026 22:24:46 -0700 Subject: [PATCH] Fix Soulseek handoff routing + stale-request flash on fast retype MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two AI-review findings from Cin (kettui) on the source-picker PR: 1. Soulseek handoff from global widget went through metadata flow _gsNavigateToSearchPage(query, 'soulseek') wrote the query into #enhanced-search-input and dispatched an input event. The Search page controller's activeSource was whatever its default was (spotify, deezer, etc.), so the debounced submitQuery ran the enhanced /api/enhanced-search flow instead of the raw Soulseek file search. The `src` parameter was effectively ignored. Fix: when src === 'soulseek', pre-fill #downloads-search-input directly and click the Search page's Soulseek icon. The icon click triggers the controller's onSoulseekSelected callback, which owns the section swap and re-runs performDownloadsSearch against the value we just wrote to the basic input. 2. Stale in-flight requests cleared loadingSources after fast retype createSearchController._fetchSource awaits the fetch result, then unconditionally mutates state.loadingSources / state.sources in the settle and catch blocks. When a user typed "abc" → fetch started → typed "abcd" before the first fetch returned, the second submitQuery aborted the first fetch and started its own. The first fetch's catch (AbortError) then ran and cleared loadingSources for that source — wiping the spinner the new request had just set, and causing a brief flash of empty/error state while the new fetch was still in flight. Fix: monotonic _requestSeq token. Each _fetchSource call captures the next value (++_requestSeq). Settle / catch blocks (and the YouTube NDJSON streaming loop) bail before mutating shared state if requestId !== _requestSeq. Existing abortCtrl behavior unchanged — this is a layered defense for the catch-clobber pattern that abort alone can't prevent. --- webui/static/downloads.js | 20 +++++++++++++++++--- webui/static/helper.js | 2 ++ webui/static/shared-helpers.js | 26 ++++++++++++++++++++++++-- 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/webui/static/downloads.js b/webui/static/downloads.js index 75e1607d..2ad5aa49 100644 --- a/webui/static/downloads.js +++ b/webui/static/downloads.js @@ -5181,10 +5181,24 @@ function _gsNavigateToSearchPage(query, src) { _gsDeactivate(); if (typeof navigateToPage !== 'function') return; navigateToPage('search'); - // After the page mounts, mirror the query into the enhanced input so the - // user doesn't have to retype it. The Search page's source picker will - // pick up `src` via its own default-source flow. + // After the page mounts, mirror the query into whichever input drives the + // requested source. Soulseek goes through the basic-search file flow, not + // the enhanced metadata flow — without this branch the Search page would + // run /api/enhanced-search instead of /api/search and the user would get + // metadata results when they clicked the Soulseek icon. setTimeout(() => { + if (src === 'soulseek') { + // Pre-fill the basic input first, then click the Search page's + // Soulseek icon. The icon's click triggers the controller's + // onSoulseekSelected callback, which owns the section swap and + // re-runs performDownloadsSearch with whatever's in the basic + // input (i.e., the value we just wrote). + const basicInput = document.getElementById('downloads-search-input'); + if (basicInput && query) basicInput.value = query; + const soulseekIcon = document.querySelector('#enh-source-row [data-source="soulseek"]'); + if (soulseekIcon) soulseekIcon.click(); + return; + } const input = document.getElementById('enhanced-search-input'); if (input && query) { input.value = query; diff --git a/webui/static/helper.js b/webui/static/helper.js index 37d89cf3..7d46ddce 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3458,6 +3458,8 @@ const WHATS_NEW = { { title: 'Fix Clean Search History Automation Failing with AttributeError', desc: 'The hourly Clean Search History maintenance automation was crashing with "DownloadOrchestrator object has no attribute base_url". Root cause: the check `soulseek_client.base_url` was written before the orchestrator refactor — `soulseek_client` is now a DownloadOrchestrator that wraps individual download clients, with the real Soulseek client at `.soulseek`. Two other call sites in web_server.py already used the correct `soulseek_client.soulseek.base_url` pattern; this one was missed. Now matches the same getattr-guarded pattern and the hourly cleanup runs again', page: 'stats' }, { title: 'Search Results Always Visible — Show/Hide Button Removed', desc: 'The "Show Results / Hide Results" toggle next to the search bar is gone. There was nothing else on the page worth seeing instead of results, so toggling visibility never made sense. Cin flagged it during PR review. Dropdown visibility is now a pure function of query state — empty input hides it, results show it', page: 'search', unreleased: true }, { title: 'Cached Search Results Restore on Navigate-Back', desc: 'Previously, navigating away from /search via a sidebar link dismissed the dropdown (the click registered as outside-click). When you came back, the input still held your query but the results were hidden until you typed again or clicked Show Results. Now the per-query cache renders automatically when you re-enter /search, so your results are right where you left them. Cin flagged the round-trip during PR review', page: 'search', unreleased: true }, + { title: 'Fix Soulseek Handoff from Global Search Going Through Metadata Flow', desc: 'When you clicked the Soulseek icon in the sidebar global search popover, it navigated to /search and wrote the query into the enhanced-search input — which then ran the metadata flow against whatever your default source was (Spotify, Deezer, etc.) instead of the raw Soulseek file search you actually wanted. Cin flagged it during PR review. Now the handoff pre-fills the basic-search input directly and clicks the Search page\'s Soulseek icon so the controller\'s onSoulseekSelected callback owns the section swap and runs performDownloadsSearch with the right query', page: 'search', unreleased: true }, + { title: 'Stale Search Requests No Longer Flash Empty Results on Fast Retype', desc: 'Cin flagged a race in createSearchController: when you typed a query then quickly re-typed before the first fetch returned, the first fetch\'s catch block (firing on AbortError after the second submitQuery aborted it) cleared loadingSources and notified the UI, causing a brief flash of empty/error state while the new query\'s fetch was still mid-flight. Added a monotonic _requestSeq token — each fetch captures the next value, and stale completions bail before mutating shared state. The controller still aborts in-flight fetches on supersession; this just keeps the abort-cleanup of the old request from clobbering the new one\'s spinner', page: 'search', unreleased: true }, ], '2.39': [ // --- April 22, 2026 --- diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index 80ac7ecf..ece7efca 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -172,6 +172,13 @@ function createSearchController({ for (const src of SOURCE_ORDER) state.configuredSources[src] = true; let abortCtrl = null; + // Monotonic request token. Each _fetchSource call captures the next + // value; settle/error blocks bail before mutating shared state if a + // newer request has superseded them. Without this, a fast retype lets + // the in-flight fetch's catch (or settle) clear loadingSources / write + // stale data into state.sources, causing a flash of empty/error UI + // while the new query's fetch is still running. + let _requestSeq = 0; function _notify() { if (onStateChange) onStateChange(state); } @@ -305,6 +312,8 @@ function createSearchController({ const query = state.query; if (!query) return; + const requestId = ++_requestSeq; + state.loadingSources.add(src); renderSourceRow(); _notify(); @@ -314,12 +323,14 @@ function createSearchController({ try { if (src === 'youtube_videos') { - await _fetchYouTubeVideos(query, abortCtrl.signal); + await _fetchYouTubeVideos(query, abortCtrl.signal, requestId); } else { const data = await enhancedSearchFetch(query, { source: src, signal: abortCtrl.signal, }); + // Bail without writing if a newer query has superseded us. + if (requestId !== _requestSeq) return; state.sources[src] = { artists: data.spotify_artists || [], albums: data.spotify_albums || [], @@ -331,10 +342,15 @@ function createSearchController({ if (served && served !== src) state.fallbacks[src] = served; } + // Only the latest request gets to clear loadingSources + notify. + // A stale completion would otherwise wipe the spinner the new + // request just set. + if (requestId !== _requestSeq) return; state.loadingSources.delete(src); renderSourceRow(); _notify(); } catch (err) { + if (requestId !== _requestSeq) return; state.loadingSources.delete(src); renderSourceRow(); _notify(); @@ -344,7 +360,7 @@ function createSearchController({ } } - async function _fetchYouTubeVideos(query, signal) { + async function _fetchYouTubeVideos(query, signal, requestId) { const res = await fetch('/api/enhanced-search/source/youtube_videos', { method: 'POST', headers: { 'Content-Type': 'application/json' }, @@ -353,6 +369,9 @@ function createSearchController({ }); if (!res.ok) throw new Error(`YouTube search failed: ${res.status}`); + // Bail before allocating cache entry if superseded by a newer request. + if (requestId !== _requestSeq) return; + state.sources['youtube_videos'] = { artists: [], albums: [], tracks: [], videos: [], db_artists: [], }; @@ -364,6 +383,9 @@ function createSearchController({ while (true) { const { done, value } = await reader.read(); if (done) break; + // Mid-stream supersession check — abort cleanly without writing + // additional chunks into stale cache. + if (requestId !== _requestSeq) return; buffer += decoder.decode(value, { stream: true }); let idx; while ((idx = buffer.indexOf('\n')) !== -1) {