Fix Soulseek handoff routing + stale-request flash on fast retype

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.
This commit is contained in:
Broque Thomas 2026-04-23 22:24:46 -07:00
parent ab7aeb302c
commit 005c6ad73a
3 changed files with 43 additions and 5 deletions

View file

@ -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;

View file

@ -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 ---

View file

@ -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) {