Name a song's sections automatically, when the user asks for it - #486
Merged
Conversation
added 5 commits
August 28, 2026 22:11
Adds automatic functional song-structure analysis on top of All-In-One (mir-aidj/all-in-one). After separation, an isolated worker maps StemDeck's six stems into the model's four-stem layout, runs inference, and normalizes the result into the existing editable Sections schema. Every field of the model's output is treated as untrusted. The stage is non-fatal by construction: a failure, a timeout, a stall, or a missing model leaves an otherwise usable separation exactly as it was. Cancellation stays authoritative so a long CPU pass can be stopped. Off by default -------------- Structure extraction costs a CPU inference pass per job and its labels are suggestions rather than ground truth, so nobody pays for it without asking. A "Song structure" toggle sits beside Split stems and writes to the server, not to the browser, so two devices pointed at one StemDeck cannot disagree about whether the next import pays for it. The runner reads the setting once per job, so a change applies to the next import without a restart. STEMDECK_AUTO_SECTIONS=1 turns it on for a deployment. Four defects found by evaluating nine real tracks ------------------------------------------------- Inference was run once per track and cached, so before and after were compared on byte-identical model output. Two tracks produced no sections at all and now work; none regressed. Wish You Were Here beat confidence 60 0 -> 12 sections Drum For Your Life 64 12 -> 14 Defying Gravity 30 0 -> 21 Come As You Are 89 13 -> 13 five others unchanged 1. A span past the analyzed duration discarded the whole song. duration_sec is rounded while the model reads the stems, so its timeline overhangs by a fraction of a second. One track ended with a 10 ms span beyond it, which clamped to zero length and was read as malformed. Malformed input is still rejected; a span with no overlap left to keep is now skipped. 2. start and end are ordinary label classes, not only brackets. One track was labelled start for 34 seconds in its middle and end for 32 seconds. Rejecting the song over a misplaced sentinel threw away every section. Sentinels are now stripped only at the extremes of the timeline, and an interior one becomes the neutral part it always meant. 3. Boundary discovery was gated on beat-grid confidence, so refinement did nothing at all below the trust threshold. That is precisely where upstream spans most need splitting: rubato, live and free-time material is the hardest to track rhythmically. Discovery now always runs, and the grid only aligns candidates that evidence has already accepted. 4. Section analysis could not run on Windows at all. The Hugging Face cache populates itself with symlinks, which an unelevated process cannot create, and the resulting WinError 1314 is an OSError rather than the PermissionError the hub falls back on, so the download crashed instead of copying. Zero of four tracks fetched the model before the fix and six of six after. Both entry points that download it now set HF_HUB_DISABLE_SYMLINKS. Adjacent sections that share a kind are numbered ------------------------------------------------ The model predicts boundaries and labels with separate heads, so two neighbouring Verse spans are a real predicted boundary between verse one and verse two rather than a duplicate. An earlier revision merged them and silently discarded five true boundaries on Come As You Are. The boundary is kept and the labels read Chorus 1 and Chorus 2 instead, which keeps the data and stops the timeline looking broken. A kind used once is never numbered, and a section renamed by hand keeps its own name. Known limits ------------ The Harmonix label set has no refrain or pre-chorus class, so some sections cannot be named correctly by construction, and label accuracy falls off in the back half of some songs. Sections are presented as experimental, draggable suggestions, which is what they are. Feeding the model its own four-stem separation instead of StemDeck's remixed six-stem audio raised boundary recall from seven of nine to nine of nine on the reference track. That is measured on one song and would cost a second separation pass per job, so it is left as a follow-up rather than taken here. Note: this adds all-in-one-infer to the dependency set, so uv.lock changes and existing desktop installs will be sent to the full download rather than an in-app update.
…ental twice Three things about the sections bar, all reported from looking at it. A Clear button sits beside Add, hidden entirely while there is nothing to clear. Clearing cannot be undone and an automatic set costs a whole re-import to regenerate, so the first click only arms the button and the second one clears; it disarms itself after four seconds. The app has no modal-confirm idiom anywhere, and this is the lightest guard that still makes a mis-click harmless. The header laid its controls out with space-between, so every direct child drifted to its own corner and Clear ended up marooned from Add. They are one group now, and the split is title against controls rather than control against control. The "Experimental - drag to adjust." badge is gone, along with the _sectionsSource state that existed only to drive it. How a set arrived is still recorded server-side; the timeline just no longer labels it, because a marker is editable either way. The word now sits under the Song structure toggle instead, where the choice is actually made rather than on the results after the fact. While the toggle was being touched: it takes the same width and height as Split stems, so the pair reads as two segments of one bar rather than a chip loose beside a button. The width is shared through a variable and set as a minimum, not a fixed size, so a longer translation grows instead of clipping. It keeps the timeline blue rather than the accent amber, because two amber controls side by side read as two competing primary actions. Repeated section kinds are numbered. The model predicts boundaries and labels with separate heads, so two neighbouring Verse spans are a real predicted boundary between verse one and verse two rather than a duplicate, and merging them was already tried and found to discard five true boundaries on the reference track. The boundary stays and the labels read Chorus 1 and Chorus 2. A kind used once is never numbered, and a section renamed by hand keeps its own name. Reworking the badge tests into Clear-visibility tests caught a real bug: Clear lives in the header rather than the ribbon, so its state has to be refreshed before the container guard in initSections, and it was not.
Exporting a mixdown of a long track as WAV failed with a 500, after the server had already spent the whole render producing it. The threshold is about 49.5 minutes at 44.1 kHz and 45.5 at 48 kHz, both inside the 60 minutes StemDeck accepts, so this was a supported track failing rather than an unsupported one being refused. _render_to_file finishes by moving the render into the mixdown cache, pruning the cache, and returning the path the response is built from. The prune evicts oldest-first while the directory is over budget, and a single render larger than the whole 500 MB budget puts it over on its own. So the loop deleted the render, newest and only entry though it was, and FileResponse was handed a path that no longer existed. The prune now takes the file about to be served and never evicts it. Its size still counts toward the total, so an oversized entry clears everything else and then stops, leaving the cache one file over budget until the next render. That is the intended trade: a render the user is waiting on outranks the budget. Reproduced by shrinking the budget below one render, which is the same shape as a 60-minute WAV against the real 500 MB. The regression test asserts both halves, because exempting the served file must not quietly turn the prune into a no-op. The streaming path prunes too, but the client already has the bytes by then, so there the same eviction only costs a cache entry rather than the response. Closes #482
Section analysis stages its work in a temporary directory inside the job's own stems folder and removes it in a finally. That covers every ordinary ending, cancellation included. It does not cover the process dying: a force quit, a lost machine, an OOM kill, or the desktop shell tearing the backend down while the stage runs. Nothing else in the codebase had ever heard of the prefix, so what was left behind stayed forever. It is not a trivial amount. other.wav inside it is a real file, the other/guitar/piano mix written as pcm_f32le: roughly 1.27 GB for a 60-minute track, plus the extracted spectrograms beside it. The name starts with a dot, so a user wondering why their library outgrew their songs had no obvious way to find it. The window is not small either, because the stage is a CPU inference pass measured in minutes and it is the last thing a job does, which is exactly when an impatient user quits. The sweep runs once at startup and nowhere else. Nothing is analyzing at that point, so every workspace it finds is certainly dead; running it later could delete one out from under a live job. It reuses the same guard as the in-band cleanup, refusing to remove anything whose name lacks the prefix or whose parent is not the stems directory, because it is deleting inside the user's library. Its failures are swallowed and logged for the same reason the stage itself is non-fatal: tidying up must never be the thing that breaks a separation. Closes #483
A PATCH to a job's sections accepted a list of any length. A single request with a large body blocked the event loop, so everything else waited on it, including the progress stream a running job depends on. Measured against a local server, an idle health check went from 32 ms to 5219 ms behind a 33 MB body. The stall grew linearly with the body, bounded only by what the sender was willing to upload. Two things made it worse than a slow endpoint. It needed no valid job, because the body is parsed before the handler runs and only then answers 404. And with a real job it was permanent rather than transient: the list was written to metadata.json and held in the registry, which is re-serialised on every persist. There are two bounds now, because one of them alone does not work. The model caps the list at 10000. Both normalize_sections and the timeline editor refuse a section shorter than half a second, so the longest track StemDeck accepts cannot legitimately carry more than 7200. This is what stops a huge list being stored. That cap does not stop the stall. FastAPI reads and validates a request body before the handler runs, so a model constraint bounds what is kept and not what is parsed: with the cap alone the stall was still 5219 ms. Content-Length is therefore checked in middleware, which is the last point that runs before the body is touched, and the same shape as the upload pre-check already in app/api/jobs.py. Health latency under the same attack is 16 ms. The ceiling is 4 MB, far above either editor's reach: 10000 sections at the longest permitted name is about 1.6 MB and 20000 beats about 0.4 MB. It covers the beat grid too, which had the same flaw in milder form at 922 ms, and leaves uploads alone on their own path and their own 400 MB limit. Closes #481
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
The new section tests reached for the module through a module-level alias while every other test in the file imports it locally inside the test body, so the same module was being imported in two styles. Fixed by following the file rather than the newcomer: the alias is gone and the three new tests import it the way the other fourteen already do. The reverse would have been a larger diff across tests this branch has no business touching. Behaviour is identical either way; both names bind the same module object, so monkeypatch sees no difference.
The real source of the mixed import style in this file was one line the review bots never pointed at: a test that did "from app.api.jobs import _write_json_atomic" while the other sixteen tests bind the module and go through it. That test now binds the module too, so the file has one style and no exceptions. The bots flagged the sixteen and suggested converting two of them, which would have left fourteen the other way. Behaviour is unchanged; both forms resolve to the same function object.
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.
Automatic song-structure identification, off by default, plus three bugs the work turned up in already-released code.
Closes #484
Closes #485
Closes #481
Closes #482
Closes #483
What this adds
After separation, an isolated worker maps StemDeck's six stems into the four-stem layout All-In-One expects, runs inference, and normalizes the result into the existing editable Sections schema. Every field of the model's output is treated as untrusted.
The stage is non-fatal by construction: a failure, timeout, stall, or missing model leaves an otherwise usable separation exactly as it was. Cancellation stays authoritative so a long CPU pass can be stopped.
It is off by default. A
Song structuretoggle sits besideSplit stemsand writes to the server rather than the browser, so two devices pointed at one StemDeck cannot disagree about whether the next import pays for an inference pass. The runner reads it once per job, so a change applies to the next import without a restart.STEMDECK_AUTO_SECTIONS=1turns it on for a deployment.Evaluated on nine real tracks
Inference was run once per track and cached, so before and after were compared on byte-identical model output. Two tracks produced no sections at all and now work; none regressed.
The six-second clip correctly yields nothing at either revision: it is shorter than one minimum section. The reference track's thirteen spans match the recorded timings to within 4 ms, verified through the real worker subprocess rather than an offline harness.
Four defects caused the two failures, each now covered by a test that fails without its fix:
duration_secis rounded while the model reads the stems, so its timeline overhangs by a fraction of a second. One track ended with a 10 ms span beyond it, which clamped to zero length and was read as malformed.startandendare ordinary label classes, not only brackets. One track was labelledstartfor 34 seconds in its middle andendfor 32 seconds. Rejecting the song over a misplaced sentinel threw away every section.WinError 1314is anOSErrorrather than thePermissionErrorthe hub falls back on. Zero of four tracks fetched the model before the fix, six of six after.Three bugs in already-released code
Found while auditing everything since v0.15.2, and unrelated to sections except that looking for them is what turned them up.
#481 A PATCH to a job's sections accepted a list of any length, and one request held the event loop for everything else, including a running job's progress stream. An idle health check went from 32 ms to 5219 ms behind a 33 MB body, and needed no valid job to do it. Two bounds now: the model caps the list, and Content-Length is checked in middleware, which is the last point that runs before the body is touched. Health latency under the same attack is 16 ms. The beat-grid endpoint had the same flaw in milder form and is covered too.
#482 Exporting a track over about 50 minutes as WAV failed with a 500 after the full render. A single render larger than the whole 500 MB cache budget put the cache over on its own, so the prune deleted it, newest and only entry though it was, and
FileResponsewas handed a path that no longer existed. StemDeck accepts 60 minutes, so this was a supported track failing.#483 A crash during section analysis left its workspace inside the user's stems folder forever, and nothing else in the codebase had heard of the prefix.
other.wavinside it is a realpcm_f32lefile: about 1.27 GB for a 60-minute track. It is dot-prefixed, so it was invisible. A sweep now runs once at startup, when nothing can be analyzing.Display
Repeated section kinds are numbered rather than merged. Merging was the original behaviour and discarded five true boundaries on the reference track, so the boundary stays and the labels read
Chorus 1andChorus 2. A kind used once is never numbered, and a section renamed by hand keeps its own name.A
Clearbutton sits besideAdd, hidden while there is nothing to clear, arming on the first click because clearing cannot be undone. TheExperimentalbadge moved from the results to the toggle that produces them, taking with it the state that existed only to drive it.Known limits
The Harmonix label set has no refrain or pre-chorus class, so some sections cannot be named correctly by construction, and label accuracy falls off in the back half of some songs. Sections are presented as experimental, draggable suggestions.
Feeding the model its own four-stem separation instead of StemDeck's remixed six-stem audio raised boundary recall from seven of nine to nine of nine on the reference track. That is one song, and it would cost a second separation pass per job, so it is left as a follow-up rather than taken here.
Verification
889 backend tests pass; the 4 failures are pre-existing and Windows-only (CRLF line endings, a Linux installer check, process-watchdog semantics). 28 JS tests pass.
ruff checkandruff format --checkclean. i18n complete at 468 keys across all nine tables, with a European Portuguese override where the wording genuinely differs.Before merging
uv.lock.all-in-one-inferis a new dependency, so existing desktop installs will be sent to the full download rather than an in-app update. Worth deciding deliberately rather than discovering after tagging.0.15.2.