mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-11 19:29:33 +00:00
8f014e6a30b8eaae4b94eeeb3f29eb7df8521ca0
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
8f014e6a30 |
refactor(server): carve the library scanner into lib/scan.py (R3b) (#901)
lib/scan.py (326). server.py 2,098 -> 1,870.
The background scan, its spawn ProcessPoolExecutor, and the kick/runner plumbing that
serialises passes. Bodies VERBATIM except the seam reads.
Everything shared is read LATE off appstate — the same contract every module in
lib/routers/ uses, and it is not cosmetic: tests monkeypatch CONFIG_DIR and swap meta_db,
so a value captured at import time pins the wrong one for the life of the process.
CONFIG_DIR -> appstate.config_dir
meta_db -> appstate.meta_db
_default_settings -> appstate.default_settings()
_stat_for_cache -> appstate.stat_for_cache()
━━━ THE SCAN STATUS IS REBOUND, NOT MUTATED ━━━
_background_scan does `global _scan_status; _scan_status = {**INIT, ...}` at every stage
transition. It REPLACES the dict; it never updates it in place. So nothing may hold that
dict by value — a reference captured once goes permanently stale at the first stage change
and would report "listing" forever while the scan ran to completion.
Hence `scan.status()`, a getter, and hence appstate publishes scan_status as a CALLABLE.
appstate.py already said so in a comment; this is the code that makes it true. (Same for
the plugin_context entry, which was already `lambda: dict(_scan_status)` — late-bound, so
it survives the move unchanged. The contract test from #898 covers it.)
━━━ appstate.server_root: A TRAP CLOSED PERMANENTLY ━━━
_background_scan seeds the builtin content, which needs the directory holding server.py.
`Path(__file__).resolve().parent` is correct in server.py and silently WRONG anywhere under
lib/ — it yields lib/, which holds no docs/ or data/ — and it fails by finding NOTHING
rather than by raising, so the seeds would just quietly never run.
lib/builtin_content.py (#900) closed that by taking the root as a parameter. This adds the
other half: server.py publishes it ONCE as appstate.server_root, so no module under lib/
ever has a reason to derive it. Documented at the slot.
pyflakes caught two more missing imports on the way in (loosefolder_mod, enrichment) —
each a NameError on a live scan path, and the suite would have handed them over one failure
at a time. It stays part of every server.py slice.
TESTS. The two scan fixtures (test_settings_api::scan_module,
test_feedpak_extension::scan_server) patched server._make_scan_executor to swap the spawn
pool for an in-process ThreadPool; they now patch it on lib/scan.py. Worth noting WHY that
still works: the fixtures re-import `server` per test, but `scan` stays cached in
sys.modules — and it picks up the fresh CONFIG_DIR anyway, because the appstate reads are
late-bound. The seam is doing exactly the job it was built for.
━━━ TEST ISOLATION: A REGRESSION THE CARVE ITSELF CREATED (Codex [P2]) ━━━
background_scan() deliberately NEVER sets running=False — ownership of that flag lives in
_scan_runner, so a kick_scan() racing the terminal write cannot observe a stale False and
start a second runner. Correct in production.
But the scan fixtures call background_scan() DIRECTLY, skipping the runner. That was
harmless while the state lived on `server`, which the fixtures RE-IMPORT per test. It is
NOT harmless now: `scan` stays cached in sys.modules across sys.modules.pop("server"), so
the status dict OUTLIVES the test. One direct call leaves the shared scanner marked
"running" forever, and every later scan or rescan returns "already in progress" and quietly
does nothing.
Verified: after a direct call, kick_scan() returns False and starts no scan at all.
The suite passed anyway, on ordering luck — which is exactly how this class of bug ships.
tests/conftest.py::reset_scan_state now snapshots and restores lib/scan.py's module state
around the two fixtures that drive it directly.
pytest 2398, pyflakes 0, Codex 0.
Refs #48
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
6b8f79dd9a |
fix(server): a raising tuning provider no longer takes down get_merged() for everyone (#899) (#904)
One word. server.py's TuningProviderRegistry.get_merged():
except Exception:
- logger.exception("tuning provider %r raised during get_merged()", provider_id)
+ log.exception("tuning provider %r raised during get_merged()", provider_id)
There is no `logger` in server.py — the module logger is `log`. So the handler written to
swallow-and-report a bad provider instead raised NameError from inside the except, and that
NameError propagated out of get_merged().
The effect was the exact OPPOSITE of what the handler is for: one misbehaving plugin took
the whole merged-tunings call down for every other provider, AND the traceback named the
wrong problem ("name 'logger' is not defined" rather than the provider that actually blew
up). Doubly silent: nothing was ever logged either, because the logging call was the thing
that crashed.
Found by pyflakes while carving server.py (R3b). It survived because NOTHING exercised the
failure path — no test ever had a provider raise. That is the whole reason this class of
bug is invisible: it lives only on error paths, so the suite is green and the feature is
broken exactly when it matters.
tests/test_tuning_provider_isolation.py is that path:
* a raising provider must not lose the HEALTHY providers' tunings, nor the defaults
* and the failure must actually be LOGGED — swallowing is only acceptable if it reports
Bite-tested: restoring `logger` fails both.
(The log assertion attaches caplog's handler to the feedBack logger directly. It sets
propagate=False, so pytest's root capture sees nothing from it — test_plugins.py has a
capture_logger() for this, but it is not importable here: pyproject pins pythonpath to
[".", "lib"], so `tests` is not a package. Three lines beat churning 21 call sites in an
unrelated file to convert that helper into a fixture.)
pytest 2410, pyflakes 0 undefined names in server.py, Codex 0.
Closes #899
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
70dbe45e27 |
refactor(server): carve builtin-content seeding into lib/builtin_content.py (R3b) (#900)
lib/builtin_content.py (321 lines moved). server.py 2,418 -> 2,098.
The calibration/diagnostic sloppaks and the starter library: _copy_builtin_packs,
_write_builtin_pack, the two seed helpers, their source tables, and the seed marker.
━━━ THE ONE SIGNATURE CHANGE, AND WHY THE CARVE IS UNSAFE WITHOUT IT ━━━
server.py has:
def _feedBack_server_root() -> Path:
return Path(__file__).resolve().parent
That is correct IN server.py: the repo root in dev, resources/feedBack when bundled — the
tree that actually holds docs/ and data/.
Move that body into lib/ unchanged and it keeps working, silently, and returns lib/. There
is no docs/diagnostics under lib/, so every seed would find nothing, log "source missing"
at debug, and return. Nothing raises. Nothing fails. The starter library simply never
appears, and the calibration sloppak is never seeded — on a fresh install, in the field.
A verbatim move whose MEANING changed because __file__ did.
So this module cannot compute a root: `server_root` is a PARAMETER, and server.py — the
only place that legitimately knows where it lives — passes it in. The trap is now
structurally impossible rather than merely avoided. (_copy_builtin_packs already took the
root that way; the two seed helpers now do too.)
Everything else is byte-identical. CONFIG_DIR is read late as appstate.config_dir and the
DLC root through dlc_paths._get_dlc_dir — the same seam every router in lib/routers/ uses,
late-bound because tests monkeypatch it.
━━━ PYFLAKES FOUND THREE MISSING IMPORTS THE TESTS WOULD HAVE FOUND ONE AT A TIME ━━━
The moved code uses `secrets`, `stat` and `tempfile`; none was in my import block. Each is
a NameError on a live path. `python3 -m pyflakes` names all three in one shot — this is the
Python twin of the no-undef gate that guarded every frontend carve, and it should run on
every server.py slice from here.
It also flagged a PRE-EXISTING one I deliberately did not touch: server.py's
TuningProviderRegistry.get_merged() calls `logger.exception(...)` in an except handler and
there is no `logger` in the module (it is `log`). So a raising tuning provider takes down
the merged-tunings call for everyone, with a NameError naming the wrong problem. Filed as
issue #899 rather than smuggled into a carve whose whole value is being behaviour-neutral.
The constants lost their underscore prefix: they cross a module boundary now (the seed
tests read them), so `_BUILTIN_STARTER_SOURCES` was a lie.
pytest 2397, pyflakes 0, Codex 0. Guarded by the plugin_context contract test (#898).
Refs #48
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
1cef01d02c |
test(plugins): pin the plugin_context contract before carving server.py (R3b) (#898)
tests/test_plugin_context_contract.py (3 tests). No production code changes. server.py is about to be carved apart around startup_events(), and `plugin_context` — the 20-key dict handed to every plugin's setup() — is built inline inside it. Issue #48 flagged this while planning the split and asked for exactly this guard: "Plugin context[...] are passed as live references into already-loaded plugins. Refactoring must preserve the exact callables — moving them to a new module is fine, but renaming or wrapping them breaks third-party plugins. We'd want a 'plugin context unchanged' assertion in CI." It never got written. Writing it FIRST, because a key silently dropped or renamed by a move is invisible to every other test in the suite — nothing in-tree reads most of these — and would break plugins at runtime, in the field. This is the backend's version of the window contract, and the frontend carve just taught me what that costs: 43 of library.js's exports were referenced ONLY from app.js's top-level window block, invisible to any call-graph scan, and trusting the scan would have shipped a dead A-Z rail with CI fully green. A contract only external code reads has to be pinned BY NAME, before the move, not after. THE SURFACE IS BIGGER THAN server.py's DICT. Shipped plugins read `log` and `load_sibling`, and neither is in it — plugins/__init__.py layers them on per-plugin. A test pinning only server.py's 18 keys would have missed both. ━━━ CODEX CAUGHT ME WRITING A VACUOUS ASSERTION ━━━ My first identity test built a dict locally and called setup() on it — asserting `dict(x)['k'] is x['k']`, which is trivially true and blind to everything the loader does. [P2], and correct. It now drives the REAL plugins.load_plugins() with a probe plugin, which matters: the loader DOES deliberately wrap one key (register_library_provider is scoped per-plugin so a plugin cannot forge owner attribution and impersonate another). The test pins that single intentional exception so it cannot quietly become two. Codex then caught [P2] number two: my hand-rolled teardown restored only PLUGINS_DIR and LOADED_PLUGINS, while load_plugins() also mutates sys.path, sys.modules and PENDING_PLUGINS — order- and environment-dependent. tests/test_plugins.py already had a fixture that does this properly, so `reset_plugin_state` moved to tests/conftest.py: ONE copy, shared, rather than a second that will drift. BITE-TESTED IN FIVE DIRECTIONS — drop a key, rename a key, drop a per-plugin key, wrap extract_meta in the loader (all key names intact, identity broken), and remove the register_library_provider scoping (the impersonation guard). Each fails. pytest 2399, Codex 0. Refs #48 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
756588678b |
fix(plugins): make a module plugin actually re-evaluate on reload (#879) (#897)
A plugin reload silently did nothing for scriptType:"module" plugins. ES modules are evaluated ONCE PER URL PER DOCUMENT, so re-inserting a <script type="module"> whose src the module map has already seen fires `load` without re-running the body — and the loader then recorded the reload as applied. A no-op that reported success. THE ISSUE UNDERSTATES IT. #879 says "upgrades are fine — a new version yields a new URL". That is true of screen.js and FALSE of the plugin. I drove a real browser through install(1.0.0) -> upgrade(1.1.0) -> rollback(1.0.0), counting evaluations of src/main.js: ONE. Not three, not two. The upgrade re-runs the one-line screen.js shim at its new ?v= URL; the shim does `import './src/main.js'`; a relative specifier resolves against the base URL WITH THE QUERY DROPPED; that is the same URL as before; the module map hands back the already-evaluated v1.0.0 module. The plugin's own code never re-ran. Busting the entry point cannot fix this, whatever token you hang off it. So the token goes in the PATH: /api/plugins/<id>/g/<n>/screen.js. From there './src/main.js' resolves to /api/plugins/<id>/g/<n>/src/main.js — every relative import inherits it, at every depth, for free. No import-specifier rewriting (which could never see `import(expr)` anyway). Same browser drive after the fix: THREE evaluations. Keyed on the plugin ID, not id@version: EVERY re-load of a module plugin needs a fresh path, not just a rollback. First load keeps the stable ?v= URL, so the ETag/304 live-edit caching the R0 rails depend on is untouched. Classic-script plugins are not affected and never take a /g/ path. ━━━ A PATH REWRITE, NOT TWO MIRRORED ROUTES ━━━ Codex caught this, and it was right. The token shifts the BASE URL, so EVERYTHING the module graph resolves relatively moves with it — not only imports. `new URL('../assets/worklet.js', import.meta.url)` from /api/plugins/x/g/1/src/main.js resolves to /api/plugins/x/g/1/assets/worklet.js. Mirroring only screen.js and src/ would have fixed imports and 404'd every asset, worklet and wasm file the graph reaches — and would have broken again the next time someone added a plugin route. So the /g/<token> segment is STRIPPED BEFORE ROUTING. Every plugin route, present and future, works under the prefix with no extra wiring. The token is opaque and never joined into a filesystem path, so containment still rests entirely on the same safe_join. Codex then caught a [P3] in that: eagerly re-encoding raw_path with latin-1 raises UnicodeEncodeError on a valid plugin file like src/工具.js, 500ing a request the plain route serves fine. raw_path is informational and Starlette routes on scope["path"], so the mutation is simply gone — and leaving raw_path as the client sent it is more truthful for logs anyway. TESTS. tests/js/plugin_module_rollback.test.js (5) + 8 in test_plugin_src_route.py: identical bytes under the prefix, the whole graph one and two levels deep, ASSETS (the Codex [P2]), every plugin route, non-ASCII filenames (the [P3]), an opaque token, and containment asserted as PARITY with the un-prefixed route rather than a guessed 404 — `../screen.js` legitimately 200s on both, because the URL normalises before routing. All bite-tested: reverting the fix fails the rollback tests, disabling the rewrite fails the asset tests. Two harnesses re-anchored on `script.src = _pluginScriptUrl(` — the URL literal they keyed on now lives in the helper, further down the file, so their slice ran off the end. node 1045, pytest 2404, ESLint 0, Codex 0. Closes #879 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
bd830328f0 |
refactor(app): carve the library out of app.js (R3a) (#896)
static/js/library.js (1,988) + static/js/library-state.js (29) — bodies VERBATIM.
app.js 6,313 -> 4,451.
THE BIGGEST SLICE OF THE CARVE: 145 declarations, ~1,900 lines, 30% of what was left.
The grid, the artist tree, the A-Z rail, filters, pagination, selection, favourites, the
scan banner, and the library-provider plumbing.
A LOW module: it imports only leaves (./dom.js, ./format.js, ./library-state.js,
./tuning-display.js — all four import nothing themselves) and needs ZERO host hooks. It
calls nothing in app.js. That is not luck; it is why this cluster was picked. Two entry
points that WOULD have dragged the playback core in were left behind in app.js:
* syncLibrarySong reaches showScreen/playSong
* _handleLibArrowNav Enter on a selected row plays the song
Both are one hop from the library, and app.js is the root, so it imports from both sides
for free. Pulling them in swallows playSong, showScreen and the whole remaining core — I
measured it: the closure jumps from 145 declarations to 189.
library-state.js holds exactly FIVE fields. An imported binding is read-only, and of the
library's outward bindings only these five are genuinely WRITTEN from outside — by
showScreen, deleteSongFromModal and syncLibrarySong, none of which can move in. The other
23 are read-only from outside, so they stay plain exports (ES live bindings mean app.js
still sees every reassignment).
━━━ THE EXPORT LIST NEARLY SHIPPED A DEAD A-Z RAIL ━━━
59 exports — and 43 of them CANNOT be found by a call-graph scan. They are referenced only
from app.js's TOP-LEVEL statements: the Object.assign(window, {...}) contract and the
scattered window.X = X lines, which live outside every function, so a closure walk over
declarations never sees them. Among them are the four handler names app.js composes AT
RUNTIME into onclick="" strings — filterTreeLetter, filterFavTreeLetter, goTreePage,
goFavTreePage — the library A-Z rail and its pagination. No static tool can see those at
all. Had I trusted the call-graph, the rail would have died silently on click with nothing
failing in CI.
━━━ AND MY OWN SCANNER LIED ━━━
The cycle-risk pass reported "(none)" for this carve. It was wrong, and it could not have
been right: a dangling `else if` bound to an inner `if` instead of the outer chain, so its
`imported` map was ALWAYS empty and the check reported clean no matter what. A guard that
cannot fail is worse than no guard. Fixed, and it then found the real edges — dom.js,
format.js, tuning-display.js, library-state.js. All four are leaves, so the carve is
genuinely acyclic; I just now know it instead of assuming it.
(The AST rewriter had its own trap: `MAP[name]` with an object literal and name ===
'constructor' hits Object.prototype.constructor — truthy — and it happily rewrote
`constructor(id)` into `L.function Object() { [native code] }(id)`. Every identifier in
the file is looked up, so the lookup must not see the prototype chain. It is a Map now.)
TESTS. legacy_shim_hits SPLIT (loadLibraryProviders + setLibraryProvider -> the module;
syncLibrarySong stayed in app.js). v3_library_refresh now reads app.js AND the module,
rather than being re-pinned to whichever file happens to hold the emit this week.
VERIFIED. A/B against origin/main in two browsers: the whole window contract, cards render,
grid/tree/sort/filter/clear round-trip — and, specifically, the A-Z rail: 28 onclick
handlers composed at runtime, identical on both, and a real .click() on a letter works.
IDENTICAL on all 33 + 7 probes, no new page errors.
pytest 2396, node 1040/1040, host contract 2/2, ESLint 0 (no-cycle clean).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
09f7e450a5 |
refactor(app): give formatTime a home — a leaf format.js, one fewer host hook (R3a) (#895)
static/js/format.js (17). One function. Retires the formatTime hook: 12 -> 11. WHY A MODULE FOR ONE FUNCTION. formatTime was a host hook — loops.js and section-practice.js both reached back through the seam for it. It is ALSO, by pure accident of who calls it, inside the dependency closure of the library carve that comes next. Leaving it there would have made loops.js and section-practice.js import the LIBRARY in order to format a timestamp — nonsense, and a cycle waiting to happen. Same rule as the transport carve: a hook is a cycle you agreed to live with; an import is a dependency you actually have. formatTime has a real owner. It just isn't app.js, and it certainly isn't the library. Give it a home and both consumers import it directly. A leaf on purpose. Anything else that turns out to be a shared pure formatter belongs here too; nothing does yet (I checked — formatBadge, _safeImageUrl and _fetchJsonOrThrow have no callers outside the library), so nothing else is here. node 1040/1040, host contract 2/2, ESLint 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
8bec8d2466 |
refactor(app): carve the playback transport out of app.js — and RETIRE 8 host hooks (R3a) (#894)
static/js/transport.js (377) — bodies VERBATIM. app.js 6,643 → 6,316.
THIS IS THE FIRST CARVE THAT SUBTRACTS HOOKS INSTEAD OF ADDING THEM.
Every carve before this one added host hooks: a module pulled out of app.js still had
to call back into it. But four modules were all reaching through the seam for the SAME
handful of names — _audioSeek, _audioTime, setPlayButtonState, _songEventPayload,
jucePlayer. Those names have an owner, and it isn't app.js. Give them one, and the
consumers import them directly:
count-in.js 5 hooks -> 0 (host import deleted)
juce-audio.js 4 hooks -> 0 (host import deleted)
loops.js 6 hooks -> 4
section-practice.js 10 hooks -> 7
----------------------------------------------------------
configureHost() 20 hooks -> 12
A hook is a cycle you agreed to live with. An import is a dependency you actually have.
Prefer the import whenever the name has a real owner.
_audioSeekGen now stays PRIVATE. It has exactly one writer — _resetAudioSeekState(),
which moved with it — so readers get audioSeekGen() and nobody outside can desync it.
Strictly better than the hook it replaces, which handed out a getter and left the writer
behind in app.js.
THE SCAN HAD A HOLE, AND IT BIT. Picking the carve by dependency closure over app.js's
own top-level decls said this cluster was downward-closed. It wasn't:
_currentPlaybackSnapshot reads loopA/loopB — which live in ./js/loops.js, and loops.js
imports transport. The scan saw nothing, because loopA STOPPED BEING an app.js decl the
moment loops.js was carved out. Any dependency scan of a partly-carved monolith has to
resolve the imports too, or it will confidently hand you a cycle. Added that pass; it
found exactly one back-edge, and _currentPlaybackSnapshot stays in app.js (as does
restartCurrentSong, which calls _cancelCountIn). app.js is the root — it imports both
sides for free.
TESTS. Four harnesses retargeted (play_button_reroute_guard, song_event_payload,
song_seek -> transport.js; playback_app_adapter SPLIT, since
_installPlaybackTransportAdapter stayed behind).
The two CENSUS tests — "≥8 song:* emit sites", "every seek callsite passes a reason" —
now scan app.js AND every static/js/*.js, not one file. Pointed at a single file, their
count silently shrinks as code leaves, which reads as "someone deleted an emit" or, worse,
passes while genuinely missing sites. Both bite-tested: stripping a _songEventPayload()
from an emit and adding a reason-less _audioSeek() each fail the suite.
VERIFIED. A/B against origin/main, real song, real playback: song:play payload is exactly
{audioT, chartT, perfNow, time}; song:seek carries reason "seek-by" with finite from/to;
all five song:* events fire; seekBy advances the clock; restartCurrentSong returns to zero;
the play button's aria-pressed tracks state. IDENTICAL on all 21 probes, zero page errors.
pytest 2396, node 1040/1040, host contract 2/2, ESLint 0 (no-cycle clean), Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
8d0e270345 |
refactor(app): carve the JUCE/desktop audio shims out of app.js (R3a) (#893)
* refactor(app): carve resume-session out of app.js (R3a)
static/js/resume-session.js (157) — the snapshot taken when you leave a song and the
pill that offers it back. Bodies VERBATIM. app.js 7,727 → 7,601.
Fifth slice out of the strongly-connected core. ONE hook (playSong) + a
currentFilename getter.
S.pendingResume JOINS THE CONTAINER — on demand, exactly as intended. app.js WRITES
it (playSong({ resume }) arms it; the song:ready listener consumes it) while this
module reads it, so it cannot be a plain export: an imported binding is read-only.
Same reason isPlaying is there. The container grows one field per carve that needs
it, never speculatively.
THE CONTRACT TEST CAUGHT THE MISSING HOOK, again on a path nothing executes:
"playSong is read by a module but never wired by app.js — it would throw at runtime".
Second time it has caught a real wiring gap the moment it appeared.
A REAL TRAP, worth remembering: I first did the S.pendingResume rewrite by feeding
acorn's identifier RANGES from node into python, and it corrupted the file
(`_pS.pendingResume null;`). **Acorn's offsets are UTF-16 code units; Python's string
indices are code points.** static/app.js contains emoji, so every offset past one
drifts. Do an AST-driven rewrite in the SAME language that produced the offsets.
`node --check` caught it; a silent version of that bug is very easy to imagine.
VERIFIED. A/B against origin/main in two browsers, real song: the window API
(resumeLastSession / _snapshotResumeSession / _readResumeSession /
_clearResumeSession), snapshot, read-back, and clear — IDENTICAL, zero page errors.
HONEST LIMIT: my probe never got the snapshot to actually PERSIST (there is a guard
beyond the 3s minimum position that a scripted playSong does not satisfy), so that
path is verified only as identical-to-main, not as observed-working. The real
coverage is tests/browser/resume-session.spec.ts, which drives the flow properly.
Zero harnesses broke. pytest 2396, node 1040/1040, ESLint 0 (no-cycle clean),
tailwind clean, Codex 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(app): carve the JUCE/desktop audio shims out of app.js (R3a)
static/js/juce-audio.js (994) — bodies VERBATIM. app.js 7,603 → 6,643.
THE LARGEST SINGLE SLICE of the whole carve phase: 960 lines, ~13% of what was left.
Three self-installing IIFEs:
_installJuceEngineRoutingWatcher (444) routes a song to the JUCE engine or HTML5 as
the desktop output enters/leaves exclusive/ASIO
_installRendererBusFeeder (337) feeds the highway renderer bus from whichever
transport is actually running
_installJuceAudioElementShim (156) patches audio.play/pause so the rest of the app
keeps talking to the <audio> element while JUCE
owns the transport
They EXPORT NOTHING — all three publish through `window.*` (_juceMode,
_reevaluateJuceRouting, _reevaluateRendererBus, …). So app.js needs only a
side-effect import plus the one binding it actually uses
(_resetJuceAudioShimChain, which the shim IIFE assigns).
THE ORDERING QUESTION, CHECKED RATHER THAN ASSUMED. Importing this module runs the
IIFEs EARLIER than before: imports evaluate ahead of app.js's body, and therefore
ahead of configureHost(). A hook read at IIFE-execution time would THROW. So I walked
the AST at IIFE-body depth to see what they actually touch when they run: nothing but
listener registration, and `audio.play`/`audio.pause` patching — and `audio` is itself
an imported module now. Verified in the browser: both are patched on the carved build
exactly as on main, which proves the shim installs correctly at its new, earlier point.
(Had I got this wrong, host.js throws loudly rather than silently misbehaving — which
is the whole reason it has no no-op defaults.)
VERIFIED. A/B against origin/main in two browsers: the entire window.* surface the
IIFEs publish (_juceMode, _juceOutputIsExclusive, _reevaluateJuceRouting,
_reevaluateRendererBus, _clearJuceRerouteMemo), audio.play/pause patched, a real song
loading and togglePlay driving the public mirror — IDENTICAL, zero page errors.
Harnesses: juce_engine_reroute (19 tests) + renderer_bus_feeder (13) slice the IIFEs by
signature — retargeted, and each sandbox gains a `host` object routed at its EXISTING
stubs so every assertion holds unchanged. test_plugin_runtime_idempotence is SPLIT: 3 of
its 4 source-asserts stayed in app.js, the sm.emit('song:resume') one moved.
pytest 2396, node 1040/1040, ESLint 0 (no-cycle clean), tailwind clean, Codex 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
dc429ecd16 |
refactor(app): carve the player controls out of app.js (R3a) (#891)
static/js/player-controls.js (229) — the speed + mastery sliders and the four
playback-preference reads (autoplay-exit, up-next, countdown-before-song,
confirm-exit). Bodies VERBATIM. app.js 7,914 → 7,727.
The fourth slice out of the strongly-connected core, and by far the easiest: ONE
hook (handleSliderInput) and NO shared mutable state. The three groups are the same
surface — the controls under the highway — and the preference reads are the
one-line localStorage lookups half of app.js consults before deciding whether to
auto-start, show the Up Next pill, run a count-in, or confirm on exit. They travel
with the controls that set them.
Zero missed members on the first build (the no-undef pass was clean), which is the
first time that has happened in this phase.
TWO HARNESSES ARE SPLIT, and both taught something:
* speed_reset spans BOTH files — playSong (app.js) resets the speed controls
(module). Its presence GUARDS still read `src.includes('function setSpeed')`
against app.js, so once the code moved they silently evaluated FALSE and the
helpers were quietly dropped from the sandbox. A guard that disables itself is
worse than no guard. Repointed at the file the code actually lives in.
* Its `host.handleSliderInput` stub had to route at the sandbox's EXISTING spy,
not a fresh `() => {}`. The test asserts the slider was actually refreshed
(`deepEqual(__sliderInputs, ['speed-slider'])`); a fresh stub swallows the call
and the assertion passes VACUOUSLY. Same failure mode as a no-op host default —
the thing this whole seam design exists to prevent.
* autoplay_exit is split too: _autoplayExitEnabled moved, but the auto-exit
machinery around it (_clearAutoExit, holdAutoExit, _resolvePlayerOrigin) stayed.
VERIFIED. A/B against origin/main in two browsers, real song: setSpeed(0.75) ->
playbackRate 0.75; applySpeedPreset(100) -> 1; the speed slider; setMastery;
setAutoplayExit / setCountdownBeforeSong / setShowUpNext — IDENTICAL, zero page errors.
pytest 2396, node 1040/1040, ESLint 0 (no-cycle clean), tailwind clean.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
11f8c36b61 |
refactor(app): carve count-in (and the song-credits overlay) out of app.js (R3a) (#890)
static/js/count-in.js (389) — bodies VERBATIM. app.js 8,223 → 7,913. The third slice out of the strongly-connected core, and the first that WRITES shared state rather than only reading it. #889's container is what makes it possible. imports: loops (setLoop/loopA/loopB — a count-in inside an A-B loop must begin at A), audio-el, player-state, host hooks : _audioSeek, setPlayButtonState, _songEventPayload, togglePlay + a jucePlayer getter Nothing imports count-in back — app.js and section-practice both reach it through the seam — so the graph stays acyclic. app.js's autoplay path used to reach IN and set this module's credits timers itself (_creditsTimer, _creditsHideOnPlay) and read _countingIn. It cannot now, and should not have to, so the module exports the OPERATIONS instead — armCreditsHideOnPlay(), scheduleCreditsHide(), holdCreditsThen(start), isCountingIn() — and owns its own timer invariants. Third time this has happened (section-practice's resetSelection, loops' state) and each time the constraint produced better code than was there before: the module keeps its own promises instead of trusting a caller 6,000 lines away to zero the right fields. THE no-undef GATE FOUND FIVE MISSED MEMBERS, one at a time: showSongCreditsOverlay and startSongCountIn (my name regex matched startCountIn, not startSongCountIn), then _creditLineLabel, _CREDITS_MAX_MS, and _CREDIT_ROLE_VERBS. A call-graph closure does not see a const table; only the undefined-symbol pass does. AND A REAL TRAP: I computed _CREDIT_ROLE_VERBS's span against the ALREADY-MODIFIED app.js and applied it to the clean one — the line numbers had drifted, so the slice would have cut somewhere else entirely. Recomputed every span from the clean file with acorn. Never carry line numbers across an edit. VERIFIED. A/B against origin/main in two browsers with a real song: playback state, the public feedBack.isPlaying mirror, audio position, cancel-count-in — IDENTICAL, zero page errors. Unit coverage moved with the code: loop_restart's count-in cancellation-token test and the 5 song_credits_overlay tests now read count-in.js; loop_restart's sandbox gains a `host` object routed at its EXISTING stubs, so every assertion is unchanged. HONEST LIMIT: I could not make the count-in OVERLAY actually render headlessly — its autoplay path needs a fresh-load _pendingAutostart that a scripted playSong() never arms. Behaviour is identical to main on every probe and the unit tests cover the logic, but the on-screen 1-2-3-4 and the credits card want a human look. pytest 2396, node 1040/1040, ESLint 0 (no-cycle clean), tailwind clean, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
5fb28d5c5a |
refactor(app): lift the shared player state onto a container (R3a) (#889)
static/js/player-state.js — one exported object, two fields. app.js's 70 reference
sites rewritten. Provably a no-op; nothing shrinks.
WHY NOW. Every slice carved out of app.js so far only ever READ the state it shared
(loopA/loopB, _audioSeekGen, currentFilename), so a read-only getter hook was enough
and no container was needed — twice I checked and twice I got away with it. That
runs out at count-in: it genuinely WRITES `isPlaying` (it starts and stops playback,
4 sites) and `lastAudioTime` (2). `import { isPlaying }` then `isPlaying = true`
THROWS — an imported binding cannot be assigned to. So the state has to live on an
object: `S.isPlaying = true` is a property write, and works from any module holding
the same S. Same shape stems, studio, and editor all converged on.
DELIBERATELY SMALL. app.js has ~104 top-level `let` scalars; lifting all of them is
a ~977-site rewrite for no benefit, because most are private to one cluster and
travel with it. Only what a carved module must WRITE goes here. Add on demand.
THE REWRITE IS AST-DRIVEN, NOT TEXTUAL — and that is not fussiness. Of 100 textual
occurrences of these two names, only 70 resolve to the module binding:
* 22 are member accesses (`someObj.isPlaying`, `window.feedBack.isPlaying`)
* 4 are the LOCAL PARAMETER of `function setPlayButtonState(isPlaying)` — a blind
replace yields `function setPlayButtonState(S.isPlaying)`
* 1 is an object key
* 2 are shorthand properties `{ isPlaying }`, which must become
`{ isPlaying: S.isPlaying }` — and acorn gives a shorthand's key and value the
SAME range, so rewriting both produced `isPlaying: S.isPlaying: S.isPlaying`
until I deduped by range
A find-and-replace corrupts all 29. The rewrite walks the AST, skips shadows, member
properties and keys, and replaces identifier RANGES.
`window.feedBack.isPlaying` — the PUBLIC mirror — is a different thing and is
untouched. Two test sandboxes stub it; those were left alone deliberately.
VERIFIED WITH REAL PLAYBACK. A/B against origin/main in two browsers, real song:
togglePlay -> the public mirror goes true -> false -> true across two toggles, the
audio element's paused state follows, seekBy works — IDENTICAL, zero page errors.
Harnesses: 8 vm-sandbox suites slice playback code out of app.js and now see
S.isPlaying — juce_engine_reroute, loop_restart, play_button_reroute_guard,
playback_app_adapter, song_restart, song_seek, speed_reset, and the python
idempotence source-assert. Each gets the same container in its sandbox; every
assertion is unchanged.
pytest 2396, node 1040/1040, ESLint 0, tailwind clean, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
cb236e6c04 |
refactor(app): carve the A–B loop out of app.js (R3a) (#888)
static/js/loops.js (261) — bodies VERBATIM. app.js 8,421 → 8,224.
The second slice out of the strongly-connected core.
It OWNS the loop state — loopA, loopB, _loopMutationGen. Nothing outside writes
them: restartCurrentSong() looked like it did, but it declares its own local `let
loopA/loopB` shadows, so the module-level bindings only ever change in setLoop /
setLoopStart / setLoopEnd / clearLoop. All four move here. No state container.
DIRECTION IS THE WHOLE DESIGN. loops and section-practice are mutually dependent —
the SCC in miniature. clearLoop() must drop section-practice's selection, and
practiceSection() must call setLoop(). Both edges cannot be imports or no-cycle
(rightly) rejects it. So:
section-practice -> reaches loops through the HOST SEAM (host.setLoop, …)
loops -> imports section-practice DIRECTLY
section-practice is the higher-level feature — a consumer of loops, not the reverse
— so it is the one that takes the indirection. app.js hands the loop module's
exports across into the seam for it. Graph stays acyclic; no-cycle passes.
THE CONTRACT TEST EARNED ITS KEEP IMMEDIATELY. It failed on the first build with
"these hooks are wired by app.js but no module reads them: playSong". My dependency
scan had counted a mention of playSong() inside a COMMENT in loops.js as a real
call. Wired but unused is precisely the "fossil of a rename" case the test exists
for — and it caught it on a path no test executes.
VERIFIED BY DRIVING BOTH SIDES OF THE SEAM. A/B against origin/main in two browsers,
real song loaded:
* setLoop(5,12) -> true; getLoop() -> 5,12 — IDENTICAL
* clearLoop() (loops -> section-practice, a direct import) -> getLoop() ->
null,null — IDENTICAL
* onPhraseNext() (section-practice -> loops, ACROSS THE SEAM) -> ok — IDENTICAL
* loadSavedLoop / saveCurrentLoop / deleteSelectedLoop on window — IDENTICAL
* zero page errors either side. An unwired hook throws, so a live app is itself
proof the seam is wired.
Harness: loop_api extracts the loop helpers by signature — retargeted to loops.js,
`export` stripped for the vm sandbox, and the sandbox's existing _audioSeek /
_audioTime / formatTime spies are now routed through a `host` object so every
assertion holds unchanged, just through the indirection the real code uses. It is
SPLIT: one test still reads app.js for the window.feedBack API surface, which stayed.
pytest 2396, node 1040/1040, ESLint 0 (no-cycle clean), tailwind clean.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
64f04565e2 |
refactor(app): the host seam + carve section practice out of app.js (R3a) (#887)
static/js/host.js (99) + static/js/section-practice.js (1,214).
app.js 9,461 → 8,409.
THE FIRST SLICE OUT OF THE STRONGLY-CONNECTED CORE. What is left in app.js is not
a tree, it is a cycle: seeding a dependency closure from section-practice, from
loops, from count-in, or from the JUCE seek shim all return the SAME 178-function
set, and setLoop() and practiceSection() call each other directly. No closure-based
carve can cut it at any seed. So it is cut BY NAME, and the calls back into app.js
go through a host seam.
61 functions + its own 24 _sectionPractice*/_sectionParents* scalars (read nowhere
else) move out. 11 hooks come back in. 4 of those are read-only GETTERS —
loopA/loopB/_audioSeekGen/_loopMutationGen are only ever READ here, never written,
so app.js keeps owning them and NO state container is needed (a 977-site lift
avoided).
app.js used to reach IN and reset the module's state by hand (clearLoop() zeroed the
selection; changeArrangement() invalidated the parent count). It cannot now — an
imported binding is read-only — so those are exported as resetSelection() and
invalidateParentCount(). Strictly better: the module owns its own invariants instead
of trusting two callers on the far side of the file to zero the right three fields.
═══ THE SILENT-NO-OP PROBLEM, SOLVED ═══
The obvious host seam is an object of no-op defaults. That is a TRAP and we walked
into it once: the plugin loader's seam defaulted populateVizPicker to `() => {}`, so
a dropped wiring line would have left the viz picker quietly not refreshing with NO
test, boot check, or bot noticing. Two layers stop it here:
1. RUNTIME — host.js is a Proxy with NO defaults and NO stubs. Reading an unwired
hook THROWS. An unwired hook cannot degrade into a no-op because there is
nothing to degrade INTO. configureHost() also rejects a non-function at WIRE
time, and refuses to run twice.
2. STATIC — tests/js/host_contract.test.js asserts the hooks the modules USE are
exactly the hooks app.js WIRES. This is the layer that matters: a runtime throw
only fires if the broken path executes, and the whole danger of a seam is the
paths that never run in a smoke test. VERIFIED TO BITE in all three drift
directions: drop a hook from configureHost -> fails; rename host.setLoop in the
module -> fails; wire a hook nobody uses -> fails.
Writing that guard took three tries and each failure is instructive: (a) the
configureHost regex anchored `});` at column 0, ran past the indented close, and
swallowed app.js's 66-name window contract — 77 "hooks"; (b) an import-stripping
regex with `[\s\S]*?` ate 14,000 characters INCLUDING the drift the bite test was
meant to catch — a guard with a hole is worse than no guard, because you trust it;
(c) `host.js'` in the import path backtracked from `js` to a "hook" called `j`.
The bite tests are what surfaced all three.
CODEX FOUND A REAL RACE [P2]. configureHost() was inside the async boot function,
after several awaits — but the window handlers (onPhraseNext, …) go live during
app.js's SYNCHRONOUS module evaluation. A user clicking one in that window would hit
"[host] … was read before configureHost() ran". It is now a bare top-level statement
sitting immediately before the window contract, so the seam is always wired before a
handler can be reached. Verified live: invoking a handler 1.2s in — well before the
boot awaits settle — works.
VERIFIED. A/B against origin/main in two browsers with a REAL song loaded: popover
toggle, practice-mode change, phrase-next, and clearLoop (all of which cross the seam
— setLoop/clearLoop/_audioTime/loopA/loopB) — IDENTICAL, zero page errors. Since an
unwired hook throws, a live app is itself proof the seam is wired.
Harnesses: section_practice_dismiss retargeted; loop_api's clearLoop sandbox gains a
resetSelection SPY (not a stub) and ASSERTS it fires — the guarantee is still tested,
just through the seam.
pytest 2396, node 1040/1040, ESLint 0 (no-cycle clean), tailwind clean, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
f53d566dbc |
refactor(app): give the <audio> element a module of its own (R3a) (#886)
static/js/audio-el.js — one exported const. app.js's diff is 5 lines.
This is a HINGE, not a carve: nothing shrinks, but almost everything left in
app.js is blocked behind it.
WHY. `audio` is `document.getElementById('audio')` with 162 references in app.js
and 173 outside any one cluster. Every remaining cluster measured — settings,
app-updates, count-in (208 fns), exit-confirm (212), library-render (220) — lists
`audio` among its inbound symbols, because they all touch playback and playback
reaches for the element directly. A module that needs it cannot import app.js to
get it (that closes a cycle and fails import-x/no-cycle), so today the only way to
carve any of them would be a host seam — the exact thing #878 had to build and
#880 had to tear out.
WHY IT'S SAFE. `audio` is a `const` and is NEVER reassigned anywhere in core, so a
read-only import binding is exactly right and no state container is needed. The
162 call sites are untouched — the binding keeps its name, it is just imported
instead of declared. (Contrast the reassigned scalars — isPlaying, _avOffsetMs —
which CANNOT be shared this way: an imported binding cannot be written to. Those
still need containers, and that is the next problem, not this one.)
TIMING. app.js is <script type="module">, so it evaluates after the HTML is parsed
and its imports evaluate just before its body — the same moment app.js used to run
this exact lookup. If the element had not been in the document, `audio` would be
null and app.js's top-level `audio.addEventListener(...)` calls would throw and
kill the module. They don't.
VERIFIED WITH REAL PLAYBACK, not a boot check. A/B against origin/main in two
browsers: app alive with zero page errors (which is itself the proof the import
resolved), #audio is an AUDIO element, togglePlay/seekBy live, and playSong() on a
real library song sets audio.src and the element reports a duration — IDENTICAL on
both sides.
pytest 2396, node 1038/1038, ESLint 0 (no-cycle clean), tailwind clean, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
b5dd585d25 |
refactor(app): carve settings backup + plugin updates out of app.js (R3a) (#885)
Two leaves, one PR. app.js 9,651 → 9,457.
static/js/settings-io.js (155) — exportSettings + importSettings, the Settings
backup bundle. Imports nothing. The two-phase rationale comment (server first and
atomic; then a best-effort localStorage merge) is the contract and moved with the
code.
plugin-updates → INTO static/js/plugin-loader.js, not a module of its own.
checkPluginUpdates + updatePlugin are plugin MANAGEMENT; they belong with the code
that loads plugins. A new file for 50 lines would have been a file for its own
sake.
All four are inline handlers on the Settings screen and already in app.js's window
contract, so app.js re-exposes the imported bindings unchanged.
VERIFIED BY DRIVING BOTH FLOWS. A/B against origin/main in two browsers:
* checkPluginUpdates() -> hits the API and settles the button back to
"Check for Updates" — IDENTICAL
* exportSettings() -> POSTs /api/settings/export and writes
"Exported feedBack-settings…" to #backup-status — IDENTICAL (fetch intercepted
so the assertion is on the real call, not a stub)
* all four resolve on window — IDENTICAL
* zero console/page errors either side
Zero harnesses broke. pytest 2396, node 1038/1038, ESLint 0, tailwind clean, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
d47883c5e5 |
refactor(app): carve the tuning-display helpers out of app.js (R3a) (#884)
static/js/tuning-display.js (228 lines) — bodies VERBATIM. app.js 9,838 → 9,650.
A LEAF: imports nothing.
Tuning NAME resolution (Drop D / Eb Standard / raw-offset fallback), bass
detection, effective string count, and the target FREQUENCIES + note names the
tuner checks against. Pure functions over a small MIDI/note-name table; the 3
_TUNING_* tables are read nowhere else and move in.
NOT A SLICE — a node-level extract. The span 2309-2535 INTERLEAVES the functions
with the `window.*` / `window.feedBack.*` assignments that publish them, and one
of those is `window.feedBack = window.feedBack || {}` — the BUS BOOTSTRAP, not
tuning code at all. Every ExpressionStatement stays exactly where it was; only the
16 functions and 3 tables move. app.js re-exposes the imported bindings from the
same lines, so the public surface and its ordering are untouched (constitution II
names window.feedBack).
app.js -> { plugin-loader, viz, diagnostics-export, dom, highway-colors,
tuning-display }
HARNESSES — 4 broke, and 3 of them broke in the SAME informative way: they sliced
app.js from `function isBassArrangement(` UP TO the marker
`window.feedBack.parseRawTuningOffsets = parseRawTuningOffsets;` — an end-marker
that (correctly) stayed behind in app.js. The module is now nothing BUT the tuning
helpers, so there is no block to slice: they read it whole and strip `export ` so
the vm sandbox still evaluates it as a script.
tuner_auto_open is SPLIT — its autoplay-gate test still reads app.js, so it keeps
APP_JS and gains TUNING_JS. Retargeting its path wholesale (my first attempt)
silently pointed the autoplay test at the wrong file.
VERIFIED BY DRIVING THE CONTRACT. A/B against origin/main in two browsers, through
the real window surface: displayTuningName -> "E Standard" / "Drop D" /
"Eb Standard", parseRawTuningOffsets('-2,0,0,0,0,0') -> [-2,0,0,0,0,0],
isBassArrangement, effectiveStringCount, displayTuningTargets, and
window.feedBack.displayTuningName / .songTuningContext — IDENTICAL on both, zero
console/page errors either side.
pytest 2396, node 1038/1038, ESLint 0, tailwind-fresh clean, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
ebbfc8da6f |
refactor(app): carve the highway string-colours out of app.js (R3a) (#883)
static/js/highway-colors.js (601 lines) — bodies VERBATIM. app.js 10,415 → 9,837. Under 10k. DECOMPOSED, not sliced. The "settings" blob measured 47 fns / 19 inbound and was not carvable as-is. Seeding the closure from a FUNCTION (initHighwayColors) found only 18 fns and left 4 HWC_* constants used outside it — i.e. the seed was wrong, not the cluster. Re-seeding from the STATE (every function touching HWC_*/`_hwc*`) found the true cluster: 45 top-level nodes, lines 2751-3331, CONTIGUOUS, with ZERO foreign nodes inside the span. INBOUND: 0. EXPORTS: 2 (initHighwayColors, hwcInitSettingsUI). The other 43 symbols — the HWC_* tables, the 12 presets, the theme store, the share codec, the picker handlers, the window.feedBack.highwayColors facade — are used nowhere else in core and stay private. No inline on*= handlers here (the Settings buttons are wired by addEventListener inside hwcInitSettingsUI), so nothing needed re-exposing on window. The three bus listeners register inside initHighwayColors, which app.js calls — not at module top level — so no ordering change. THE no-undef GATE EARNED ITS KEEP. My closure said INBOUND=0; the module actually uses `uiPrompt` (the "name this theme" prompt). It was missed because uiPrompt is no longer an app.js DECLARATION — it's an IMPORT BINDING (from #882's dom.js), and I was collecting declarations only. `no-undef` with typeof:true caught it. => Lesson for the next carve: seed `tops` from ImportDeclaration bindings too. => And it VALIDATES carving dom.js early: this module just imports uiPrompt from it. Had dom.js still been stranded in app.js, this carve would have needed a host seam. app.js -> { plugin-loader, viz, diagnostics-export, dom, highway-colors } plugin-loader -> viz highway-colors -> dom viz, diagnostics-export, dom -> (leaves) VERIFIED BY DRIVING THE FACADE. A/B against origin/main in two browsers: window.feedBack.highwayColors installed, identical method surface, 12 presets, identical default slot colours, and a share-code encode→decode round-trip returning #112233 — IDENTICAL on both, zero console/page errors either side. Harnesses: highway_colors_facade + highway_string_colors retargeted (both brace-extract blocks out of the source by signature). pytest 2396, node 1038/1038, ESLint 0, tailwind-fresh clean, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
14b4058bc6 |
refactor(app): carve the DOM/modal primitives out of app.js (R3a) (#882)
static/js/dom.js (203 lines) — esc, _escAttr, _isElementVisible, _trapFocusInModal,
_confirmDialog, uiPrompt. Bodies VERBATIM. app.js 10,593 → 10,414.
A GATHER, not a slice — the six lived in six different places (108, 635, 659,
2617, 2623, 8892). They belong together because they are the BOTTOM of the UI
stack: `esc` alone has 25 call sites and `_escAttr` 23, and every later carve that
renders HTML will need them.
That is the actual point of doing this one now. Give them a home and the next
carve imports them; leave them in app.js and the next carve that renders HTML has
to invent a host seam to reach back into app.js — exactly the trap the
plugin-loader carve had to work around until the viz layer became a module. This
is the cheapest possible way to stop that recurring.
app.js -> { plugin-loader, viz, diagnostics-export, dom }
plugin-loader -> viz
viz, diagnostics-export, dom -> (nothing)
Zero imports. Six exports (every one is used outside the cluster).
VERIFIED BY DRIVING THE MODALS, not just booting — they are interactive, so a
green suite says little. A/B against origin/main in two browsers:
* window.uiPrompt / _confirmDialog / _trapFocusInModal all resolve
* uiPrompt() mounts its modal, accepts typed input, and resolves with the typed
value ('typed') — IDENTICAL on both
* _confirmDialog() mounts and resolves true on confirm — IDENTICAL
* zero console/page errors either side
pytest 2396, node 1038/1038, ESLint 0, tailwind-fresh clean, Codex 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
bfb31a8b89 |
refactor(app): carve the diagnostics-bundle export out of app.js (R3a) (#881)
static/js/diagnostics-export.js (280 lines) — bodies VERBATIM.
app.js 10,858 → 10,592.
Chosen BY MEASUREMENT, not by eye. Ran the transitive closure over four candidate
clusters and took the one with the smallest interface:
diagnostics 7 fns 235 lines span 4110-4378 imports 1 exports 2
shortcuts-modal 5 fns 235 lines span 104-9897 imports 4 exports 5
settings+updates 47 fns 1012 lines span 1409-7636 imports 19 exports 30
library-render 220 fns 4064 lines span 20-10536 imports 126 exports 117
diagnostics is contiguous and nearly closed; its one inbound symbol
(_DIAG_FILE_LABELS) lives inside the region and is read only by _renderDiagPreview,
so it moves in and the module ends up a LEAF — imports nothing.
app.js -> { plugin-loader, viz, diagnostics-export }
plugin-loader -> viz
viz, diagnostics-export -> (nothing)
Exports exactly 2: previewDiagnostics + exportDiagnostics, both already in app.js's
window contract (they're inline handlers in the Settings screen) — so app.js keeps
re-exposing them, now as imported bindings. The preview renderer, the file-label
table, and the byte/HTML formatters are used NOWHERE else in core and stay private.
VERIFIED BY DRIVING IT, not just booting. Zero harnesses broke — because the
diagnostics export flow had NO source-level test at all, which is exactly why a
green suite proves nothing here. So the flow was exercised for real: A/B against
origin/main in two browsers, window.previewDiagnostics() invoked, the preview
container rendered identical content on both, both entry points resolve on window,
zero console/page errors either side.
pytest 2396, node 1038/1038, ESLint 0, tailwind-fresh clean, Codex 0.
NOTE for the next carve: library-render is NOT a cluster — 220 functions and 126
inbound symbols is most of app.js entangled together. It cannot be carved as a
unit; it needs decomposing from the inside first.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
a222b45c02 |
refactor(app): carve the viz layer out of app.js — and delete the loader seam (R3a) (#880)
static/js/viz.js (770 lines) — the viz picker, renderer selection, Auto-match, the WebGL2 probe, the 3D-promotion nag, the notation hints. Bodies VERBATIM. app.js 11,603 → 10,857. THE SEAM IS GONE. #878's plugin-loader needed configurePluginLoader({ populateVizPicker }) purely because _populateVizPicker lived in app.js and importing app.js would have closed a cycle. viz.js is a LEAF — it imports NOTHING — so plugin-loader now imports _populateVizPicker straight from it. The _host object, the configure function, its loud-default guard, and the wiring line in app.js are all deleted. The second carve simplifies the first. app.js -> { plugin-loader, viz } plugin-loader -> viz viz -> (nothing) NOT A PURE MOVE — one listener block had to be SPLIT. app.js had a single top-level `if (window.feedBack) { … }` registering four handlers, and only two were viz. song:loaded / arrangement:changed / song:ready (the mastery slider) stay in app.js and now call the imported _autoMatchViz / _maybeShowNotationViewHint. The viz:reverted handler MOVES, because it REASSIGNS _cancelPendingAutoLabel and an imported binding is read-only — `_cancelPendingAutoLabel = null` would throw if the listener stayed behind while the state moved. ORDER CHECKED, NOT ASSUMED: viz.js's song:ready listener now registers BEFORE app.js's own (imports evaluate first). Safe — _pendingPromotionNag is only ever set inside _populateVizPicker, which runs at boot/plugin-refresh, never from inside the other song:ready handler, so the two are independent. VERIFIED — the listeners are the risk here, so they were DRIVEN, not just booted. A/B against origin/main in two browsers: * viz picker: 6 options (auto|default|venue|drum_highway_3d|keys_highway_3d| highway_3d), selected highway_3d, Auto label — IDENTICAL. This alone proves plugin-loader's direct import of viz.js works. * emit('viz:reverted') -> picker resets to default, localStorage resets to default, the warning logs — IDENTICAL. The MOVED listener fires. * emit('song:ready') -> mastery slider enables, no throw — IDENTICAL. The SPLIT listener still does both halves. * plugin screens, module injections, 37 capability participants — IDENTICAL. * zero console/page errors on both. pytest 2396, node 1038/1038, ESLint 0, tailwind-fresh clean. no-cycle re-bitten on the 3-module graph (viz -> plugin-loader fails). Codex preflight raised a [P2] claiming viz.js's top-level bus guards would be false because "app.js only creates the event bus later" — FALSE POSITIVE. app.js does not create the bus; capabilities.js does, from its own <script type="module"> at index.html:122, and module scripts execute in document order, so the bus exists long before app.js's import graph evaluates. Instrumented the setter: by viz.js's turn `window.feedBack.on` is already a function, and the viz:reverted listener is provably attached (firing it resets the picker). The ordering is also enforced by test_app_shell_loads_capability_registry_before_app_runtime. Harnesses: 5 tests retargeted to viz.js across legacy_shim_hits, venue_scene_3d, venue_viz (each SPLIT — their non-viz tests still read app.js). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
38772f604a |
refactor(app): carve the plugin loader out of app.js into static/js/ (R3a) (#878)
The first carve, and deliberately the riskiest: app.js IS the plugin loader (the
R0 host rails), so it goes first while the module graph is still one edge deep.
static/js/plugin-loader.js (829 lines) — bodies VERBATIM. app.js 12,217 → 11,439.
Core's first `static/js/` module, exactly as constitution II anticipates.
CLOSURE (measured with acorn, not regex — brace-matching stripped source drifted):
the block at app.js:11246-12031 is contiguous and self-contained. It needs only
TWO things from the rest of app.js, and exports only TWO:
exports: loadPlugins (the window contract), bootstrapPluginsAndUi (boot)
inbound: window.showScreen — already the public host contract (constitution II),
so it is called through `window`, not re-coupled as an import
_populateVizPicker — injected via configurePluginLoader()
WHY A SEAM, NOT AN IMPORT. plugin-loader must not import app.js: app.js imports
it, so that would close a cycle. I checked whether _populateVizPicker could just
move into the module instead (which would delete the seam entirely) — it drags 9
further symbols (_canRun3D, _autoMatchViz, _showPromotionNag, …), i.e. a whole
viz cluster. That is its own carve, so the seam stays.
THE SEAM'S DEFAULT IS LOUD, ON PURPOSE. A no-op stub is the classic silent
failure for this pattern (see the editor's setHostHooks trap, hit twice): drop the
wiring call and the loader keeps working while the viz picker quietly stops
refreshing — no test, no boot check says a word. The default now console.errors,
so the smoke harness catches it. VERIFIED BY BITE TEST: removing
configurePluginLoader() from app.js surfaces
"[plugin-loader] host seam not configured" at boot. The seam IS exercised on the
plugin-startup path, so an unwired hook cannot pass silently.
no-cycle is now LIVE on core's own graph for the first time. eslint.config.js
gains `static/app.js` + `static/js/**` to the module block — app.js now `import`s,
so parsing it as a script would be a syntax error. VERIFIED BY BITE TEST: making
plugin-loader import app.js back fails with "Dependency cycle detected".
HARNESSES (the R3a note said budget one conversion per carve — it was five):
retargeted capability_inspector_nav, plugin_hydration_wipe,
plugin_loader_script_type, plugin_style_injection, legacy_shim_hits (SPLIT — one
test needs the loader, one still needs app.js) + test_plugin_runtime_idempotence.
legacy_shim_hits was missed by a symbol-name grep because it greps for a code
STRING; only the failing run found it. test_capability_events' NEGATIVE asserts
now span app.js + the loader — carving code out of app.js would otherwise make
them vacuous instead of failing.
VERIFIED: A/B against origin/main in two browsers — mounted plugin screens, 14
loaded plugin scripts, the 3 module plugins injected as <script type="module">,
37 capability participants, 14 shims, window.loadPlugins: IDENTICAL, zero
console/page errors on both. /static/js/plugin-loader.js serves 200; R0 rails
intact (src/main.js 200, conditional GET 304, script_type passthrough).
pytest 2396, node 1032/1032, ESLint 0, Codex 0.
Codex preflight caught a REAL [P1] first pass: static/js/plugin-loader.js was
untracked, so a checkout would have served an app.js importing a nonexistent
module — a failed static import kills the whole module and every window handler
with it. Now tracked.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
92c86f5393 |
refactor(ui): load app.js as an ES module (R3a) (#876)
One attribute. #871/#872/#874/#875 exist to make this line safe. app.js's 385 top-level `function` declarations stop being implicit `window` properties: 87 stay reachable via the explicit contract (#874's Object.assign block + the 47 pre-existing `window.X = X` assignments), and 298 become module-private. Verified NO unexposed name is read from outside app.js. Strict mode (modules are always strict) checked ahead of the flip: app.js parses clean as `sourceType: module` (no octal, dup params, `with`), and has no implicit globals, no `eval`/`new Function`, no top-level `this`. `registerShortcut` is called bare at 15 top-level sites but is assigned at `window.registerShortcut` (app.js:10387) before its first call (10648), and a bare identifier in a module still resolves through the global object — verified `typeof window.registerShortcut === 'function'` in the browser. HARD GATE — app.js IS the plugin loader: - /api/plugins script_type passthrough: editor/stems/studio = "module" - /api/plugins/stems/src/main.js -> 200; conditional GET -> 304 (live-edit ETag) - deep graph: stems/src/transport.js, editor/src/state.js -> 200 - window.loadPlugins present; 5 plugin screens mount; the 3 migrated plugins injected as <script type="module"> - 37 capability participants, 14 compatibility shims, bus + capabilities v1 Every one of the shell's 336 inline handlers resolves on window under module scope, and the A-Z rail / pagination execute 6/6 with no ReferenceError. A/B against origin/main: the ONLY unresolvable handler is `editorToggleStemMixer`, which is equally broken on main (a dead handler in the editor plugin — not defined anywhere in its source; pre-existing, flagged separately). Codex preflight raised a [P1] claiming restartCurrentSong / requestExitSong / editRegionInEditor / returnToEditorFromHighway would ReferenceError — FALSE POSITIVE. It scanned only #874's new Object.assign block and missed app.js's 47 scattered `window.X = X` assignments; all four are at app.js:7086/7204/8492/8511 and all four resolve as `function` in the browser with app.js loaded as a module. pytest 2396, node 1032/1032, ESLint 0 errors, tailwind-fresh clean. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
c223ace419 |
refactor(ui): load the capabilities as ES modules (R3a) (#875)
ship-ci / ci (push) Waiting to run
The 12 capability <script> tags become type="module". No JS changes — the capability scripts already self-register on the window.feedBack bus, version-negotiate (`capabilities.version !== 1` → bail), and self-guard for idempotency. They never import or call app.js; it is pure pub/sub. Verified they export nothing by name: no top-level declaration in capabilities.js or capabilities/*.js is read by any other script, so losing global scope costs nothing. This is the first REAL exercise of the ordering fix from #872. A module defers to after HTML parse, so the capabilities now execute AFTER the document is parsed — while app.js still calls `window.feedBack.on(...)` at its top level. That only works because #872 put every classic script into the same deferred queue, where document order IS execution order: capabilities.js (line 122) still runs before app.js (line 1237). Had app.js stayed a plain classic script it would have run during parse, hit a bare `{}`, and died on `.on is not a function`. A/B against origin/main, 11 probes — capabilities.version, registered participants (37), compatibility shims (14), the bus, workingTuning, theme, setViz/showScreen/playSong, mounted plugin screens: IDENTICAL, zero console/page errors on both. 12 module tags served and executed; pytest 2396, node 1032/1032, ESLint 0, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
ff7e855e35 |
refactor(app): make app.js's window contract explicit — 66 names (R3a) (#874)
app.js is a classic script, so each of its 385 top-level `function foo()` decls
is implicitly a property of `window`. As an ES module it will not be — module
scope is not global scope — and every name reached from outside this file would
silently vanish. This adds the explicit `window.*` assignments BEFORE the flip.
Provably a NO-OP: all 66 are top-level function declarations, so while app.js is
still a classic script `Object.assign(window, {...})` only re-assigns what
`window` already has. That is what makes it safe to land on its own, ahead of
the flip that needs it.
The consumers are wider than the inline handlers in index.html:
- inline on*= handlers in static/v3/index.html
- on*= handlers app.js BUILDS inside template literals (goFavPage,
updatePlugin, hideScanBanner, ...) — they resolve against window at CLICK
time, but live in a JS string, so scanning the HTML alone never finds them
- static/v3/*.js (showScreen alone has 17 consumers), capabilities
- feedback-desktop and the external plugin repos — easy to miss, they live in
other repos and no core test covers them
- capabilities/visualization.js reads window.setViz behind a `typeof` guard,
so losing it DEGRADES IN SILENCE rather than throwing
Constitution II names window.playSong / window.showScreen / window.feedBack as
the public extension contract, so this is an obligation, not a convenience.
FOUR names are invisible to every static tool. app.js:2156-2157 picks the
handler NAME at runtime —
const letterFn = favoritesOnly ? 'filterFavTreeLetter' : 'filterTreeLetter';
— and interpolates it into `onclick="${letterFn}('A')"`. The names exist only
inside string literals, so ESLint, no-undef, and any grep for `onclick="fn` all
miss them. They are the library A-Z rail and its pagination: drop one and those
buttons throw at click time and nowhere else.
New tests/js/window_contract.test.js scrapes the HTML's handlers AND app.js's
template-literal handlers, and pins the 4 runtime-composed names by hand.
Verified to BITE: dropping showScreen, goTreePage, or setViz each fails it with
the right message.
On-device: 28 A-Z rail buttons render with their real onclick sources
(filterTreeLetter('A'), ...) and 8/8 execute with no ReferenceError; all 66
names resolve on window in the browser.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
4b4c156fce |
refactor(ui): defer every classic script, keep boot() on DOMContentLoaded (R3a) (#872)
Puts every external `<script>` in the v3 shell into the deferred queue, and
keeps each script's boot() firing at DOMContentLoaded exactly as it does today.
Behaviourally a no-op; it is what makes the ES-module flips safe.
WHY. `type="module"` defers execution to after HTML parse. Classic-`defer` and
module scripts share ONE "execute after parsing" list and run in DOCUMENT ORDER,
but a plain classic script runs DURING parse — ahead of all of them. So the
moment capabilities.js becomes a module while app.js is still plain, app.js runs
FIRST, and its 11 top-level `window.feedBack.on(...)` calls (app.js:6245-6722)
hit a bare `{}` — `_ensureFeedBackEventBus()` (capabilities.js:33), which
attaches .on/.emit/.off, would not have run yet. TypeError, app.js dies
mid-parse. Deferring everything now keeps document order == execution order
through the rest of the migration.
THE CATCH (Codex preflight caught this — a real ordering change). 22 scripts
guard their boot with `if (document.readyState === 'loading')`. A deferred
script runs at readyState 'interactive', so that test is FALSE and the else-branch
fires boot() immediately, at the script's position in document order — instead of
at DOMContentLoaded, after every script has evaluated.
That matters far more than one call site: a scan of the shell's scripts found
**43 forward references** where a script's boot() reads a global that a LATER
script defines (shell.js -> profile.js's window.v3Onboarding, songs.js ->
settings.js's window._confirmDialog, badges.js -> songs.js's
window.displayTuningName, ...). Every one of them resolves today only because
all boots happen at DOMContentLoaded. So the guards now treat 'interactive' as
not-ready (`!== 'complete'`), restoring that exactly.
Codex's specific finding (first-run onboarding silently skipped) did NOT
reproduce — shell.js's boot() awaits /api/profile, and that yield lets the
remaining deferred scripts run first. But the race it described is real, the
guard is silent when it fails (`&& window.v3Onboarding`), and the other 42
forward refs have no such await protecting them. Fixed at the root rather than
at the one site.
VERIFIED. A/B against origin/main on a fresh profile, 13 probes (onboarding
overlay, v3Onboarding/v3Songs/v3Profile/fbNotify/v3Badges/uiPrompt/showScreen,
bus, capabilities.version, createHighway, plugin scripts, mounted screens):
IDENTICAL, zero console/page errors on both. pytest 2396, node 1028/1028,
ESLint 0 errors, Codex 0.
New guard: test_every_external_script_defers_so_document_order_is_execution_order
fails if any external tag is plain classic — verified to fail on a single
reverted tag, so it actually bites.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
9d0bf95716 |
refactor(ui)!: remove the classic v2 shell — v3 is the only UI (R3a) (#871)
* refactor(ui)!: remove the classic v2 shell — v3 is the only UI (R3a)
Deletes `static/index.html`, the `/v2` route, and the `FEEDBACK_UI` v2/legacy
opt-out. `/` and `/v3` both serve `static/v3/index.html`, which has been the
default since 0.3.0.
This is step 0 of the core-frontend ES-module migration (R3a). Both shells load
the same `static/app.js`, so every later step of that migration — exposing the
window contract, the `defer` ordering fix, the `type="module"` flips — would
otherwise have to be made and verified twice. Removing the fallback now halves
that surface before any of it is touched.
Incidentally fixes a latent bug in `index()`: its guard read
`if getenv_compat("FEEDBACK_UI") or getenv_compat("FEEDBACK_UI") in ("v2", "legacy")`,
whose left operand is truthy for *any* non-empty value — so `FEEDBACK_UI=v3`
actually served the **v2** shell.
- `static/tailwind.min.css` regenerated: the content globs scanned the deleted
file, so v2-only utility classes are now purged (CI's tailwind-fresh job
rebuilds and diffs it).
- Constitution amended to 1.3.0 — Principle II's frontend file list now names
`static/v3/index.html`.
- Tests: 4 suites read the v2 shell (3 via a constructed `path.join` that a
literal grep misses). Their v2 halves are paired duplicates of v3 tests that
stay, so they are dropped; `alpha_warning_banner` and the capability-registry
script-order test retarget to `static/v3/index.html`.
BREAKING CHANGE: `FEEDBACK_UI=v2` / `=legacy` and the `/v2` route are gone.
Unset the variable and use `/`. No chart, settings, or plugin data changes, and
no plugin API changes — v3 reuses the same engine.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs: drop the stale '/ is v2' plugin-verification guidance (CodeRabbit)
The v3-only rewrite updated the intro paragraphs but left three lines that
still instructed plugin authors to verify in 'both / (v2) and /v3' — now the
same shell. Historical 'in v2 it was X' contrasts are kept: they still orient
authors whose plugins also ship to users on older cores.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
5e30138c87 |
refactor(server): extract the artist routes into routers/artist.py (R3) (#870)
The artist page + external-links payload (/api/artist/{name}/page, /links,
/links/refresh) plus their exclusive helpers (_artist_links_payload,
_artist_links_from_mb, the URL-slot table) move to lib/routers/artist.py. Bodies
verbatim except @app->@router and the seam reads (meta_db->appstate.meta_db,
CONFIG_DIR->appstate.config_dir, _default_settings->appstate.default_settings).
MB link enrichment is reached as enrichment.X; the URL-safety validator is
imported from lib/library_registry.py. No new seams.
server.py: 2,507 -> 2,413 (-94).
Verified: pyflakes clean; route set unchanged (143); full pytest 2395 passed.
eslint 0.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
0547f55844 |
refactor(server): extract the media/file-serving routes into routers/media.py (R3) (#869)
Song audio (/audio/{f}), the local-audio-path resolver (/api/audio-local-path),
and raw sloppak-member serving (/api/sloppak/{f}/file/{rel}) — plus the shared
_resolve_sloppak_local_file helper — move to lib/routers/media.py. Bodies verbatim
except @app->@router and the cache/static path seams (AUDIO_CACHE_DIR->
appstate.audio_cache_dir, STATIC_DIR->appstate.static_dir, SLOPPAK_CACHE_DIR->
appstate.sloppak_cache_dir — all already in the seam). No new slots.
The two test fixtures that redirect STATIC_DIR to a temp dir now also patch
appstate.static_dir (the moved routes read the seam, not server's global).
server.py: 2,638 -> 2,507 (-131).
Verified: pyflakes clean; route set identical (143), all unique-path (no
catch-all, no shadowing); full pytest 2395 passed (the audio-local-path +
sloppak-file-traversal cases). eslint 0. Boot smoke: /audio, /api/sloppak/{}/file
serve 404 for unknown, 0 tracebacks.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
b7624b7e65 |
refactor(server): extract the enrichment route handlers into routers/enrichment.py (R3) (#868)
The 14 /api/enrichment/* routes — status, kick/cancel, per-song state, the Match-Review queue (accept/reject/pick/search/rematch/refresh/states), and the AcoustID fingerprint identify endpoints — plus the route-exclusive candidate sanitizer move to lib/routers/enrichment.py. Bodies verbatim except @app->@router and the seam reads (meta_db->appstate.meta_db, CONFIG_DIR->appstate.config_dir). The enrichment engine (transport, matcher, worker, upload caps) already lives in lib/enrichment.py from the earlier subsystem move and is reached as enrichment.X. No new seams. server.py: 2,925 -> 2,638 (-287). Verified: pyflakes clean; route set identical (143); full pytest 2396 passed (the enrichment route + Match-Review + identify cases, which fake the network on the enrichment module). eslint 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron GamatosandGitHub
|
f00ba2217d |
Revert "feat(audio): loopback feeder mode — all app audio under exclusive/ASIO (#865)" (#867)
This reverts commit
|
||
|
|
f09c4a217f |
refactor(server): extract library + collections routes + the provider registry (R3) (#866)
The library query surface (songs/albums/artists/stats/genres/tuning-names/ practice-suggestions), the provider list/art/sync endpoints, and collection CRUD move to lib/routers/library.py. The registry itself — LibraryProviderRegistry, LocalLibraryProvider, SmartCollectionProvider, the collection-provider lifecycle (_sync/_unregister_collection_provider), and the shared query/collection helpers (_library_filter_args, _split_csv, _sanitize_collection_rules, _safe_art_redirect_url, the filter-key sets) — moves to lib/library_registry.py. The PLUGIN CONTRACT is untouched: server.py still constructs the singleton (LocalLibraryProvider needs meta_db), still exposes register_library_provider / unregister_library_provider to plugins via plugin_context (with the per-plugin ownership scoping in plugins/__init__.py), and injects the registry + local provider into appstate. The router reads appstate.library_providers / appstate.local_library_provider at call time; the provider classes are duck-typed so no plugin imports a base class. Acyclic: library_registry imports routers.art (for LocalLibraryProvider.get_art) + appstate, never server. server.py: 3,692 -> 2,925 (-767). Verified: pyflakes clean; ORDERED route table identical set (library block mounts at one site — all exact/specific-path, no catch-all, no shadowing); full pytest 2397 passed (incl the plugin register/unregister + collection-as-provider tests). eslint 0. Boot smoke: /api/library + providers list "local"; a created collection surfaces as a `collection:N` provider through the seam; collection CRUD clean. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
bbdff4e10f |
refactor(server): extract the song routes into routers/song.py (R3) (#864)
Upload/delete, the catalog-metadata write-back, user-meta, overrides, gap-fill,
and the per-song info payload (11 routes) move to lib/routers/song.py with their
exclusive helpers (the atomic upload commit + the song-IO lock, the upload caps,
the gap-fill proposal builders). Bodies verbatim except @app->@router and the
seam reads: meta_db->appstate.meta_db, art_override_paths->appstate.art_override_paths,
and the scan/ingest helpers that stay in server.py (the scan lifecycle owns them)
-> new appstate seam callables: kick_scan, invalidate_song_caches, stat_for_cache,
and scan_status() (a getter — the underlying dict is reassigned). The gap-fill
MBID/ISRC regexes are reached as enrichment.X; _MULTIPART_OVERHEAD_SLACK (shared
with the staying AcoustID-identify route) moves to lib/enrichment.py beside
_ACOUSTID_MAX_UPLOAD_BYTES.
ROUTE ORDER: song_router mounts AFTER art_router — get_song_info's catch-all
`/api/song/{filename:path}` would otherwise shadow `/api/song/{path}/art*`
(Starlette matches first-registered; the :path converter is greedy).
server.py: 4,478 -> 3,692 (-786).
Verified: pyflakes clean; ORDERED route table preserves the specific-before-catch-all
invariant; full pytest 2397 passed (incl the art/cover 304 + CAA-fetch tests that
caught the shadowing before it was fixed). eslint 0.
BEHAVIORAL — needs an on-device pass (upload a sloppak, edit metadata write-back,
delete a song) before merge.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
7258e1066a |
refactor(server): extract the settings routes into routers/settings.py (R3) (#863)
GET/POST /api/settings, /api/settings/reset, and the two-phase atomic export/import bundle (/api/settings/export|import) move to lib/routers/settings.py with their exclusive helpers (the relpath allowlist validator, the atomic writer, the library-DB snapshot + sqlite integrity gate, the config-type validator, the bundle schema). Bodies verbatim except @app->@router and the seam reads: meta_db->appstate.meta_db, CONFIG_DIR->appstate.config_dir, _running_version->appstate.running_version(), and _default_settings-> appstate.default_settings (the canonical defaults builder stays in server.py — the scan + artist-links code share it — and is injected as a new seam callable). server.py: 5,539 -> 4,478 (-1,061). Verified: pyflakes clean (bar the pre-existing File/safe_join/tuning_name/ET); route table IDENTICAL (143); full pytest 2397 passed (154 settings cases incl the export→import round-trip + library-DB snapshot/restore + relpath-allowlist SSRF/ traversal guards, retargeted onto the settings module). eslint 0. BEHAVIORAL — needs an on-device settings export→import round-trip sign-off before merge (do not merge on green CI alone). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
73127d5416 |
refactor(server): extract the album-art routes into routers/art.py (R3) (#862)
The six song-art routes — GET /api/song/{f}/art, .../art/cover-search,
.../art/candidates, POST .../art/upload, .../art/url, DELETE /api/art/{f}/override
— plus their exclusive helpers (the ETag/304 response machinery, _save_art_override,
_url_host_is_internal, _fetch_art_url + the art size/redirect caps) move to
lib/routers/art.py. Bodies verbatim except @app->@router and the seam reads:
meta_db->appstate.meta_db, ART_CACHE_DIR->appstate.art_cache_dir, and the three
shared art helpers that stay in server.py (used by the song/delete routes too)
-> appstate.<callable> (_song_pack_art_exists, _art_override_paths — already
seam-injected for the enrichment worker — plus a new art_safe_name slot). The
CAA / release-search transport lives in lib/enrichment.py and is reached as
enrichment.X. LocalLibraryProvider.get_art now calls art_router.get_song_art.
server.py: 5,988 -> 5,540 (-448).
Verified: pyflakes clean (bar the pre-existing File/safe_join/tuning_name/ET);
route table IDENTICAL (143); full pytest 2399 passed (33 art serve/candidates/
override/url cases incl the SSRF-guard _url_host_is_internal + _fetch_art_url
size-cap tests, retargeted onto the art module); test_packaging 43; eslint 0.
Boot smoke: /art 404, /art/candidates 404, DELETE /override 200 from the router.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
165475d115 |
refactor(server): move the metadata-enrichment subsystem into lib/enrichment.py (R3) (#861)
* refactor(server): move the metadata-enrichment subsystem into lib/enrichment.py (R3)
MusicBrainz / Cover-Art-Archive / AcoustID transport, the match-scorer glue, and
the background enrichment worker (~930 lines, 61 defs) leave server.py as one
cohesive unit. Bodies are verbatim; the only changes are seam reads:
meta_db / config_dir / sloppak_cache_dir / art_cache_dir -> appstate.<slot>
_song_pack_art_exists / _art_override_paths (stay in server.py for the art +
delete routes) -> appstate.<callable> (new seam slots, injected by reference)
_env_flag -> env_compat.env_flag_compat (the existing identical helper)
_artist_title_from_filename -> imported from metadata_db (its home)
the User-Agent VERSION lookup: Path(__file__).parent ->
Path(__file__).resolve().parents[1] (lib/enrichment.py -> app root)
server.py drives the worker through the module (import enrichment; the routes +
scan lifecycle call enrichment.X). Tests that faked the network on `server`
(_mb_http_get, _enrich_network_enabled, _caa_http_get, ...) now patch the same
names on `enrichment` — the module attribute is resolved at call time, so one
setattr reaches both the routes and the worker's internal callers. Acyclic:
enrichment imports appstate/appconfig/dlc_paths/metadata_db/mb_match/
acoustid_match/sloppak/loosefolder, never server.
server.py: 6,917 -> 5,988 (-929).
Verified: pyflakes clean (bar the pre-existing File/safe_join/tuning_name/ET);
route table IDENTICAL (143); full pytest 2400 passed (140 enrichment/art cases
incl the offline-safety + transport-error-pauses-pass contracts that fake the
network); test_packaging 44 passed (enrichment.py resolves under lib/); eslint 0.
Boot smoke: /api/enrichment/status + POST /kick serve from the new module.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: reset enrichment worker state between tests (CodeRabbit)
lib/enrichment.py now owns the worker, and it stays imported for the whole
session while the `server` fixtures pop-and-reimport `server` — so the cancel
Event / status dict / caches would leak across tests, and a stale `_enrich_cancel`
could short-circuit a later direct `_background_enrich()`. An autouse conftest
fixture clears that process-global state before each test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: tighten the enrichment-reset fixture (CodeRabbit)
Narrow the import guard to ImportError (not blind Exception, BLE001), and stop
clearing _caa_index_locks — it's guarded by _caa_index_locks_guard, so an
unlocked clear() would race a still-alive worker, and its per-release mutexes
carry no test state.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
508829c012 |
refactor(server): extract /api/tunings into routers/tunings.py + lib/appconfig.py (R3) (#858)
ship-ci / ci (push) Waiting to run
The merged-tuning-catalog route moves to lib/routers/tunings.py, verbatim except
@app->@router, CONFIG_DIR->appstate.config_dir, and the two seam substrates it
needed:
- lib/appconfig.py — the pure config.json reader `_load_config` (used by ~11
server sites + future config-reading routers). server.py re-imports it, so
those call sites and any `server._load_config` test reference are unchanged.
- appstate.tuning_providers — the TuningProviderRegistry instance injected by
reference (a stable object mutated in place via register()/unregister()), so
the router reads the same registry plugins populate through plugin_context.
The instance stays defined in server.py, so `server.tuning_providers` still
resolves — zero test retargets.
The tuning constants (DEFAULT_REFERENCE_PITCH, TUNING_PRESET_MIDIS,
freqs_to_midis) already live in lib/tunings.py and are imported directly.
server.py: 6,960 -> 6,917.
Verified: pyflakes clean (bar the pre-existing unused `tuning_name` import);
route table IDENTICAL (143); full pytest 2400 passed (110 tuning/config cases);
eslint 0. Boot smoke: /api/tunings serves referencePitch + tunings + tuningMidis.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
cce95cbd1e |
refactor(server): extract the diagnostics routes into routers/diagnostics.py (R3) (#857)
The three /api/diagnostics/* routes (export, preview, hardware) plus their exclusive payload-cap helpers and the `_diag_*` normalisers. Bodies verbatim except @app->@router, CONFIG_DIR->appstate.config_dir, _running_version()-> appstate.running_version() (a new seam slot; the impl stays in server.py where the settings region also calls it), and the builtin-plugins lookup in _diag_plugins_roots: Path(__file__).parent -> Path(__file__).resolve().parents[2] (routers -> lib -> app root; plugins/ ships at the app root in every packaging path). The pure caps/normalisers (_diag_cap_console/_dict/_contributions, _diag_coerce_bool, _diag_normalize_include, _DIAG_MAX_*) are re-exported from server.py so the existing `server._diag_*` / `server._DIAG_*` tests keep resolving — none of them monkeypatch these, so no test retargets. server.py: 7,216 -> 6,960. Verified: pyflakes clean (bar the intentional re-export lines); route table IDENTICAL (143); full pytest 2399 passed (77 diag/packaging + 122 diagnostic- matched cases incl the cap/coerce/normalize suites); eslint 0; Codex pending. Boot smoke: /hardware, /preview, and POST /export (200 application/zip) all serve from the new router location. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
32ebc7671e |
refactor(server): extract the version route into routers/version.py (R3) (#856)
ship-ci / ci (push) Waiting to run
GET /api/version + its exclusive _safe_http_url URL validator. Bodies verbatim except @app -> @router and the VERSION-file lookup: Path(__file__).parent (the app root when this lived at the top level) -> Path(__file__).resolve().parents[2] (routers -> lib -> app root). VERSION ships at the app root in every packaging path (Dockerfile COPY VERSION /app/, desktop bundle). server.py: 7,275 -> 7,214. Verified: pyflakes clean; route table IDENTICAL (143); pytest 2401 passed (33 in test_version_endpoint, incl the URL-validation + env-override cases); eslint 0. Boot smoke: /api/version returns the real version + validated source/license URLs. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
46f3be7fd7 |
refactor(server): extract XP + per-song stats into routers/stats.py (R3) (#855)
The last of the progression cluster. XP award (1 route) + per-song practice stats
(record/recent/best/top/per-song, 5 routes) — meta_db-only apart from the two
seam accessors record_stats uses (get_progression_content, builtin_diagnostic_
filename, both landed by shop/progression). Bodies verbatim; @app -> @router,
meta_db -> appstate.meta_db, _as_int from metadata_db, _clean_str from reqfields.
Three scattered source blocks (xp, the stats block, and the separated
/api/stats/{filename:path} which the /api/library/practice-suggestions route
splits off) are assembled into one module and mounted once. Registration order
is preserved WHERE IT MATTERS: the /api/stats/{filename:path} catch-all is
assembled LAST inside the router, so it still can't shadow the fixed /recent
/best /top paths — verified against the live route table (recent/best/top all
precede the catch-all) and the route SET is identical to origin/main (143).
server.py: 7,478 -> 7,275.
Verified: pyflakes clean; route set identical + catch-all-last; pytest 2401
passed (113 across song_stats/profile/progression); eslint 0. Boot smoke:
/stats/recent /best /top all 200 (not shadowed), xp/award 200.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
4cc8fa3b4d |
refactor(server): extract the profile routes into routers/profile.py (R3) (#854)
6 routes (get/set profile, bundled+custom avatars, avatar upload/serve, progress) + the exclusive _list_bundled_avatars helper. Bodies verbatim; @app -> @router, meta_db -> appstate.meta_db, CONFIG_DIR/STATIC_DIR -> appstate.config_dir/ static_dir (seam), _clean_str from reqfields, _get_progression_content() -> appstate.get_progression_content(). No STATIC_DIR test retarget: _list_bundled_avatars reads appstate.static_dir but test_profile_api doesn't patch STATIC (only the sloppak/audio/traversal suites do, for handlers that stay in server.py). server.py: 7,594 -> 7,478. Verified: pyflakes clean; route table IDENTICAL (143); pytest 2401 passed (61 in test_profile_api); eslint 0. Boot smoke: GET /api/profile 200 (drives get_progression_content), /avatars lists via appstate.static_dir, /progress 200. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
f9f33320ac |
refactor(server): extract the progression routes into routers/progression.py (R3) (#853)
4 routes (overview/add-paths/onboarding/events, spec 010) + their EXCLUSIVE helpers (_goal_ui_progress, _progression_overview 112L) + the _PROGRESSION_EVENT_TYPES whitelist. Bodies verbatim; @app -> @router, meta_db -> appstate.meta_db, _clean_str from reqfields. The two SHARED server accessors read through the seam: get_progression_content (added for shop #851) and builtin_diagnostic_filename (new slot — a trivial const-returning fn shared with the stats router's api_record_stats). Both are injected via the second appstate.configure() after their defs (the import-top configure runs before them). The cache + fns stay in server.py, so test_progression_api's server._progression_content patch is untouched — 0 retarget. server.py: 7,798 -> 7,594. Verified: pyflakes clean; route table IDENTICAL (143); both seam accessors wired; pytest 2401 passed (63 in test_progression_api); eslint 0. Boot smoke: GET /api/progression 200 (drives _progression_overview + both accessors), events 400 on bad body. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
5f58af4faa |
refactor(server): extract the shop routes + inject get_progression_content into the seam (R3) (#851)
The progression-content substrate: `_get_progression_content` (a lazy, double-checked-locking content cache) is now published into the appstate seam as a CALLABLE. The cache global + lock + the function stay in server.py (startup uses it, and test_progression_api patches `server._progression_content` directly), so ZERO test retargeting — routers just call `appstate.get_progression_content()`. Because the accessor is defined at server.py:1152 but the import-top configure() runs at :346, a second `appstate.configure(get_progression_content=...)` publishes it right after the def (configure is idempotent/additive). First consumer: routers/shop.py (3 routes: buy/equip/list). Bodies verbatim; @app -> @router, meta_db -> appstate.meta_db, _clean_str from reqfields, _get_progression_content() -> appstate.get_progression_content(). This unblocks stats/progression/profile next (all share the accessor). server.py: 7,880 -> 7,845. Verified: pyflakes clean; route table IDENTICAL (143); pytest 2401 passed (test_progression_api's server._progression_content patch still works via the kept cache); packaging guard; eslint 0. Boot smoke: GET /api/shop 200 (drives appstate.get_progression_content), buy 400 on bad body. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
ea8834862d |
refactor(server): batch the small meta_db user-state endpoints into routers/library_extras.py (R3) (#850)
work keeper-chart prefs (3) + favorites toggle + tags list + saved toggle + session/continue — 7 routes across 5 domains, all meta_db-only (0 setattr, 0 helpers). Bodies verbatim; @app -> @router, meta_db -> appstate.meta_db, _clean_str from reqfields. These were scattered singletons with no natural neighbor, so they're grouped as "small library/user-state endpoints" and mounted once. All paths are distinct and non-overlapping, so registering them together doesn't change routing: verified the route SET is identical to origin/main (143) AND that no moved path shadows or is shadowed by another (order-independence check). server.py: 7,880 -> 7,833. Verified: pyflakes clean; route set identical + order-independent; pytest 2401 passed; packaging guard 45; eslint 0. Boot smoke: tags/session GET 200, favorites/saved toggle (400 on missing filename), work/charts 200. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
2281cac438 |
refactor(highway): lift 79 per-instance closure vars into hwState (R3c H lift) (#849)
* refactor(highway): lift 79 per-instance closure vars into `hwState` (R3c H lift) Collapses createHighway()'s 79 mutable closure `let`s into one per-instance `hwState` object. Scope-resolved rewrite via acorn + eslint-scope: 1059 edits (1057 references + 79 defs - the deleted `let canvas, ctx, ws`), with the four names shadowed in inner scopes (chartTime/ctx/notes/chordTemplates) resolved correctly so only closure-bound refs move. Enables the later module split: extracted renderer/ws modules close over `hwState` as a factory arg, so multi-panel plugins (highway_3d, note_detect, splitscreen) don't share one highway's state. Container is `hwState`, NOT `H` — `H` is already canvas height (70 uses). The frame-time gate caught that collision instantly (0 draws, `H._drawHooks is not iterable` in the shared draw-hook path). PERF (the whole risk): identical to the pre-lift baseline. Draw p50 2.1-2.2 ms, p95 2.7-3.0 ms (pre-lift 2.7-3.2), measured on the Arcturus feedpak, headless. Each closure-slot read became a `hwState.<slot>` monomorphic property load; the hot loop pays nothing. On-device: Byron confirmed the 2D highway plays smoothly. Tests: the ~30 highway JS suites brace-extract functions/patterns from the source; their state references + the monotonic-clock vm sandbox now use `hwState.<slot>` (the const _CHART_MAX_INTERP_MS etc. stay top-level, not lifted). node --test: 1030/1030 green. Two self-inflicted over-replacements caught and reverted (`_lefty` is a prefix of the 3D-local `_leftyCached`; `STRING_COLORS` a suffix of `DEFAULT_STRING_COLORS`) — substring replaces on the brace-extract regexes need word care. Transformer saved at ~/.local/share/feedback-editor/highway-h-lift.mjs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(highway): pin the setNoteStateProvider assertion to hwState._noteStateProvider (CodeRabbit) The [^}]* form matched an unqualified _noteStateProvider =, so a regression to closure-level state could still pass. Require the hwState-qualified assignment. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
b6098e3695 |
perf(harness): add 2D-highway frame-time measurement + capture the R3c gate (H0) (#848)
* perf(harness): add 2D-highway frame-time measurement + capture the R3c gate (H0) scripts/perf-baseline.mjs gains a `--song` mode: it wraps requestAnimationFrame before any page script, TAGS the frames the highway actually painted (via highway.addDrawHook), starts playback, and reports draw-frame p50/p95/p99 over `--frames` seconds across `--runs` runs. Tagging is the point — ~half the rAF callbacks are other cheap loops (~0.1 ms); averaging them in would hide a renderer regression, so only draw frames are counted. This is the metric the plan says gates the highway.js split (R3c) but the harness never measured (it did server latency + boot + heap only, and the R0 numbers were against an empty library). docs/perf-baseline.md now records the pre-lift baseline on the Arcturus feedpak: p50 ~2.2 ms, p95 spread 2.7-3.2 ms across 3 runs. The H-container lift (next) re-runs this on the same box and must stay within noise. Maintainer/CI-only tooling; the existing no-`--song` run is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(harness): close page on error + MD040 fence + CHANGELOG (CodeRabbit) - frameTimeOnce now closes its page in a finally, so a failing run doesn't leak the page until the final browser.close() (CodeRabbit). - fenced code block gets a bash language hint (MD040). - CHANGELOG mentions the new --song frame-time mode. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
cbc65458e3 |
refactor(server): extract the wishlist routes into routers/wanted.py (R3) (#847)
Free after _clean_str moved to lib (#841): wanted's deps are JSONResponse, _clean_str (reqfields), app + meta_db (seam). 3 routes (list/add/remove), 0 setattr targets, 0 helpers to relocate. Bodies verbatim; @app -> @router, meta_db -> appstate.meta_db. include_router at the original site; 143-route table identical. No test retargeting. server.py: 8,003 -> 7,974 (this branch is independent of the chart PR). Verified: pyflakes clean; route table identical; pytest 2401 passed (52 in test_wanted_api); eslint 0. Boot smoke: add wishlist entry -> list -> delete, 400 on missing artist+title (_clean_str path). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
7c87538d6b |
refactor(server): extract the chart routes into routers/chart.py (R3) (#846)
Unblocked by the DLC-path substrate (#843): chart's only server-module deps are now app + meta_db (both seam); _get_dlc_dir/_resolve_dlc_path come from dlc_paths, sloppak/loose detection from the shared lib modules. 4 routes (split/unsplit/ work/fileinfo), meta_db-only otherwise. 0 setattr targets, 0 helpers to relocate. Bodies verbatim; @app -> @router, meta_db -> appstate.meta_db. include_router at the original site; 143-route table identical to origin/main. No test retargeting. server.py: 8,003 -> 7,909. Verified: pyflakes clean on the router; no new undefined/dead in server.py; route table identical; pytest 2401 passed (74 across work_charts/context_menu/ group_filter/packaging); eslint 0. Boot smoke: chart/work 200, chart/fileinfo resolves the real pack path through _resolve_dlc_path. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
c8701991cb |
fix(ws_highway): swallow a mid-stream WebSocketDisconnect quietly (#845)
* fix(ws_highway): swallow a mid-stream WebSocketDisconnect quietly Pre-existing (byte-identical to origin/main), flagged by CodeRabbit on #844: the outer try's only guard was `except Exception as e: log.exception("highway_ws unhandled error")`. The inner `except WebSocketDisconnect` covers ONLY the post-`ready` keep-alive loop, and the drum_tab/notation sends have their own localized guards — but the ~13 other streamed sends (beats, sections, notes, chords, anchors, …) fall through to the blanket handler. Since WebSocketDisconnect is an Exception subclass, a routine tab-close mid-load was logged as an error at whichever send was in flight. Fix: a dedicated `except WebSocketDisconnect: return` before the blanket handler, matching the two localized guards and the lib/ coding guideline. tests/test_ws_highway_disconnect.py drives highway_ws with a fake websocket that raises WebSocketDisconnect on the first streamed send (`loading`), over a real minimal sloppak. Negative-checked: removing the guard makes it fail (the disconnect is logged as `highway_ws unhandled error`); the fix passes. Uses a raw handler on `feedBack.server` rather than caplog, since configure_logging() reroutes that logger through structlog where caplog doesn't observe it. Full suite green. Closes the review thread on #844. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ws_highway): assert streaming stops after disconnect (exactly one send) — CodeRabbit The stub now raises only on the first send and the test asserts ws.sends == 1, so a handler that caught the disconnect and kept streaming would fail. Still negative-checked: removing the guard fails the test. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
514461167e |
refactor(server): extract the highway WebSocket into routers/ws_highway.py (R3) (#844)
The single largest handler in server.py — the 902-line /ws/highway/{filename}
chart streamer — plus its 3 exclusive helpers (_pick_smart_arrangement,
_sanitize_authors, _sanitized_song_offset). server.py: 9,008 -> 8,003 (-1,005,
the biggest single R3 cut).
Clean move despite the size: the handler's only server-module deps are the 4
path constants + 3 exclusive helpers + app + log. Everything else it uses is
either NESTED inside the handler (_evict_audio_cache, _fill_scale_degree,
_manifest_entries, _tone_names, _xml_rank, _send_keepalives) or imported from
the shared lib modules (song/audio/sloppak/drums/notation/dlc_paths/metadata_db).
- Path constants read through the appstate seam: added static_dir /
sloppak_cache_dir / audio_cache_dir slots (config_dir already there);
server.py configures them. Bodies otherwise verbatim (@app.websocket ->
@router.websocket, PATHS -> appstate.*, log -> module logger).
- sloppak_cache_dir IS setattr-patched, so the 3 test_highway_ws_* suites now
also `setattr(appstate, "sloppak_cache_dir", ...)` next to their existing
server patch. _sanitize_authors unit tests import it from routers.ws_highway
(it moved). No other test churn.
- Removed 23 now-dead imports from server.py (song/audio/drums/notation/
bisect/contextvars/structlog/WebSocket*/_arr_smart_sort_key) — diffed against
the origin/main unused-import baseline so only NEWLY-dead ones went.
owns_tmp (assigned, never read) moved verbatim — it's pre-existing dead on
origin/main too; left as-is to keep the move faithful.
Verified: route table identical to origin/main (143, paths/methods/order);
handler body verbatim spot-checked; pyflakes clean (server has no new
undefined/dead); pytest 2400 passed; packaging guard 53; eslint 0. Boot smoke:
the highway WS streams the full chart (song_info/beats/sections/notes/chords/
notation/anchors/drum_tab -> ready) BYTE-for-byte the same message sequence as
origin/main across arrangements 0/1/2, zero tracebacks.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
0dcc9136b6 |
refactor(server): move DLC path resolution to lib/dlc_paths.py (R3 substrate) (#843)
The keystone for the path-dependent routers (ws_highway, song, audio-local-path, sloppak): the "where do the song files live + safe containment" layer leaves server.py so a router can reach it. - `_resolve_dlc_path` (pure — args only) moves verbatim. - `_get_dlc_dir` reads the env-derived paths through the appstate seam (appstate.dlc_dir/dlc_dir_env/config_dir) instead of module globals, so lib/dlc_paths.py does no import-time IO. - appstate gains `dlc_dir`/`dlc_dir_env` slots (config_dir already there); server.py configures them. All three are env-derived, so a setenv+reimport fixture reconfigures them for free — ZERO setattr retargeting. - server.py RE-EXPORTS both (`from dlc_paths import _get_dlc_dir, _resolve_dlc_path`), so its 24+16 call sites AND the tests that reach `server._get_dlc_dir()` / `server._resolve_dlc_path()` directly (test_dlc_junction, test_highway_ws_*) resolve unchanged — no test edits. server.py: 9,085 -> 9,008. Verified: _resolve_dlc_path byte-identical to origin/main; _get_dlc_dir's only change is the three path identifiers -> appstate.*; pyflakes clean; route table identical (143); pytest 2407 passed (test_dlc_junction + both highway_ws suites green via re-export); packaging guard 52; eslint 0. Boot smoke: library scans (8 songs, _get_dlc_dir), a real song resolves + serves art (_resolve_dlc_path), and the highway WS reaches `ready` with notes. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
6da01c55a4 |
fix(playlists): split cover decode (400) from persist (500), unique temp, existence re-check (#842)
Two pre-existing cover-upload issues CodeRabbit flagged on #841 (verbatim move, so correctly not fixed there): 1. One `except Exception as e` wrapped BOTH the PIL decode and the img.save/ tmp.replace, returned 400 for both, and echoed `e` — so a disk/permission failure was mislabeled a client error and could leak a filesystem path. Now: decode/validation -> 400 "Invalid image" (generic); save/replace failure -> logged 500 "could not save cover" (no detail). 2. A shared `{pid}.png.tmp` let two concurrent uploads clobber each other's temp file. Now a unique `tempfile.mkstemp` in the cover dir, atomic replace to publish. Plus an existence re-check just before publishing so an upload that raced a playlist delete can't leave an orphan cover. mkstemp itself is INSIDE the try (Codex catch): an unwritable dir / full disk raises there and is the same persistence failure as save/replace, so it hits the logged generic-500 path instead of escaping as an unhandled 500. Cleanup guards `tmp is not None` for the mkstemp-failed case. Did NOT add a full per-playlist critical section (CodeRabbit's "heavy lift"): FeedBack is single-user (Principle I), so a cover upload racing a delete on the same id can't happen — documented in the code rather than building a lock framework for a precluded race. tests/test_playlist_cover_errors.py pins all of it; negative-checked three ways: the old single-except-400 shape fails the 500 + no-leak-400 tests, and moving mkstemp back outside the try fails the temp-creation-500 test. Fix passes 5/5. Full suite 2405 passed; boot smoke: valid cover 200, bad image -> generic "Invalid image" 400, no .tmp litter. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
a883f9213f |
refactor(server): extract the playlists routes into routers/playlists.py (R3) (#841)
The biggest router yet — 12 playlist routes + custom covers — and the first that needs the config-path seam. server.py: 9,302 -> 9,085 (-217). Three causally-linked pieces, all required for playlists: - `config_dir` joins the appstate seam (the plan always put the path constants there; deferred in S3, needed now). It's env-derived, so the ~49 pop-and-reimport fixtures reconfigure it for free — ZERO setattr retargeting. STATIC_DIR/SLOPPAK_CACHE_DIR (patched via setattr) stay in server.py until a router that reads them is extracted, and get retargeted then. - `_clean_str` (pure request-field sanitizer, 14 callers) -> lib/reqfields.py; server.py imports it back. Unblocks wanted/saved/collections/profile/... later. - routers/playlists.py: bodies verbatim, `@app`->`@router`, `meta_db`-> `appstate.meta_db`, `CONFIG_DIR`->`appstate.config_dir`, `_clean_str` from reqfields, `_ART_CACHE_HEADERS` as a local const (art keeps server.py's). The two exclusive cover helpers (_playlist_cover_path/_url) move with it. include_router at the original site; full 143-route table identical to origin/main. One test retarget: test_playlists_api called `server._playlist_cover_path` directly -> now imports it from routers.playlists (reads appstate.config_dir, which the `server` fixture configures). Verified: pyflakes clean; route table identical; pytest 2401 passed (28 in playlists+collections+appstate); packaging guard 51 (auto-picked up reqfields); eslint 0; boot smoke drives create/rename/add-song/cover-upload/serve/delete — the cover writes 1.png under CONFIG_DIR THROUGH appstate.config_dir and serves 200 with an mtime cache-bust token; a wrong-typed name field still 400s via _clean_str; demo untouched. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
4fd0cd49e7 |
fix(loops): take the DB lock for the count+insert and the list read (#840)
Two pre-existing races in the loops routes, flagged by CodeRabbit on #839 (the verbatim extraction moved them unchanged from server.py, so they correctly weren't fixed there): 1. save_loop computed `COUNT(*)` OUTSIDE meta_db._lock, then inserted inside it. Two simultaneous unnamed POSTs read the same count and both mint "Loop N". Fix: one lock scope around COUNT + INSERT. 2. list_loops read the shared single connection (check_same_thread=False) with no lock, so it could overlap a POST/DELETE commit. Fix: read under the lock, like every writer. Low severity in context — FeedBack is single-user (Principle I), so concurrent unnamed-loop POSTs essentially can't happen — but each fix is one lock scope. tests/test_loops_concurrency.py pins both with a threading.Barrier that releases 16 workers into save_loop at once. Negative-checked: reverting the COUNT back outside the lock fails the uniqueness assertion 5/5 runs; the fix passes 3/3. pytest 2400 passed; on-device two unnamed POSTs -> ['Loop 1','Loop 2']. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
b41361eb1b |
refactor(server): extract the loops routes into routers/loops.py (R3) (#839)
Third router. Practice loops (saved A/B regions per song): GET/POST/DELETE /api/loops, meta_db-only (0 setattr targets, 0 helpers to relocate per router_scan.py). Bodies verbatim; @app -> @router, meta_db -> appstate.meta_db. include_router at the original site; 143-route table identical to origin/main. server.py: 9,337 -> 9,301. No test retargeting (test_demo_mode only names the paths in middleware regexes). Verified: pyflakes clean; route table identical; pytest 2398 passed; packaging guard green; eslint 0; boot smoke drives POST (auto-names "Loop N") / GET / DELETE / missing-fields error, and demo mode 403s both writes while allowing the read. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
6c98aba433 |
refactor(server): extract the artist-alias routes into routers/artist_aliases.py (R3) (#838)
* refactor(server): extract the artist-alias routes into routers/artist_aliases.py (R3) Second router, picked by router_scan.py: the artist-aliases / Tidy-up (P4) group ranks at 0 monkeypatch.setattr targets and 0 helpers to relocate. 5 routes (list/set/merge/delete aliases + raw-artist picker), all meta_db-only. Bodies verbatim; only @app.<m> -> @router.<m> and meta_db -> appstate.meta_db. include_router mounts at the original site; full 143-route table identical to origin/main (paths, methods, order). No test retargets: test_artist_alias drives via TestClient(server.app) + server.meta_db, neither of which moved. Verified: pyflakes clean on the router; no new undefined in server.py; JSONResponse still used in server.py (not dead); pytest 2398 passed (18 in test_artist_alias); packaging guard green; eslint 0; boot smoke drives all 5 routes end-to-end (set ACDC->AC/DC, read back, 400 on missing fields, delete). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: credit both router extractions in the size-exemptions rationale (CodeRabbit) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
b3215694e7 |
fix(build): move appstate.py + routers/ under lib/ so the desktop app ships them (#836)
The packaged desktop app died at startup:
File ".../Resources/slopsmith/server.py", line 71, in <module>
import appstate
ModuleNotFoundError: No module named 'appstate'
feedback-desktop's scripts/bundle-slopsmith.sh copies a HARDCODED list of core
files into the bundle -- server.py, VERSION, lib/, data/, static/,
plugins/__init__.py. The root-level appstate.py (#833) and routers/ (#834)
shipped fine in Docker, passed every test, and were silently dropped from the
packaged app.
Both now live under lib/, the one core directory every packaging path already
copies wholesale -- Dockerfile `COPY lib/`, docker-compose.yml, and the desktop
bundler's `cp -r lib` -- and that all three put on sys.path (on Windows via the
embeddable-Python ._pth, where PYTHONPATH is ignored: build-windows.sh writes
`../slopsmith` and `../slopsmith/lib`). No feedback-desktop change and no new
release are needed for this to take effect.
lib/ is also the CORRECT home under Principle V, and always was once the design
settled: with the injection seam, appstate.py constructs nothing and does no
import-time IO, and a route module only builds an APIRouter. The premise that
forced root placement -- "appstate opens sqlite at import" -- stopped being true
when configure() replaced ownership. The Dockerfile / .dockerignore /
docker-compose.yml entries added for the root layout are reverted; nothing else
in core changes (git mv, so --follow survives).
tests/test_packaging.py is the guard: it walks server.py's module-level imports,
keeps the ones resolving inside this repo, and fails if any lives outside a
directory the packagers copy -- with the ModuleNotFoundError spelled out. So the
next root-level core module can't ship broken. Negative-checked: restoring
appstate.py to the root fails it; the message names the file and the four
packaging files a root module would have to teach.
(It also has to skip `origin in {"built-in","frozen"}` -- on 3.14 the frozen
stdlib reports origin="frozen", and Path("frozen").resolve() lands inside the
repo, which flagged `os` and `stat` as first-party.)
Verified: the bundler's copy replicated exactly into a temp dir and booted with
PYTHONPATH=<bundle>:<bundle>/lib -- `import server`, `import appstate`,
`import routers.audio_effects` all resolve, appstate.meta_db is server.meta_db,
143 routes. The same simulation against origin/main reproduces the production
ModuleNotFoundError. pytest 2398 passed (2348 + 50 new); route table still
identical to origin/main (paths, methods, order); docker build context reaches
lib/appstate.py and lib/routers/ with no __pycache__; native uvicorn boot smoke
serves /api/version, /api/library, the moved audio-effects router, and all three
migrated plugins' src/ graphs; eslint 0 errors.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
ebe59d3f97 |
refactor(server): extract the audio-effects routes into routers/audio_effects.py (R3) (#834)
ship-ci / ci (push) Waiting to run
The first route module through the appstate seam (#833). Picked BY MEASUREMENT, not by the plan's guess: a transitive dep-closure scan over every route group ranked audio-effects at 0 monkeypatch.setattr targets and exactly one exclusive helper. (The same scan disproved the plan's assumption that artists/aliases was free -- api_artist_links reaches _mb_http_get and _enrich_network_enabled, both setattr targets.) Bodies are verbatim. The only edits are mechanical: @app.get(...) -> @router.get(...) audio_effect_mappings.x -> appstate.audio_effect_mappings.x The singleton read must stay a module attribute resolved at call time, so a re-imported server re-publishes a fresh DB into the seam and monkeypatch reaches this module. `routers/` never imports `server`: server -> routers -> appstate. `app.include_router(...)` sits exactly where the routes used to be defined -- FastAPI matches in registration order, so the mount site preserves it. Verified by diffing the FULL route table against origin/main: 143 routes, identical paths, methods AND order. server.py: 9,445 -> 9,386 lines. `fastapi.Query` went dead with the move and was removed (the other four unused imports are pre-existing on main). Packaging: COPY routers/ /app/routers/ plus `!routers/` + `!routers/**` in .dockerignore (that file opens with a blanket `*`). Verified against the real docker daemon: routers/ reaches the build context, __pycache__ does not. Verified: pyflakes clean on routers/; no new undefined name in server.py; pytest 2348 passed (75 in the audio-effects + demo-mode suites); eslint 0 errors; boot smoke drives all five routes end-to-end (create -> read back -> activate -> clear -> delete -> 404 on missing -> 400 on bad body), Query(...) still 422s on a missing required param, and demo mode still 403s all four moved write routes while allowing the read. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
d6f2df14f7 |
feat(server): add appstate.py, the router seam (R3) (#833)
* feat(server): add appstate.py, the router seam (R3)
Routes moving out of server.py need `meta_db` and friends, but must not
`import server` -- that goes circular the moment server imports them back.
server.py keeps CONSTRUCTING its singletons and now injects them once via
`appstate.configure(...)`; routers read them back as module attributes at call
time (`import appstate; appstate.meta_db`). The Python analogue of the frontend
refactor's `configureX({...})` seams and of the plugin `setup(app, context)`
contract: dependencies flow one way, server -> routers -> appstate.
Two properties are load-bearing, both pinned by tests/test_appstate.py:
1. `import appstate` constructs nothing and touches no disk. This is why the
~49 test fixtures that `sys.modules.pop("server")` + re-import (to rebuild
meta_db under a patched CONFIG_DIR) keep working UNTOUCHED. A singleton
owned by appstate would survive that pop and go stale -- verified.
2. Reads must be late-bound. `from appstate import meta_db` freezes the binding
and defeats both a later configure() and monkeypatch.setattr -- the same
read-only-binding trap as ES imports.
configure() raises on an unknown slot instead of silently creating a global
nothing reads, and the suite asserts server ACTUALLY calls it. Negative-checked:
dropping the configure() call fails exactly the two wiring tests while the other
five stay green -- those five are the false-green a seam test must not be.
The new suite imports server through an `isolated_server` fixture that patches
CONFIG_DIR to tmp_path and closes both DB connections on teardown. An unguarded
`import server` constructs MetadataDB + AudioEffectsMappingDB under the real
`~/.local/share/feedback` (reproduced: running the file alone created
web_library.db + audio_effects.db there). The full suite now leaves the real
config dir untouched.
Packaging: `COPY appstate.py /app/` plus a .dockerignore allowlist entry. That
file opens with a blanket `*` exclusion, so root-level Python must be re-allowed
explicitly -- without it the image build fails on the COPY. Verified against the
real docker daemon (build context reaches /app/appstate.py). docker-compose.yml
gains the dev bind-mount; docker-compose.nas.yml runs the baked image, so the
COPY covers it. `routers/` will need the same two entries when it lands.
Verified: pyflakes clean; pytest 2348 passed (2341 + 7 new); eslint 0 errors;
boot smoke serves /api/version, /api/library, /api/audio-effects/mappings, and
all three migrated plugins' src/ graphs, with `appstate.meta_db is server.meta_db`
asserted against the live import.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(appstate): address CodeRabbit — restore slots on teardown, really re-import
Two real findings on #833, both fixed:
(1) `isolated_server` closed server's DB connections but left `appstate.meta_db`
and `appstate.audio_effect_mappings` published and pointing at the closed
handles -- a live-looking, dead singleton for any later test. Teardown now
snapshots and restores both slots.
(2) `test_reimporting_server_republishes_the_fresh_singletons` never performed a
second import: it only re-asserted what `test_server_wires_the_seam` already
covers, so it could not detect the very staleness it names. (I introduced
that regression while fixing Codex's CONFIG_DIR isolation finding.) It now
pops `server`, re-imports under a SECOND CONFIG_DIR, and asserts the seam
republishes -- `second_server.meta_db is not first_db` and
`appstate.meta_db is second_server.meta_db`.
Negative-checked both directions: simulating an appstate-OWNED singleton
(configure() only-first-wins) now fails the re-import test, and dropping
server's configure() call still fails exactly the two wiring tests.
NB CodeRabbit's committable suggestion inserted the snapshot above the
fixture docstring, which would have demoted it from __doc__; written by hand
instead.
pytest 2348 passed; the full suite leaves the real ~/.local/share/feedback
untouched.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
94a58b7a42 |
refactor(server): extract AudioEffectsMappingDB into lib/audio_effects_db.py (R3) (#831)
Move-only, same shape as the MetadataDB extraction. The core-owned song/tone -> audio-effect-provider routing index leaves server.py for a flat lib/ module. The class body is byte-identical; server.py reconstructs exactly from origin/main minus the cut range plus the import-back and the call site. server.py: 9,705 -> 9,433 lines. The only non-verbatim change is the constructor seam: `__init__` takes `config_dir` instead of reading the module-level CONFIG_DIR, so the module does no IO at import (Principle V). The `audio_effect_mappings` singleton stays in server.py -- no route, no test, and none of the `monkeypatch.setattr(server, ...)` targets move. No import went dead. Verified: pyflakes clean on the new module; no new undefined name in server.py; pytest 2341 passed; eslint 0 errors; boot smoke drives the extracted DB end-to-end (POST a mapping -> GET reads it back -> audio_effects.db lands in CONFIG_DIR, proving the config_dir seam) and all three migrated plugins still serve their src/ module graphs. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
58120745bc |
refactor(server): extract MetadataDB into lib/metadata_db.py (R3) (#830)
Move-only. The library metadata cache -- the `MetadataDB` class (4,018 lines) plus the query helpers it owns (keyset paging cursors, the tuning grouping key, smart-arrangement naming, tag normalisation, the startup DB-restore swap) -- moves out of server.py into a flat `lib/` module. Every moved block is byte-identical to its server.py original; server.py is exactly origin/main minus the six cut ranges, minus the now-dead `import contextlib`, plus the import-back block and the constructor call site. server.py: 14,037 -> 9,705 lines. The one non-verbatim change is the seam that lets the class leave server.py: `MetadataDB.__init__` now takes `config_dir` explicitly instead of reading the module-level CONFIG_DIR, so `lib/metadata_db.py` does no IO at import (Principle V). The `meta_db` singleton stays in server.py, so `server.meta_db` (282 refs) and `server.app` (67 refs) resolve unchanged and no route moves. None of the 114 `monkeypatch.setattr(server, ...)` targets moved. Logging still goes through the `feedBack.server` logger, so log filters and caplog assertions resolve to the same logger object. `tests/test_settings_export_library_db.py` imports `_apply_pending_db_restore` from metadata_db (the test moves with its subject); no other test changed. Verified: pyflakes clean on the new module (zero undefined names, zero unused imports) and no new undefined name in server.py; pytest 2341 passed; node --test 1030 passed; eslint 0 errors; uvicorn boot smoke serves /api/version, /api/library, and all three migrated plugins' src/ module graphs (stems, studio, editor -> 200). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron GamatosandGitHub
|
950e348357 |
R0: module-migration rails (src/ serving, live-edit cache, scriptType loading, governance) (#812)
ship-ci / ci (push) Has been cancelled
Host enablement for the plugin ES-module migration: sandboxed /api/plugins/{id}/src/ serving, no-cache+weak-ETag/304 live-edit caching on src/+screen.js+assets, scriptType:module loader injection + scriptType/minHost manifest passthrough; constitution v1.2.0 + module playbook + signed size-exemptions register + maintainer/CI-only ESLint gate; rerunnable perf-baseline harness. Reviewed by Codex (local), Copilot, and CodeRabbit.
|
||
|
|
115c3529e9 |
fix(midi): guard non-positive division in the legacy inline tempo path (#805)
ship-ci / ci (push) Waiting to run
convert_midi_track_to_keys_wire builds its own inline tempo map and divides by the raw midi.ticks_per_beat at two sites (the tempo-table precompute and the tick_to_seconds closure). A malformed header with division == 0 raised ZeroDivisionError, and an SMPTE division (which mido returns as a NEGATIVE signed short) produced negative/garbage note times. Guard the divisor with `ticks_per_beat if ticks_per_beat > 0 else 480` so both the zero and negative cases fall back to the SMF default. The `> 0` form (not `or 480`) is required because a negative value is truthy and would slip past `or`. Positive-division behavior is unchanged. Follow-up to #796, which fixed the same class of bug in the newer convert_midi_tempo_map / _build_tick_to_seconds path. Adds two focused tests: division == 0 no longer crashes and emits a non-negative time, and a negative/SMPTE division yields sane non-negative times. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron GamatosandGitHub
|
010edc239b |
Merge pull request #806 from got-feedBack/fix/tuner-inject-player-button-render
fix(tuner): anchor injectPlayerButton to a direct-child button (feedBack#800) |
||
|
byrongamatosandClaude Opus 4.8
|
9fb63fd3b5 |
fix(tuner): anchor injectPlayerButton to a direct-child button (feedBack#800)
`injectPlayerButton()` anchored the injected Tuner button with
`controls.querySelector('button:last-child')`, which can match a NESTED
button that is not a direct child of `#player-controls`. `insertBefore(btn,
nestedButton)` then throws `NotFoundError` (the reference node must be a
direct child); since injection runs from the tuner's `screen:changed`
handler, the throw propagated out of the player-screen transition and
stalled its render. The v3 path was already safe (plugin-control slot);
only the classic anchor was bad.
Use `:scope > button:last-of-type` (direct child only) with a
`parentNode === controls` guard before insertBefore, falling back to
appendChild. Bump plugins/tuner 1.3.3 → 1.3.4.
Test: tests/plugins/tuner/js/inject_player_button.test.js — extracts the
real function and runs it over a faithful DOM model whose insertBefore
enforces the direct-child invariant; the nested-last-button case reproduces
the throw on the old anchor and passes on the new one (5 tests).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: byrongamatos <xasiklas@gmail.com>
|
||
|
|
69c8ad4e0c |
fix(settings): don't let a stale DLC path block saving the Demucs server address (#795)
ship-ci / ci (push) Waiting to run
The v3 Settings "Save" button posts dlc_dir together with demucs_server_url, default_arrangement and av_offset_ms in one request. POST /api/settings validated dlc_dir first and early-returned "DLC directory not found" before it ever processed demucs_server_url, so on a machine whose DLC path doesn't resolve (fresh install, unplugged/network drive, a path carried over from another machine) setting the Demucs server address silently failed — reported in got-feedBack/feedBack-demucs-server#3 (macOS 07-05 nightly). - server: a non-resolving dlc_dir is now recorded as a warning and skipped rather than aborting the whole POST, so the co-submitted keys still persist. The bad path is surfaced via a new additive `warnings` field and folded into `message` so the settings status line still shows it. - client (v3): the Demucs input now autosaves on blur/enter via a single-key persistSetting POST, like every other v3 setting, so it never depends on the coupled Save button. - tests: cover the decoupling and the unchanged happy path. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
1a8540935b |
fix(gp_autosync): slope-constrained DTW steps — stop path collapse on riff-based songs (#791)
librosa.sequence.dtw's default step sizes permit pure horizontal/vertical moves; on songs whose chroma is self-similar for long stretches the flat cost surface let the path collapse (minutes of score onto one audio frame), so auto-sync produced monotonic-but-garbage sync points and the per-bar warp imported charts badly out of sync while reporting success. Use the standard music-sync step pattern [[1,1],[1,2],[2,1]] (local tempo ratio bounded to 0.5x-2x), falling back to unconstrained steps if the global length ratio makes it infeasible. Validated on the reported song (138 BPM tab, YouTube audio): coarse points now track 1:1, refine holds slopes 0.77-1.04, warped downbeats hit onset peaks at 3.3x background energy. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
92dc321fdf |
feat(gp_autosync): piecewise time-warp helpers + refine_sync onset pass (#787)
* feat(gp_autosync): piecewise time-warp helpers + refine_sync onset pass auto_sync computes per-bar sync points but consumers only ever applied the scalar bar-1 audio_offset, so any tempo drift between the recording and the tab's authored tempo accumulated over the song. Add the librosa-free helpers needed to apply the full piecewise mapping: - bar_start_times(gp_path): per-bar score times sharing auto_sync's axis (GPIF bar-resolution map, GP3/4/5 per-tick integration) - build_warp_anchors(points, bar_starts): monotonic (score, audio) anchors - warp_time(t, anchors): piecewise-linear map with edge-slope extrapolation - warp_song_times(song, warp): retime a lib.song.Song in place (notes, sustains, chords, beats, sections, anchors, handshapes, phrase levels, tone changes, tempo overrides) - gp_has_expandable_repeats(gp_path): detects GP3/4/5 repeat/volta/direction markup whose playback expansion auto_sync's as-written points cannot map Also implement refine_sync() — the editor's refine-sync endpoint has imported it since the snapshot but it never existed in lib, so the Refine button 500'd. It densifies the DTW points to every Nth bar and re-times each with a local onset phase sweep (radius clamped under half a beat to avoid one-beat locks, short scoring grid + median residual snap against the first beats). Synthetic click-track validation: ~13ms mean / ~40ms max error from ±180ms coarse input across 117-123 BPM recordings of a 120 BPM tab. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * review: Copilot round 1 — normalize bar_start_times GP3/4/5 parse failures to ValueError, document ImportError Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
2c1c6f7eac |
fix(starter): sync _BUILTIN_STARTER_SOURCES with content/starter on disk (#775)
Two commits (delete beethoven-ode_to_joy, re-add The Adicts' Ode to Joy) never updated _BUILTIN_STARTER_SOURCES: it still listed the deleted pack and omitted the added one. The listed-but-missing file made the all-present gate never fire, so NO starter content seeded on first run — and the on-disk-but-unlisted pack would bundle as dead weight. Both starter-seed guard tests were red on main, reddening ci/test on every core PR. Sync the manifest to disk. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
14eaad09e9 |
feat(library): add Star-Spangled Banner + Ode to Joy starter content (#744)
Add two more public-domain starter songs alongside Für Elise, wired into _BUILTIN_STARTER_SOURCES so they seed into DLC_DIR/starter/ on first run: - The Star-Spangled Banner (lead) — John Stafford Smith; cleaned the "Unknown" artist / placeholder album, author "Fee[dB]ack". - Ode to Joy (lead/rhythm/bass + drums) — Beethoven. Replaces the raw "Ode to Joy (VST Cover)_The Adicts.feedpak" that was committed to main but never added to the seed list (so it bundled 23 MB of dead weight and never appeared). Fixed metadata (artist Beethoven, year 1824, author "Fee[dB]ack"), and repointed the stem from the 22 MB editor WAV to the byte-identical-render full.ogg (both exactly 85.324 s), shrinking the pack 23.8 MB -> 1.7 MB. Add guard tests asserting every _BUILTIN_STARTER_SOURCES entry has its file committed and that a seed run lands them all — this catches exactly the listed-but-missing (or committed-but-unlisted) mismatch that left Ode to Joy un-seeded. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
7c873f5cc2 |
feat(library): seed bundled starter content into the library on first run (#743)
Ship a public-domain Für Elise (keys) feedpak as starter content so a fresh install isn't an empty library. server._seed_builtin_starter_content() copies bundled packs into DLC_DIR/starter/ exactly once, guarded by a marker in CONFIG_DIR — unlike the always-reseeding diagnostic seed, a user who deletes the starter song does not get it back. `starter/` is deliberately outside the diagnostics/tutorials library carve-out so the song surfaces as a normal library entry. Extract the shared symlink-safe, mtime-aware copy loop into _copy_builtin_packs() and route both the diagnostic and starter seeds through it (diagnostic behavior unchanged; existing tests green). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
68e29a8b6e |
fix(plugins): don't treat transient absence from /api/plugins as uninstall (#741)
* fix(plugins): don't treat transient absence from /api/plugins as uninstall The backend clears its plugin registry at the start of load_plugins() and repopulates it incrementally while HTTP stays up, so every backend restart (desktop: Audio Quality soundfont switch, LAN toggle, update restart) serves a window of partial — even empty — /api/plugins responses. loadPlugins() treated absence from the current response as an uninstall, with three destructive consequences for still-loaded plugins: 1. Their settings-panel and screen DOM were wiped while their _loadedPluginScripts entry survived, so the NEXT refetch failed the DOM-existence check and re-evaluated the plugin's screen.js mid-session. For the desktop audio_engine plugin that re-ran init() against the surviving native audio chain and exactly duplicated every VST/NAM/IR stage (the alpha testers' "chain duplicates after leaving the Audio menu" / blown-out gain reports). 2. _reconcilePluginStyles dropped their stylesheet, leaving them visible but unstyled until they reappeared. 3. The stale-contribution sweep unmounted their UI contributions and unregistered their capability participant with no re-registration path (plugin scripts don't re-run thanks to the loadedScripts guard). Absence is now a non-signal everywhere in loadPlugins: the DOM wipe and style reconcile are scoped to plugins the response actually names, and the absence sweep is removed. Present plugins still fully re-sync via _registerLegacyPluginUiContributions each round; failed plugins are present in the response and still cleaned up; nav is rebuilt from the response so genuinely uninstalled plugins drop out of it, and their (un-unloadable) already-evaluated scripts keep their DOM until reload. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: update idempotence contract to the absence-is-not-uninstall invariant The removed-plugin sweep contract pinned the old behavior this branch deletes; pin the new invariant instead (no absence sweep + respondedIds scoping on the DOM/style reconcilers). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
d27cbe78ba |
chore: remove stale root README (#739)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron GamatosandGitHub
|
803bd0cdf3 | Bump version to 0.3.0-alpha.1 | ||
|
|
9456790083 |
fix(release): lowercase the ghcr repo name in image tags (#738)
The repo is 'got-feedback/feedBack' (capital B) after the rename, so ${GITHUB_REPOSITORY} produced an invalid Docker tag ('repository name must be lowercase'). Use ${GITHUB_REPOSITORY,,}. nightly/rc already hardcode lowercase 'feedback'.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
286c59707b |
fix(tests): isolate plugin routes modules + redact .feedpak filenames (#736)
Two pre-existing failures the segfault had been masking (the run aborted at ~25%, so they never ran until #735 let the suite complete): 1) Tuner group (~24): plugins ship a bare-named routes.py, so sys.modules['routes'] leaked between plugin test dirs (achievements ran first, tuner got its module). Each plugin conftest now pops the stale 'routes' and an autouse fixture binds sys.modules['routes'] to that plugin's module for the duration of its tests (covers runtime 'import routes' in test bodies). 2) Diagnostics group (5): _SONG_FILENAME_RE never matched the tests' .feedpak/.archive filenames — it also lacked 'feedpak' (the current primary format), a real redaction gap. Added feedpak to the regex and switched the tests off the fake .archive to the real .feedpak. Verified: full suite 2183 passed, 0 failed. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
97a941c45d |
fix(tests): join background scan/enrich workers before closing the DB (#735)
Root cause of the flaky pytest segfault (exit 139): the background scan and enrichment daemon threads (_scan_runner/_enrich_runner) use the shared MetadataDB connection, but test fixtures closed that connection in teardown without stopping them. A daemon thread mid-query on a freed SQLite conn is a native use-after-free → SIGSEGV. The app's startup kicks a scan, so almost any app-booting fixture was vulnerable. It only surfaced now because got-feedback/feedBack#728 added a push trigger, so ci/test runs on every push to main. Fix: server.py retains the scan/enrich thread handles and adds _join_background_db_threads(); every test fixture now joins the workers before conn.close(). Verified: the full suite runs to completion (no segfault) where it previously crashed at ~25%. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
9d6fdfe232 |
feat(v3): use PNG logo in sidebar nav instead of the text wordmark (#734)
Replaces the fee[dB]ack text wordmark in the #v3-brand sidebar header with the exported PNG logo (static/v3/brand/feedback-logo-light.png, 664x165). Sized width:100% + height:auto so it fits the 256px sidebar's content width (~208px inside the p-6). Updated both the no-JS fallback (index.html) and the shell.js boot render. Inline style avoids introducing a new Tailwind utility (constitution P-II). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
df2d660d1e |
Re-enable the v3 "Support Us!" donate button (#727)
Funding was cleared to come back online 2026-06-30 (offending
functionality fully removed). Restore the v3 topbar donate button
(hand-re-applied revert of
|
||
|
|
f5c9c34291 |
library: scraper options — per-source + per-field auto-apply, review-queue order (R1) (#726)
* library: scraper options — per-source + per-field auto-apply toggles, review-queue order (R1) Grows the Settings→Library "Metadata matching" card into the full scraper-options panel — not everybody needs the same things out of a scraper: - Sources: enrich_src_musicbrainz gates the background matcher (phase 2; identity hashes still stamp, and manual Fix-match/search stays available — same contract as the master toggle). enrich_src_caa gates the Cover Art Archive fetch (phase 3). Rows skipped by an off toggle stay unevaluated, so re-enabling picks them up on the next pass — nothing is permanently forfeited. - Auto-apply fields: enrich_apply_names/year/genres filter what an AUTOMATIC match may canonicalize (_enrich_field_filter, applied on all three automatic paths: cache copy, mbid/isrc exact keys, text auto). MusicBrainz ids always stamp — they're identity, not display; the art fetch and future re-matching need them. A match the user confirms in the review modal applies in full. enrich_apply_art gates the art fetch alongside the CAA source toggle (two axes, one behaviour today — future art sources slot in without re-teaching the panel). - Review queue order: enrich_review_order = missing_first (default, today's behaviour) | artist | recent, read by GET /api/enrichment/review; unknown stored values degrade to the default. - Settings card: Sources / Auto-apply / Review-queue-order groups wired in match-review.js; the master toggle is relabelled "Match songs automatically" so it doesn't read the same as the new MusicBrainz source toggle. No tailwind rebuild needed — every class was already scanned from core source. Tests: tests/test_scraper_options.py (9) — settings validation, MB-source-off stamps-without-matching + re-enable, per-field stripping on auto matches with ids preserved, review-accept full-apply despite toggles, CAA gating on both axes, review-order modes incl. the unknown-value fallback. Full-suite failure set A/B-identical to the base (39 env/pre-existing). Stacked on feat/enrichment-art (#715) — merge that first. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN * library: per-field auto-apply honours "nothing forfeited" (backfill + no partial seeding) The R1 per-FIELD auto-apply toggles settled a `matched` row with the disabled fields stripped, but enrichment_pending() never revisits an unchanged-hash matched row — so re-enabling a field never backfilled it, and enrichment_cache_lookup() (which gates only on mb_recording_id) could seed sibling charts with the stripped blanks. This broke the same "nothing is permanently forfeited" contract the source (mb_on) and art (art_on) toggles already keep. Fix: persist an `apply_mask` marker (sorted blocked apply-keys) on every AUTOMATIC match: - migration: additive `apply_mask TEXT` column (idempotent ALTER). - enrichment_pending(allowed_keys=...): re-queues a `matched` row whose apply_mask names a field that is now re-enabled → backfill on re-enable, converges (a fully-applied row is never re-queued). - enrichment_cache_lookup: only fully-applied donors (apply_mask empty/NULL) may seed siblings; a partial row is skipped and the sibling falls through to its own re-filtered match. - _enrich_apply_mask()/_enrich_blocked_apply_keys() helpers; threaded through _enrich_one → apply_enrichment_match. Review/manual writers leave it NULL (a confirmed pick applies in full). Tests: re-enable-backfills-and-converges; partial row is not a cache donor (fully-applied one is). 13 scraper-options tests pass; 170 enrichment/ settings tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: ChrisBeWithYou <christian.a.cowan@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
2e4383524f |
fix(v3): drawer fav-sync honours data-fav-idle (no dim-heart on List View) (#717)
_patchCardFav (the Song Details drawer's like -> card heart sync) hardcoded
`classList.toggle('text-white', !fav)`, so toggling the like from the drawer
left List-View rows' `text-fb-textDim` idle class in place — the exact
dim-heart bug #654 fixed for the on-card click handler, reintroduced on the
drawer path. Read the per-heart `data-fav-idle` and swap that class instead,
mirroring wireCards. +regression assertion in v3_favorites_toggle.test.js.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
13db718bda |
test(v3): sync A–Z rail assertion to the railParams refactor (#702) (#714)
refreshRail was refactored (PR #702 work-grouping) to build a `railParams = { sort_letters: 1 }` object (adding group when grouping is active) before calling queryParams(), instead of the inline queryParams({ sort_letters: 1 }). Behaviour is unchanged — it still opts into the active-sort breakdown — but the source-assertion test lagged and went red on main. Point the assertion at the new railParams shape. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
9b4bef3fd1 |
fix(drum_highway_3d): enlarge note gems ~20% for readability (#713)
Disc radius 3.6->4.3, cymbal 3.0->3.6 (heights proportional) — hands-on feedback said the gems were hard to read at the default camera. All variant shapes key off these radii; accents still fit the lane gap. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
3e2703d8e1 |
feat(keys_highway_3d): audio-reactive ambience + score FX overlay (K4) (#709)
- BG_STYLES port (off/particles/lights/geometric; lights use the pitch-class palette) mounted behind the scene; _bgGetAnalyser/ _bgReadBands (stems-first, one-shot #audio fallback, permanent-failure latch); Ambience intensity + Audio-reactive settings; remounts on style/intensity change - Score-FX overlay canvas (drum_highway_3d pattern): +1 pops at the scored key, ring pulse every 10-combo, milestone bursts at 25/50/100, red wash on 3+ streak break (wrong notes AND swept misses); cleared when idle, removed in teardown - Tests: style id validation + FX defaults (30 total) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
d409d55615 |
feat(drum_highway_3d): audio-reactive ambience + score FX overlay (D4) (#708)
- BG_STYLES port (off/particles/lights/geometric) into a renderOrder -1 group; _bgGetAnalyser/_bgReadBands (stems-first, one-shot #audio fallback, permanent-failure latch, 5ms bands cache); Ambience intensity slider + Audio-reactive toggle; remounts on style/intensity/ palette change and across kit-change scene rebuilds - Score-FX overlay canvas (guitar drawScoreFx adapted to internal scoring): +1 pops at the struck lane, ring pulse every 10-combo, milestone bursts at 25/50/100, red wash on 3+ streak break; pooled, cleared when idle, removed in teardown - butterchurn/image/video deliberately out of scope - Tests: style id validation + FX defaults (15 total) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
4214ef365e |
feat(drum_highway_3d): materials & themes — studio env map, metal cymbals, BG_THEMES, cinematic, glow/vibrancy (D3) (#712)
- _makeStudioEnv: procedural PMREM environment (no RoomEnvironment addon vendored) -> scene.environment; rebuilt on kit-change, disposed both teardown paths - Cymbals roughness 0.2 / metalness 0.85 / envMapIntensity 1.2 (metal finally reads); discs satin 0.45 + envInt 0.5; floor 0.7 + envInt 0.35; hit bar envInt 1.0 - BG_THEMES port (guitar ids/values; drum 'default' = original palette byte-for-byte; single pick drives clear/fog + board + lane stripes); drumH3dSetTheme + drum_h3d_bg_theme; live _applyTheme - Cinematic lighting toggle (0.3/1.2 on, 0.4/1.0 off = stock) - Glow slider (base * glow*2; 0.5 default = stock) across notes/hit bar/snare stripe; Lane vibrancy slider (stripe base 0.12+0.24v + halo/ghost opacities), stacking under the D2 approach highlight - Fixed pre-existing floor/hit-bar geometry+material leak on kit change - Tests: theme table parity + default preservation + fallbacks (13) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
80fc371f11 |
feat(keys_highway_3d): anti-plastic materials, studio env, themes, gradient sky, cinematic + glow (K3) (#711)
- Note gems -> MeshPhysicalMaterial: clearcoat 1.0/0.18, roughness 0.32, envMapIntensity 0.9 — lacquered glass, not plastic (the explicit ask) - _makeStudioEnv (PORTED drum_highway_3d): PMREM studio -> scene.environment; black keys 0.22 roughness / 1.3 envInt (glossy piano black), whites 0.42/0.55 ivory, floor 0.55/0.15/0.4 stage sheen - Vertical-gradient background (light horizon -> theme clear -> dark deck), sRGB-tagged for the composer path - BG_THEMES port (guitar ids/values; keys 'default' = original palette; themes drive gradient/fog/floor/rails, never the pitch-class colors); keys3dSetTheme + keys3d_bg_theme, live _applyTheme - Cinematic lighting toggle (0.55/1.3 on; stock 0.75/1.1 off); Glow slider across NOTE_EMISSIVE_BASE / consume-flash / key approach-glow - Env RT + gradient texture disposed in teardown - Tests: theme table parity + default preservation (28 total) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
95b0786725 |
feat(keys_highway_3d): hit FX — vibrancy, timing-colored sparks, hit-line kick (K2) (#699)
- Note vibrancy: gem opacity 0.8 -> slider-driven (default 0.92), NOTE_EMISSIVE_BASE 0.08 -> 0.22, lane guides scale with the slider, _applyVibrancy retints the live scene without a chart rebuild - Sparks (PORTED highway_3d, pool 96) at the struck key, colored by _timingHex/_classifyTiming (±100ms window, inner 40% = on-time; delta recovered from judgeHit's noteKey prefix — contract untouched); streak-scaled counts - Hit-line brightness kick on scored presses (exp(-t*6) decay, hitFx slider), folded into the existing pulse incl. the bloom damp gate - Settings: Hit sparks / Timing colours / Streak feedback + Hit feedback intensity + Note vibrancy sliders (keys3d_bg_*, live-applying) - Tests: classifier boundaries, noteKey time round-trip, FX defaults (26 total) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
56419dc789 |
feat(drum_highway_3d): hit FX — sparks, timing colors, lane flashes, kick pulse, open hi-hat (D2) (#697)
- Pooled spark bursts (PORTED highway_3d, pool 160) at the struck lane, timing-colored via _timingHex/_classifyTiming (±50ms window, inner 40% = on-time); streak-scaled counts (streakFx) - Lane flashes resurrected as pooled additive gauss-tex quads at the hit line (timing-colored; red for wrong hits); kick = full-width quad - Kick pulse: camera dip + amber floor wash, exp decay, hitFx-scaled - Approach highlight: lane stripes brighten toward their next note - Open hi-hat: hh_open renders a warm ring around the gem (closes TODO); orthogonal to accent/ghost/flam variants - Settings: Hit sparks / Timing colours / Streak feedback toggles + Hit feedback intensity slider (drum_h3d_bg_*, live-applying) - All FX resources pooled/shared, disposed in BOTH teardown paths - Tests: _classifyTiming boundaries + FX defaults (10 total) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
beefd7bc09 |
feat(keys_highway_3d): foundation for guitar-highway visual parity (K1) (#696)
- setPixelRatio at last: DPR (cap 2; 1.25 when >1 instance) x host adaptive bundle.renderScale — HiDPI displays were rendering at CSS resolution and upscaling (soft/aliased) - Bloom: port _bloomEnsure/_bloomDispose (UnrealBloomPass 0.65/0.5/0.82, MSAA HalfFloat target, ACES<->None switch); hit-line, flames and consume-glow benefit immediately; direct render is the degrade path - First settings panel: settings.html (graphics category) with a live Glow (bloom) toggle; FX scaffold (FX_DEFAULTS/readFxSettings/ window.keys3dSetFx, keys3d_bg_* keys, keys3d:settings event) - Combo/accuracy/best-streak DOM HUD (drum_highway_3d pattern), gated on a live MIDI session - tests/fx_settings.test.js (3 tests; vm harness) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
a7ea719652 |
feat(drum_highway_3d): foundation for guitar-highway visual parity (D1) (#695)
- Consume host adaptive-quality bundle.renderScale; fold into DPR like highway_3d (splitscreen proxy: >1 instance caps baseDPR at 1.25) - Bloom: port _bloomEnsure/_bloomDispose (UnrealBloomPass 0.65/0.5/0.82, MSAA HalfFloat target, ACES<->None tone-mapping switch); rebuilt across the kit-change renderer recreation; direct render is the degrade path - FX settings scaffold: FX_DEFAULTS + readFxSettings + drumH3dSetFx (drum_h3d_bg_* keys), Graphics section in settings.html (bloom toggle, default ON, live-applies) - _applyLaneFlashes dead-code comment updated (visual consumer lands in the hit-FX PR) - __test export + tests/data_layer.test.js (vm harness, 8 tests) — picked up by the CI glob from the bundling PR; README refreshed Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
1e2d5a6162 |
Bundle drum_highway_3d + keys_highway_3d as in-tree core plugins (#693)
* Initial commit — clean relaunch * Initial commit — clean relaunch * Remove external game/format terminology from docs and code Reword references to the external game and its proprietary file formats in comments, docstrings, UI text and identifiers; no behaviour change. * Repoint dead slopsmith URLs -> got-feedback Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(midi): consume core midi-input domain + session robustness fixes (#2) * feat(midi): consume the core midi-input domain instead of private requestMIDIAccess (#881) Route Web-MIDI device access through window.slopsmith.midiInput (discover/list/select/open/close) rather than a private navigator.requestMIDIAccess(). One shared device-access boundary with piano/drums/onboarding; retires this plugin's private Web-MIDI. The note-detection 'midi' exact-verdict provider role and the audio-input source export are preserved (now backed by midi-input source data). Saved pick ({id,name}) stays compatible — domain sourceId == the old MIDIInput.id. Live listener is the domain handle's addListener/removeListener; the async init/resume/pause gate and device-vanish handling are kept. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): stop exporting MIDI into the audio-input domain Keys MIDI now lives in the dedicated midi-input domain (#881). Continuing to register pseudonymized 'midi-input-N' sources into audio-input polluted audio device pickers — notably the onboarding guitar audio-input dropdown showed MIDI devices under cryptic names. Unregister any leftovers and register none. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): guard async connect races, denied-discovery latch, key consistency Codex preflight findings on the midi-input consumption: - Don't latch `_midiReady` when discover() resolves denied/unavailable — only on a handled outcome — so reopening retries the permission prompt. - Add a generation guard to async `_midiConnect`: a slower open() from an earlier selection can resolve after a device/None switch and install a stale handle/listener. Discard (and close) superseded opens. - Carry the domain `logicalSourceKey` on the source/selection descriptor and use it for select/open/close instead of synthesizing `web-midi::<id>`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): invalidate pending opens on detach; clear phantom selection; manifest Codex re-review (round 2): - A detach driven by device removal (sources-changed) didn't advance _midiConnectSeq, so a pending _midiConnect open could resume and install a handle/listener for a now-gone source. Bump the generation inside _midiDetach and capture myGen after it, so any later detach supersedes an in-flight open. - If mi.open() yields no handle, _midiInput stayed set — a phantom connected device the render loop's miss-sweeping would penalize. Clear it on the no-handle and catch paths. - plugin.json: replace the stale audio-input provider role (the removed MIDI-into-audio-input export bridge) with the midi-input requester it now uses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): make _midiResume idempotent under multiple live instances Codex re-review (round 3): with the domain addListener API, a second live renderer instance (splitscreen/overlapping lifetimes) calling _midiResume() while already active could register the same listener again and double-deliver a MIDI note to the focused instance (hit + duplicate misses). Return early when already active rather than relying on the provider's Set-backed de-dup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): release domain session on teardown; counter-free legacy source cleanup Codex re-review (round 4): - [P2] Last-instance teardown only detached the listener and never released the midi-input domain session, leaking the requester ref. Add _midiReleaseSession() (delegates to _midiDetach: close + null + generation bump) at the destroy site; re-mount's _midiInit auto-connects from the saved pick. - [P3] The legacy audio-input MIDI-source cleanup looped over the module-local _aiRegisteredCount, which resets to 0 on an in-page upgrade so the prior build's 'keys-midi:input-N' entries were never unregistered. Iterate a fixed bound over the known sourceId pattern instead (unregister of an absent source is a no-op). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): gate miss-sweep on live handle; don't open without a live renderer Codex re-review (round 5): - Miss-sweeping was gated on _midiInput, set as soon as a device is picked, but the async mi.open() may still be pending (slow / permission prompt) — notes passing during that window banked false misses. Gate on _midiHandle, truthy only after a handle is opened and wired. - A settings-only ensure-init (or a discover resolving after the last instance was torn down) could open a midi-input session with no renderer to release it. Gate mi.open() on _instances.size > 0; the pick is saved and a later mount re-runs auto-connect. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): guard open-failure clobber + don't clobber saved device on unplug Bringing keys-highway-3d in line with the piano/drums hardening: - The _midiConnect catch cleared _midiInput unconditionally; a stale older open's rejection could wipe a newer connect's _midiInput/_midiHandle (and leak the handle). Only clear when myGen === _midiConnectSeq. - The sources-changed recovery called _midiAutoConnect(), whose fallback persists a substitute device — overwriting the user's saved pick on a transient multi-device unplug. Parameterize _midiAutoConnect(allowFallback); recovery passes false (reconnect the saved device only, never a persisted fallback). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): keep first-hotplug working; don't tear down a live session on re-init Codex round-11: - The unplug-recovery _midiAutoConnect(false) skipped ALL fallbacks, which broke first-hotplug (open visualizer with no keyboard, plug one in → never picked → no connect). Skip the substitute only when a saved pick exists but is absent (preserve it); allow the fallback when nothing was ever picked. - _midiInit re-ran _midiAutoConnect on every ready re-init (settings/splitscreen), tearing down the live handle + releasing held keys. Only re-connect when there's no live handle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(midi): consume core midi-input domain + session robustness fixes (#2) * feat(midi): consume the core midi-input domain instead of private requestMIDIAccess (#881) Route Web-MIDI device access through window.slopsmith.midiInput (discover/list/select/open/close) rather than a private navigator.requestMIDIAccess(). One shared device-access boundary with piano/drums/keys/onboarding. Saved pick ({id,name}, with legacy id-only fallback) stays compatible — domain sourceId == the old MIDIInput.id. The async in-flight init guard, _midiActive teardown gate, device-vanish handling, and settings device APIs are preserved; the live listener is the domain handle's addListener/removeListener. Degrades to no-MIDI when the domain is absent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): guard async connect races, denied-discovery latch, key consistency Codex preflight findings on the midi-input consumption: - Don't latch `_midiReady` when discover() resolves denied/unavailable — only on a handled outcome — so reopening retries the permission prompt. - Add a generation guard to async `_midiConnect`: a slower open() from an earlier selection can resolve after a device/None switch and install a stale handle/listener. Discard (and close) superseded opens. - Carry the domain `logicalSourceKey` on the source/selection descriptor and use it for select/open/close instead of synthesizing `web-midi::<id>`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): invalidate pending opens on detach; clear phantom selection; manifest Codex re-review (round 2): - A detach driven by device removal (sources-changed) didn't advance _midiConnectSeq, so a pending _midiConnect open could resume and install a handle/listener for a now-gone source. Bump the generation inside _midiDetach and capture myGen after it, so any later detach supersedes an in-flight open. - If mi.open() yields no handle, _midiInput stayed set — a phantom connected device the render loop's miss-sweeping would penalize. Clear it on the no-handle and catch paths. - Declare the midi-input requester capability (degrade-noop) in plugin.json. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): make _midiResume idempotent under multiple live instances Proactively mirror the keys-highway-3d round-3 fix (same multi-instance lifecycle and identical code): a second live renderer instance calling _midiResume() while already active could double-register the listener and double-deliver a MIDI hit. Return early when already active. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): release the domain session on final teardown Mirror the keys-highway-3d round-4 fix: last-instance teardown only detached the listener and never released the midi-input domain session, leaking the requester ref. Add _midiReleaseSession() (delegates to _midiDetach: close + null + generation bump) at the destroy site; re-mount's _midiInit auto-connects. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): gate miss-sweep on live handle; don't open without a live renderer Codex re-review (round 5): - [P2] drumH3dEnsureMidiInit (settings-only, no renderer) auto-connected and opened a midi-input session that destroy()/_midiReleaseSession would never run to release — held until reload. Gate mi.open() on _instances.size > 0; the pick is saved and a later renderer mount re-runs auto-connect. - Mirror the keys-highway-3d miss-sweep fix: gate accumulation on _midiHandle (live wired session), not _midiInput (set before the async open resolves), so notes during a pending open don't bank false misses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): guard open-failure clobber + reconnect saved device on replug Bringing drum-highway-3d in line with the piano/drums hardening: - The _midiConnect catch cleared _midiInput unconditionally; a stale older open's rejection could wipe a newer connect's _midiInput/_midiHandle. Only clear when myGen === _midiConnectSeq. - The sources-changed handler only refreshed the list (never reconnected), so a replug of the saved device didn't reattach. Reconnect on sources-changed via _midiAutoConnect(false) — saved device only, no fallback, so a transient unplug can't switch to / persist another input. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): keep first-hotplug working; don't tear down a live session on re-init Same two round-11 fixes as keys-highway-3d (shared structure): the recovery _midiAutoConnect(false) now allows a fallback when nothing was ever picked (first-hotplug) while still preserving a saved-but-absent pick; and _midiInit only re-connects when there's no live handle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): select by logicalSourceKey, not provider-local sourceId (#4) Codex review of the midi-input migration: resolving a selection with find(s => s.id === id) treats the provider-local sourceId as globally unique, so when the core midi-input domain aggregates multiple providers two devices sharing a sourceId can't be distinguished — the wrong device opens and unplug detection can miss that the selected key vanished. Thread the globally-unique logicalSourceKey through save/read, _midiConnect, auto-connect reconnect, the public keysH3dSetMidiInput entry, and the capability _aiOpen path, preferring the key and keeping the bare sourceId / name only as a legacy fallback. Mirrors the already-merged drums/piano migration (_midiResolveSaved). Self-reviewed against that reference. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(midi): select by logicalSourceKey, not provider-local sourceId (#3) Codex review of the midi-input migration (same issue as keys-highway-3d): resolving a selection with find(s => s.id === id) treats the provider-local sourceId as globally unique, so when the core midi-input domain aggregates multiple providers two devices sharing a sourceId can't be distinguished — the wrong device opens and unplug detection can miss the vanished key. Thread the globally-unique logicalSourceKey through save/read, _midiConnect, the auto-connect reconnect, and the public drumH3dSetMidiInput entry, preferring the key with the bare sourceId / name only as a legacy fallback. Mirrors the already-merged drums/piano migration. Self-reviewed. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: add plugin metadata (category, description, thumbnail) (#5) * chore: add plugin metadata (category/description/thumbnail) Refs got-feedback/feedBack#571 * chore: add placeholder thumbnail Refs got-feedback/feedBack#571 * chore: add plugin metadata (category, description, thumbnail) (#4) * chore: add plugin metadata (category/description/thumbnail) Refs got-feedback/feedBack#571 * chore: add placeholder thumbnail Refs got-feedback/feedBack#571 * fix(viz): register feedBackViz_<id> so the player viz picker lists this plugin (#5) After the slopsmith->feedBack rename the factory was still registered as window.slopsmithViz_<id>; the host viz picker only looks up window.feedBackViz_<id> (no slopsmithViz alias exists) and silently skipped this plugin. Add the feedBackViz_ global (aliased to the existing slopsmithViz_ one for back-compat). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(viz): register feedBackViz_<id> so the player viz picker lists this plugin (#6) After the slopsmith->feedBack rename the factory was still registered as window.slopsmithViz_<id>; the host viz picker only looks up window.feedBackViz_<id> (no slopsmithViz alias exists) and silently skipped this plugin. Add the feedBackViz_ global (aliased to the existing slopsmithViz_ one for back-compat). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(plugins): bundle drum_highway_3d + keys_highway_3d as in-tree core plugins Import both 3D highway plugins from their standalone repos via git subtree (history preserved), ahead of the visual-parity epic that ports the guitar highway's polish to them. - .gitignore: !plugins/drum_highway_3d/ + !plugins/keys_highway_3d/ exceptions - keys plugin.json: "bundled": true (drum already had it) - CI: JS test step gains 'plugins/*/tests/*.test.js' (+20 keys tests) - static/tailwind.min.css regenerated (core build scans plugins/**) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(drum_highway_3d): narrow Auto-mode predicate so bundling can't steal guitar arrangements has_drum_tab is pack-level; first-match-wins Auto order sorts this plugin before highway_3d. Claim only drum arrangements, or packs nothing more specific can render (Codex preflight P2). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: CHANGELOG — note the deliberate Auto-predicate narrowing Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(drum_highway_3d): MIDI lifecycle — no false misses on mid-song connect, promote survivor on destroy - _missSweepFloor exempts notes that passed before the device wired up from the miss sweep (lowered on seek-back; cleared with scoring resets) - destroy() promotes a surviving instance to _activeInstance so splitscreen panel teardown doesn't drop all MIDI routing (Codex preflight round 2, both pre-existing in the imported code) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Sin <deathlysin@outlook.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
96a84996a2 |
fix(tuner): mic-verify stamps the tuning it actually checked + on-device test plan (#684)
Working-tuning follow-ups: - Mic-verify used _state.currentSongOffsets for the 'verified' stamp, but for a MANUALLY-selected tuning (tuner opened off a song) that's a stale/different song's tuning — so verify could mark the WRONG tuning verified. It now derives the verified offsets from the tuning actually being checked (its target freqs; the player's reference pitch cancels in the ratio), so 'verified' always attaches to the tuning the player confirmed. Explicit offsets still win. - Adds docs/working-tuning-on-device-tests.md: the checklist for the parts that can't be covered headlessly — the auto-open/gate flow, both-directions prompts, mic-verify detection, and the tuner-mic-vs-note_detect ASIO/exclusive-mode contention flagged in the design charrette. Test: mic-verify with no explicit offsets / no song context derives the correct offsets (Drop-D). 55 tuner tests green. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
f9607c5c94 |
Port the Min res (minimum auto-resolution) selector into the v3 player (#663)
The v2 control bar exposes a "Min res" selector next to Quality that sets highway.setMinRenderScale — capping how far the load-adaptive resolution scaler (feedBack#654) may downscale, or disabling it entirely (Full). The v3 UI only ported the Quality selector, so v3 users had no way to stop the highway auto-downscaling to as low as quarter-res on heavy scenes / weak- GPU launches — pixelated even at Quality = HD, with no workaround (worse than v2). Add the Min res row to the v3 viz/quality rail popover, under Quality, mirroring the v2 control (same options, handler, title, aria). The handler, the setMinRenderScale/getMinRenderScale API, and the shared app.js init that syncs the selector's value (guarded by element id) all already exist — only the v3 markup was missing. Fixes #662 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
a732e1f9d2 |
perf(tuner): idle the always-on tuner viz rAF when there's no signal (#647)
The v3 home tuner card runs continuously, and every tuner visualization drove a self-rescheduling 60fps requestAnimationFrame loop that never stopped — pinning a renderer core even on a silent home screen with the needle/strobe at rest. Make each viz idle its loop once there's nothing left to animate, and re-kick it from update() on the next reading that actually moves it: - analogue-gauge: stop when the needle + drum strip have settled on their targets (|target-current| below a sub-visible epsilon); restart when a new reading moves the target. - strobe / mace-fx-iii / chef-mt3: stop when there's no live signal and the strobe drift (and glow fade) have fully decayed; restart on the next note. - toilet-tuner: stop when silent and the plunger has eased back to centre; restart on the next reading (guarded so repeated no-signal updates don't re-kick a parked loop). Active tuning is unchanged — the loop runs whenever a note is sounding or the indicator is still moving. Bumps tuner 1.3.1 -> 1.3.2. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
a791a0d8fe |
feat(v3): DOM-virtualize the Songs grid (#636 item 3 stage 2) (#643)
The v3 Songs grid appended every scrolled page and never released nodes, so card-node count grew unbounded with scroll depth (24 → 624 → 2001 for a 2000-song library). Replace it with a windowed/recycled render: only the visible window (± overscan) is in the DOM while a #v3-songs-gridsizer element sized to ceil(total/cols)*rowH gives the scrollbar full-library geometry; #v3-songs-grid is absolutely positioned to the first visible row. - state.songs is a sparse, absolutely-indexed store filled a page at a time by ensureWindow(): the stage-1 keyset cursor for contiguous forward scroll (O(page)), OFFSET page= for jumps/restore/non-keyset providers. _loadPage shares an in-flight promise per page and an epoch guard discards a stale fetch that lands after a reset. - A–Z rail seeks directly via sort_letters cumulative counts (O(1), no page-through); bounded scan fallback for legacy providers without it. - Snapshot/restore is now scrollTop-based (geometry is stable). Select mode, accuracy badges, ⋮ menu, plugin card actions, and tree/folder coexistence survive cards recycling; renderWindow re-renders when select mode toggles. - Plugins get window.v3Songs.visibleCards() + a v3:library-window-rendered event instead of assuming all cards are present (highway-stutter lesson). Verified in a browser against a seeded 2001-song library: DOM bounded to ~60 nodes while the count reads "2001 songs", rail jump lands on the target row, selection survives recycling, scroll-restore exact. Codex-reviewed (3 findings fixed: stale-fetch epoch guard, await-in-flight page promise, select-mode resync on cached re-entry). Frontend-only. Tests: tests/browser/v3-grid-virtualization.spec.ts pins the bounded-DOM invariant + direct rail jump; tests/js/v3_az_rail.test.js and v3_songs_scroll.test.js updated to the new wiring. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
5ed6f454e7 |
feat(library): smart collections as a library provider (#641)
Implements feedBack#636 item 2 (P1) — saved library filters that stay live, the homelab primitive FeedBack was missing (Plex smart collections / Navidrome .nsp / *arr custom filters). A collection is a saved /api/library query surfaced as a registered library provider, so it appears in the v3 source picker and inherits the whole Songs UI (paging, stats, A–Z rail, art) with no new screen. - Storage reuses the playlist subsystem: a `playlists.rules` JSON column (additive, idempotent migration). A row with rules != NULL is a smart collection; list_playlists + get_playlist filter `rules IS NULL`, so collections are excluded from the manual-playlist list and read-only to every playlist mutation that gates on get_playlist. - SmartCollectionProvider (kind="local" — matched songs are local rows, so the client's play/art paths stay on the local branch) delegates query_page/ query_stats/query_artists to the local DB with the stored rules applied; tuning_names/get_art delegate straight through. Registered via a boot scan + on create/update (replace=True) / delete. - Rules mirror the raw /api/library query params; `_sanitize_collection_rules` drops unknown keys and is applied at API ingress AND on provider load, so a hand-edited / imported bad value can't crash a query. - API: GET/POST/PUT/DELETE /api/collections. Frontend: a "＋ Save as collection" action in the v3 filter drawer (local provider + active filters only) that names the current filter set and switches to it. Reviewed by Codex; 3 findings fixed (local-kind playback path, save gated to local provider, re-sanitize persisted rules). Tests: tests/test_collections_api.py (CRUD, provider filtering, restart re-registration, kind=local, corrupt-rule tolerance, playlist isolation), tests/js/v3_collections.test.js. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
331857ff2a |
feat(library): keyset cursor pagination + stable sort tiebreak (#636 item 3, stage 1) (#642)
Stage 1 of the virtualized-grid project (got-feedback/feedBack#636 item 3): the data layer the DOM-recycling render window will build on, plus a latent paging bug fixed on the way. - Every grid sort now appends a unique `filename` tiebreak → a TOTAL order. Without it, rows with an equal sort key (e.g. two songs by the same artist) could be skipped or duplicated across OFFSET pages. - query_page gains an opaque `after` keyset cursor: when supplied and the sort can keyset (artist[-desc], title[-desc], recent), the page is fetched with a WHERE-seek instead of OFFSET — O(page), independent of depth. The seek is NULL-aware (NULLs first in ASC / last in DESC) so it's EXACTLY OFFSET- equivalent; the legacy `dir=desc` shape is canonicalized so its cursor seeks the right direction. Unknown/compound sorts + bad cursors fall back to OFFSET. - /api/library exposes `after` + `next_cursor`. Only the true local provider is handed a cursor (a collection may pin a different sort; remote don't keyset), so both page by OFFSET safely. - Composite (artist NOCASE, filename) / (title NOCASE, filename) / (mtime, filename) indexes cover the order; `after` added to the optional provider kwargs so legacy providers drop it. Codex-reviewed; 3 findings fixed (dir=desc canonicalization, NULL-key seek, cursor only for the local provider). Tests: tests/test_library_keyset.py (keyset==OFFSET parity for 5 sorts, stable tiebreak on equal keys, dir=desc, NULL sort keys, bad-cursor + compound-sort fallback). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
8ca7ea4002 |
feat(library): persisted wishlist / "wanted" list (#640)
Closes feedBack#636 item 4 — the *arr "Wanted/Monitored" analogue FeedBack was missing. A wishlist entry is a song the user does NOT own yet, so unlike a playlist (which references owned local songs by filename) it can't reuse the playlist subsystem; it lives in a new `wanted` table keyed by descriptive identity (artist, title, source, source_ref, note, created_at). - New table + a UNIQUE index on (artist NOCASE, title NOCASE, source, source_ref); additive + idempotent (CREATE … IF NOT EXISTS). - MetadataDB.add_wanted (INSERT OR IGNORE + re-select under the write lock, so a re-run of an ownership-diff returns the existing row, never a dup), list_wanted (newest first), remove_wanted, count_wanted. - Routes GET/POST/DELETE /api/wanted. POST requires artist or title and defaults source to "manual"; idempotent on identity so producers (the find_more ownership-diff, or a manual add) can re-post freely. This is the core persistence primitive the charrette flagged as the missing piece; the consuming UI lives in the producing plugin (find_more / the_daily). Tests: tests/test_wanted_api.py (round-trip, identity idempotency incl. case-insensitive, distinct source_ref, ordering, validation, additive schema). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
07ab902604 |
feat(settings): back up the library DB + custom art in the export bundle (#639)
Closes the dev-ops lens's #1 finding from the library charrette (got-feedback/feedBack#636 item 1): scores, favorites, playlists, and play history — the only library state a rescan can't rebuild — were absent from the settings backup. Now GET /api/settings/export carries an additive `core_server_files` section: - a CONSISTENT snapshot of web_library.db via the SQLite online-backup API (a complete single file even while the server runs; taken under the MetadataDB write lock), base64-encoded; - custom playlist covers + avatar (CONFIG_DIR/playlist_covers, /avatars), walked with the existing _walk_export_paths machinery. Restore is DB-safe: - POST /api/settings/import STAGES the DB to web_library.db.restore (never over the live, open file); _apply_pending_db_restore swaps it in at the next startup BEFORE the connection opens, clearing stale -wal/-shm so a stale WAL can't be replayed onto the restored file. Response sets `restart_required` + a warning; custom art applies immediately. - The staged DB is integrity-checked (open + PRAGMA quick_check) at import AND again at startup before the live DB is touched — a corrupt/truncated restore is refused/discarded and the live DB is left intact, so a bad bundle can never brick startup or lose data. - Export hard-fails (500) if the snapshot can't be produced (no silent DB-less backup); a partial import disarms its own staged restore. Backward-compatible: older servers ignore the new section; a bundle without it imports as before. Known gap: custom uploaded *song* art is still commingled with the rebuildable thumbnail cache in art_cache/, so it isn't bundled yet (tracked follow-up on #636). Tests: tests/test_settings_export_library_db.py (snapshot consistency, staged-not-live restore, sidecar clearing, corrupt-DB refusal at import + startard, traversal rejection, export hard-fail, disarm-on-failure, full round-trip). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |