Skip to content

Keep finished tracks unless the user asks otherwise - #464

Merged
thcp merged 3 commits into
mainfrom
fix/library-retention
Aug 26, 2026
Merged

Keep finished tracks unless the user asks otherwise#464
thcp merged 3 commits into
mainfrom
fix/library-retention

Conversation

@thcp

@thcp thcp commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Closes #459
Closes #462
Closes #463

The report

A user restored a library of four completed jobs, started the 0.15.1 backend against it, and two of them were gone: from data/jobs/ and from registry.json. They reproduced it twice, kept read-only backups, and rolled back to 0.13.0.

I reproduced it on the packaged 0.15.2 backend using their own job IDs:

before after
without STEMDECK_DESKTOP=1 all four d81f3ab304ab, e0ed2aafc483
with STEMDECK_DESKTOP=1 all four all four

Same survivors they saw. The selection is age, not source type: sweep_old_jobs deletes terminal jobs past a 24 hour TTL, and it runs seconds after startup.

It was never a 0.15.1 regression. app/pipeline/collect.py has no commits between v0.13.0 and v0.15.1 and the gate in app/main.py was byte identical. Three separate problems stacked up to produce it.

1. Deletion was the default (#459)

Safety depended on an environment variable being present:

def _sweep_disabled() -> bool:
    return (
        os.environ.get("STEMDECK_DESKTOP") == "1"
        or os.environ.get("STEMDECK_PERSIST_LIBRARY") == "1"
    )

Every documented way of launching StemDeck sets one of those. The shell sets STEMDECK_DESKTOP=1, run.sh defaults STEMDECK_PERSIST_LIBRARY=1, the Unraid template ships it as 1. So the sweep only ever fired on paths nobody documents, which is where it did the most damage and where nobody would be expecting it.

A missing environment variable must not mean "delete the user's work". Deletion is now opt-in and those two variables are redundant rather than load-bearing. STEMDECK_PERSIST_LIBRARY=0 stays as the one env-based way in, for deployments that set it deliberately.

No supported deployment changes behaviour, because none of them were sweeping.

2. The settings file could not be found (#462)

_SETTINGS_PATH derives from DATA_DIR, which falls back to ROOT when STEMDECK_DATA_DIR is unset, and only the desktop shell sets that. A backend started any other way read backend/settings.json, which does not exist, and silently ignored the stems location, port, device, quality and language the user had chosen.

It now finds the data directory from the layout every package shares: ROOT named backend with a data/ sibling. Not a marker file, because only the Windows package writes one. Docker resolves ROOT to /app and a source checkout resolves it to the repo, so neither is affected. An explicit STEMDECK_DATA_DIR still wins.

This is also what made a directly-run backend a different application with the same files, which is what turned a debugging session into data loss.

3. There was nowhere to see or change it (#463)

Settings > General now has a switch, off by default, that reveals a days field when it is on.

Automatically delete finished tracks    [ off ]
    Delete after  [ 30 ] days            ← only while on

The stored choice beats the environment. That is deliberately the opposite of jobs_dir, where the env pin wins because a mounted volume is not the user's to relocate; how long someone's own work is kept is theirs to decide, and StemDeck is single-user with no separate operator to protect. A tuned STEMDECK_JOB_TTL_SECONDS seeds the default days so existing deployments keep their intent.

There is no "0 means never". In a field reading "delete after N days", 0 most naturally reads as delete immediately, and that ambiguity has no place on the one control that destroys work.

The sweep loop also re-reads the setting every pass. It used to decide once, before its first iteration, which would have made this the only setting in the panel that did nothing until relaunch.

Verified locally, before CI

  • Python: 787 passed, against 774 on the unmodified tree. Same 14 pre-existing local failures (beatgrid, worker watchdog) on both.
  • Browser: 46 Playwright tests pass, 41 existing plus 5 new covering the reveal, the round trip through a reload, the server-owned ceiling and digit filtering.
  • Lint: ruff check and ruff format --check clean; node --check on every static/js/*.js; the tests/js unit tests pass.
  • i18n: all eight tables at 443 keys with the coverage script reporting clean, and ptPT overriding only the words that genuinely differ (eliminar, predefinição, anulada).

Release notes

No dependency change, so uv.lock and the runtimeId are untouched and this reaches existing desktop installs through the in-app updater.

A Windows user restored a library of four completed jobs, started the
0.15.1 backend against it, and two of them were gone from disk and from
registry.json. Reproduced here on the packaged 0.15.2 backend with their
own job IDs: without STEMDECK_DESKTOP=1 the same two disappear, with it
all four survive.

Three problems stacked up to produce that.

The deletion was the default. sweep_old_jobs removed terminal jobs older
than 24 hours and ran seconds after startup, unless an environment
variable switched it off. Every documented way of launching StemDeck set
one: the shell sets STEMDECK_DESKTOP=1, run.sh defaults
STEMDECK_PERSIST_LIBRARY=1, and the Unraid template ships it as 1. So
the sweep only ever fired on paths nobody documents, where it did the
most damage. A missing environment variable must not mean "delete the
user's work", so deletion is now opt-in and the two variables are
redundant rather than load-bearing. STEMDECK_PERSIST_LIBRARY=0 is kept
as the one env-based way in, for deployments that set it deliberately.

The settings file could not be found. _SETTINGS_PATH derives from
DATA_DIR, which falls back to ROOT when STEMDECK_DATA_DIR is unset, and
only the desktop shell sets that. A backend started any other way read
backend/settings.json, which does not exist, and silently ignored the
stems location, port, device, quality and language the user had chosen.
It now finds the data directory from the layout every package shares:
ROOT named backend with a data/ sibling. Docker resolves ROOT to /app
and a checkout resolves it to the repo, so neither changes.

There was nowhere to see or change any of this. Settings > General now
has a switch, off by default, that reveals a days field when it is on.
The stored choice beats the environment, unlike jobs_dir where the env
pin wins because a mounted volume is not the user's to relocate; how
long someone's own work is kept is theirs to decide, and StemDeck is
single-user with no separate operator to protect. A tuned
STEMDECK_JOB_TTL_SECONDS seeds the default days so existing deployments
keep their intent.

The sweep loop now re-reads the setting every pass. It used to decide
once before its first iteration, which would have made this the only
setting in the panel that did nothing until relaunch.

No "0 means never": in a field reading "delete after N days", 0 most
naturally reads as delete immediately, and that ambiguity has no place
on the one control that destroys work.

Tests: 13 new Python tests covering the default, the env precedence, the
day clamps and the loop re-read, plus 5 browser tests for the reveal,
the round trip and the server-owned ceiling. Keys added to all nine
i18n tables, with European Portuguese overriding eliminar/predefinicao.
Comment thread static/js/i18n.js Fixed
Comment thread static/js/i18n.js Fixed
Comment thread static/js/i18n.js Fixed
Comment thread static/js/i18n.js Fixed
Thales added 2 commits August 26, 2026 15:05
The four auto-delete strings landed in `en` instead. The script that
inserted them anchored on settings.stemsLocation.moving and took the
first match, and that key exists in every table, so the first one is
English. A later duplicate key wins in a JS object literal, so English
rendered "Eliminar automaticamente as faixas concluidas" and ptPT was
left with nothing to override.

Caught by looking at the running app in English, which is the only place
it was visible. The i18n coverage script in .claude/rules/i18n.md builds
a set of keys per table, so the duplicate collapsed into one entry and
it reported en complete at 443 keys while English was serving
Portuguese. It checks that a key exists, never that it appears once or
that its value belongs to that language.

ptPT goes from 84 own keys to 88.
Two things, both found by looking at the running app.

The days row was meant to be hidden while automatic deletion is off, and
it was not. `hidden` does nothing to a settings row: base.css is not
loaded by index.html at all, and daw.css scopes its rule to `.daw` while
the settings overlay is appended to document.body. Nothing matched, so
`.settings-row { display: flex }` won and the row showed regardless.
That is also why .settings-pane.hidden had to be written explicitly.

Dimmed and inert is the better answer anyway, and it does not depend on
that rule existing. Someone deciding whether to switch deletion on can
read how long tracks would be kept without having to switch the
destructive setting on to find out.

The second one the browser tests caught. Flipping the switch POSTs, and
apply() writes the server's days value back from that response. If the
user had already started typing into the field the switch had just
enabled, their number was silently replaced about a round trip later.
apply() now leaves a focused field alone, and the days handler writes
its own result back explicitly so committing with Enter still shows the
value the server actually kept.

46 browser tests pass, 41 existing and 5 for this.
@thcp
thcp merged commit 2d38cad into main Aug 26, 2026
10 checks passed
@thcp
thcp deleted the fix/library-retention branch August 26, 2026 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant