Search and audition from the topbar, and three bugs found on the way - #445
Merged
Conversation
added 11 commits
August 25, 2026 14:03
Reported against a real import:
'charmap' codec can't decode byte 0x8f in position 20
text=True on its own decodes a child's output with the locale encoding. On
Windows that is cp1252, so one byte outside it in Demucs' progress output
killed the whole job. The output in question is a progress bar and some
echoed metadata. Diagnostic text, never worth failing a separation over.
Both halves have to agree or the mismatch just moves, so the parents now
read utf-8 with errors=replace and the children are told to write utf-8.
Three call sites: the Demucs worker, the vocal-split worker, and ffprobe on
upload, where the same trap would have failed an upload rather than a
separation.
Pre-existing and unrelated to any feature work. It surfaced now only because
error_detail started carrying the message (#434) instead of the bare word
'unknown'.
The box already took a pasted link. Typing anything that is not a link now searches instead, so finding a track no longer means leaving StemDeck, finding it in a browser, and coming back with a URL. Three tabs: YouTube songs, YouTube playlists, SoundCloud songs. SoundCloud playlists is deliberately absent, because yt-dlp exposes exactly one SoundCloud search key (scsearch, tracks only) and a tab that can only ever be empty is worse than no tab. Cost, measured rather than assumed. A search is one flat extraction, about 1.1 s for YouTube and 2.0 s for SoundCloud, and it returns titles, durations, uploaders and thumbnails for the whole page at once. Requests fire on a word boundary rather than a keystroke, so typing a full phrase costs one request, not one per character. An AbortController cancels the superseded request so results cannot land out of order. A 60 s server cache absorbs the repeats that backspacing produces. A semaphore caps concurrent yt-dlp searches, because an aborted fetch does not stop a thread that has already started. The duration limit is part of the cache key, not just the payload. It decides each result's too_long verdict and it is a live setting: raising it has to un-grey the rows now, not once a 60 s entry expires. Over that limit the pipeline refuses the job outright, so those rows are not selectable and say so. This reads the user's configured value, which is anywhere from 1 to 60 minutes, not a hardcoded 20. The SSRF boundary from #173 is unchanged. Each search gets the narrowest extractor allowlist that can serve it, generic stays out of all of them, and every result goes back through validate_youtube_url or validate_playlist_url before it can reach the pipeline. Anything that fails is dropped rather than shown. SoundCloud needs webpage_url rather than url for this: its search returns an api.soundcloud.com endpoint that is not on the allowlisted host set, so reading url first (as expand_playlist does) drops every result. Picking a result fills the box and stops there. Extraction is minutes of work, so it stays behind a deliberate press of Split stems rather than starting on a click in a list the user may still be reading. The panel lives on body rather than in the composer, which sets overflow:hidden for its rounded pill and clipped the dropdown out of existence. The topbar input is type=text now, since type=url marks a search query invalid and blocks submission; its styling keys off #url rather than the type so that cannot silently unstyle it again.
Each result carries a Preview button that expands into play, pause and a seekable time bar, so a wrong take is found in ten seconds rather than after a separation. It is labelled rather than a bare glyph on purpose: an unlabelled circle on a search result reads as 'play this result', which is the one thing it does not do. Auditioning is not selecting. The stream is proxied rather than handed to the page. The CSP allows media-src 'self' blob: data:, so a googlevideo.com URL in an audio tag is blocked, and widening that would let any injected string in the webview pull media from anywhere. The browser never talks to YouTube: it talks to us, and we fetch a URL yt-dlp resolved from an already-allowlisted page. Kept cheap. The smallest audio-only progressive format is chosen, which is 1.2 MB for a four minute track against 11.8 MB for the 360p video. Resolving is the expensive half at about two seconds, and the result is stable for hours, so it is cached for twenty minutes. Range is forwarded, so seeking is a partial request rather than a re-download. One audio element is shared by the whole panel with preload=none, and its src is dropped on close: a paused element keeps its connection, which for a proxied stream means holding a socket open for a preview nobody is listening to. HLS and video formats are refused rather than served and left to fail in the player. A manifest rewrite and a segment proxy is a lot of machinery for an audition button. The frontend half of this landed with the previous commit, since it shares static/js/search.js with the search UI.
pt-PT is a regional variant rather than a ninth full table. It overrides the 43 strings that genuinely differ from Brazilian Portuguese (ficheiro not arquivo, Definições not Configurações, Guardar not Salvar, Transferir not Baixar, and the gerunds European Portuguese renders as 'a' plus infinitive) and resolves through pt for the other 400. t() and plural() now walk a fallback chain, variant to base to English, instead of jumping straight to English on a miss. Duplicating all 439 keys would have meant writing every future string twice and drifting the first time one was missed. A key added to pt later is picked up by pt-PT for free. Locale detection sends an explicit Portugal tag to the variant; pt-BR and a bare 'pt' still take the Brazilian table, which is the more widely used variant and so the right default for an unqualified tag. Separately, the over-limit badge is now 'Over 20 min' rather than 'Over 20 min limit', with the actionable half moved to a tooltip: 'Longer than your 20 minute limit. Change it in Settings.' The row shares 620px with a title, an uploader, a duration and the Preview pill, and the title is what people scan.
The first pass scanned a hand-written list of nouns and caught 43 strings. A systematic scan found 41 more, so the variant now carries 84 overrides. The biggest omission was the progressive aspect. Brazilian Portuguese uses the gerund, European uses 'a' plus the infinitive, and that is every status line in the app: Processando, Pesquisando, Exportando, Sincronizando, Salvando, Iniciando all read as Brazilian to someone in Portugal. Those are now A processar, A pesquisar, A exportar, A sincronizar, A guardar, A iniciar. Also covered: - Registro to Registo - aplicativo to aplicação - conexão to ligação - somente leitura to só de leitura - detectar to detetar, where the 1990 orthographic agreement dropped a consonant that is silent in Portugal and kept it in Brazil - em plus article contracting to num/numa - salvá-la to guardá-la The four metronome notes are overridden together even though only two were flagged, because rewording half of a set that reads as one paragraph is worse than leaving all four alone. Two matches were left deliberately. 'Log de configuração' stays: the scan flagged it on 'configuraç', but that pattern exists to catch Configurações meaning Settings, and configuração meaning configuration is correct in both variants.
Reported as the search badge still saying 'Over 20 min' after setting the
limit to 60. The badge was right. The setting never changed.
Three places disagreed about the ceiling:
backend _DURATION_MAX = 3600 60 minutes
client Math.min(20, ...) 20 minutes
label "(max 20)" 20 minutes
So typing 60 hit the client clamp, posted 1200, and the field snapped back to
20 with nothing said. The backend had allowed 60 all along; only the client
refused to send it.
Fixed by removing the second copy rather than correcting it. /api/settings
now publishes max_duration_min_sec and max_duration_max_sec, the client sends
what was typed and lets the server clamp, and apply() writes back whatever
the server kept. The description text takes {max} from the same source
instead of a hardcoded number, in all eight tables.
That is the actual defect here. A duplicated constant does not announce
itself when it goes stale: nothing failed, no test broke, and the only
symptom was a number quietly refusing to change.
There was no written account of where the project has been. Fourteen minor versions in under four months, and the only record was a tag list and 192 PR titles, which tells you what changed and never why. Dates are the first tag in each line, read from the repository rather than reconstructed. Each release gets what it was actually for, derived from the PRs merged in its window, not a changelog restatement. Two threads are worth being able to point at later. The 0.7.0 security pass (XSS, SSRF, the webview CSP, a pinned FFmpeg checksum) still constrains every feature added since: search had to fit inside that extractor allowlist and preview had to be proxied because of that CSP. And 0.8.0's four-phase health report is the origin of project 2, so the board and the release history now reference each other instead of being separate stories. Forward-looking sections point at open issues rather than restating them, so this does not become a second place to keep the same facts current.
Bandit B310 on the proxy's urlopen, and it was pointing at something real. _pick_format reads yt-dlp's `protocol` field, which is metadata about the format rather than a guarantee about the URL string. Nothing checked the scheme of the thing actually handed to urlopen, so a file:// or a custom scheme reaching it would have been read off the host's disk and streamed straight to the client. The proxy is the one place in the app that opens an arbitrary host, which is exactly where that matters. Checked twice on purpose, the same shape as the path-traversal checks in api/stems.py: refused in resolve() where the URL is chosen, and again at the call that opens the socket. The nosec is added only after those checks exist. Suppressing B310 on its own would have hidden a real hole rather than documented a safe one, and three tests now assert the refusal so the suppression cannot quietly become untrue.
Two dead links in a file whose whole job is orientation. The repository is public and project 2 is private, so both links to the board returned 404 for everyone except the people who already knew what was on it. The board is still where the work is tracked; it is just not something a public document can point at while it stays private. Making it public is a visibility decision, not a formatting one, so the links are gone rather than the board's setting changed. The versioning link pointed at .claude/rules/versioning.md, which is gitignored. It resolved locally, which is why it looked fine when written, and 404'd for every reader. Replaced with the content itself: the version comes from the tag, promotion is manual and is what the updater watches, and desktop and Docker split CPU/GPU differently on purpose. Every remaining link now resolves anonymously, which is the only test that matters for a public file.
550 lines of new frontend with nothing holding it. The three things these protect were all found by hand during review and would all come back silently. **The request count.** Firing per keystroke instead of per word is not a visible bug, it is twenty six requests where one would do, and nobody would notice until a rate limit did. **The dropdown being visible.** It rendered correctly and was clipped out of existence by an ancestor's overflow:hidden. My own throwaway check counted rows in the DOM and passed while the user saw nothing, which is exactly how that shipped to a screenshot. These assert toBeVisible and check the panel's box against the composer's. **Picking not importing.** Extraction is minutes of work, so a click in a list the user may still be reading must not start one. Asserted by blocking POST /api/jobs and checking nothing was attempted. Also covered: a pasted link still bypasses search entirely, the tab switch that used to close the panel (the panel is a sibling of the composer, so a mousedown on a tab counted as an outside click), over-limit rows being unselectable and reading from the setting rather than a hardcoded 20 (#443), Escape, arrow-key selection with aria-activedescendant, and preview being an audition rather than a selection. Both /api/search and /api/search/preview are stubbed. A suite that reaches YouTube fails on a flagged CI IP for reasons that have nothing to do with the change under test. Two of these needed expect.poll rather than a single read. Writing them against a real browser turned up the same class of race twice: the UI settles before the network does, so reading a request count the instant a row appears is testing the wrong moment.
Deliberate change of tracking rather than a version bump. edge is the rolling image published on every merge to main, so Unraid installs now follow main instead of the last promoted release. That means Unraid users see changes before a release is tagged and verified, which is the trade being made on purpose here. It also decouples the template from the release cadence: no per-release pin commit, and no window where Unraid silently lags behind.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #440
Closes #441
Closes #442
Closes #443
Closes #444
Search started this. The three bugs were found while building it, and two of them predate it.
Search (#441)
The topbar took a pasted link, which meant finding something to work on was: leave StemDeck, open a browser, search, copy, come back, paste. It now takes a query too.
Three tabs: YouTube songs, YouTube playlists, SoundCloud songs. SoundCloud playlists is deliberately absent, because yt-dlp exposes exactly one SoundCloud search key (
scsearch, tracks only) and a tab that can only ever be empty is worse than no tab.Cost, measured rather than assumed. One flat extraction returns titles, durations, uploaders and thumbnails for a whole page at once: about 1.1 s for YouTube, 2.0 s for SoundCloud. Requests fire on a word boundary rather than a keystroke, so a full phrase costs one request, not one per character (verified: 29 characters typed quickly produced one request). An AbortController cancels the superseded request so results cannot land out of order. A 60 s server cache absorbs the repeats backspacing produces, and a semaphore caps concurrent yt-dlp searches, because aborting a fetch does not stop a thread that has already started.
The #173 boundary is unchanged. Each search gets the narrowest extractor allowlist that can serve it,
genericstays out of all of them, and every result goes back throughvalidate_youtube_urlorvalidate_playlist_urlbefore it can reach the pipeline. Anything that fails is dropped rather than shown. SoundCloud needswebpage_urlrather thanurlfor this: its search returns anapi.soundcloud.comendpoint that is not on the allowlisted host set, so readingurlfirst (asexpand_playlistdoes) silently drops every SoundCloud result.Picking a result fills the box and stops there. Extraction is minutes of work, so it stays behind a deliberate press of Split stems rather than starting on a click in a list the user may still be reading.
Preview (#442)
A title, a channel and a duration often will not tell a live take from a studio one. Getting it wrong costs a full separation; hearing ten seconds answers it.
The stream is proxied rather than handed to the page.
media-src 'self' blob: data:blocks a googlevideo.com URL in an<audio>tag, and widening it would let any injected string in the webview pull media from anywhere. That CSP exists for #171 and is not worth trading for a convenience button. The browser never talks to YouTube.Kept cheap: the smallest audio-only stream is 1.2 MB for a four minute track against 11.8 MB for the 360p video; resolving is cached for twenty minutes because it is the expensive half;
Rangeis forwarded so seeking is a partial request; and the<audio>src is dropped on close, because a paused element keeps its connection and for a proxied stream that means holding a backend socket for a preview nobody is listening to.Video preview was considered and rejected. A progressive video+audio format is only reachable with the JS solver held in #438, and it is ten times the bytes for a question the audio already answers.
The encoding crash (#440)
Reported from a real import:
Three subprocess calls used
text=Truewith noencoding=, which decodes with the Windows locale encoding, cp1252. One byte outside it in Demucs' progress output killed the whole job. Fixed at both ends, because fixing one just moves it: the parents read utf-8 witherrors="replace", and the children getPYTHONIOENCODING.Pre-existing and unrelated to search. It surfaced now only because #434 made
error_detailcarry the real message instead of the bare wordunknown.The duration ceiling (#443)
Noticed as search still reporting "Over 20 min" after the limit was set to 60. The badge was right; the setting had never changed. The backend allowed 3600s, the client clamped to 20 minutes, and the label agreed with the client.
Fixed by removing the duplicate rather than correcting it.
/api/settingspublishes the bounds, the client sends what was typed and lets the server clamp, and the description takes{max}from the same source instead of a hardcoded number in eight languages.That is the actual defect: a duplicated constant does not announce itself when it goes stale. Nothing failed, no test broke, and the only symptom was a number quietly refusing to change.
European Portuguese (#444)
pt-PTmapped to the Brazilian table. The widest difference is verb aspect, which is every status line in the app: Processando, Pesquisando, Exportando, Sincronizando all read as foreign in Portugal.Added as a regional variant, not a ninth table. It overrides the 84 strings that genuinely differ and resolves through
ptfor the other 400, via a fallback chaint()andplural()both walk. Duplicating all 439 keys would have meant writing every future string twice and drifting the first time one was missed.The parity check in
.claude/rules/i18n.mdwas updated with it: comparing a deliberately partial variant againstenreports several hundred false gaps, which is exactly the "check nobody reads" failure that section already warns about for plural families.Roadmap
ROADMAP.mdis new. Fourteen minor versions in under four months and no written account of any of it. Dates are read from the tags; each release gets what it was actually for, derived from the PRs merged in its window.Project 2 has been updated alongside it.
Verification
Beyond unit tests, the search and preview paths were driven in a real browser:
705 passing. The 14 failures on this branch also fail on
mainin this environment (ffmpeg not on PATH for beatgrid, the Linux executable bit, a CRLF assertion in the logs zip test).ruff checkandruff format --checkclean. i18n parity clean.Note for review
No dependency change, so
uv.lockand the desktopruntimeIdare untouched and existing installs can take this as an in-app update.#urlwastype="url", which marks a search query invalid and blocks form submission. It istype="text"now, and the topbar styling keys off#urlrather than the type so it cannot silently unstyle again.