Compare commits

...
Author SHA1 Message Date
byrongamatosandClaude Opus 4.8 dbe431779c refactor(server): extract the highway WebSocket into routers/ws_highway.py (R3)
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>
2026-07-10 20:55:51 +02:00
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>
2026-07-10 20:41:42 +02:00
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>
2026-07-10 20:16:22 +02:00
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>
2026-07-10 19:58:09 +02:00
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>
2026-07-10 19:16:01 +02:00
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>
2026-07-10 19:05:01 +02:00
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>
2026-07-10 18:55:03 +02:00
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>
2026-07-10 17:16:34 +02:00
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>
2026-07-10 15:24:31 +02:00
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>
2026-07-10 15:06:22 +02:00
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>
2026-07-10 14:30:05 +02:00
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>
2026-07-10 14:18:59 +02:00
e134f5c802 fix(highway_3d): size Butterchurn output canvas buffer to fill the highway (#820)
* fix(highway_3d): size Butterchurn output canvas buffer to fill the highway

The 3D-highway Butterchurn background set only the output canvas CSS size
and called setRendererSize(), but never sized the canvas DRAWING BUFFER
(canvas.width/height). Butterchurn does not size the output canvas itself
(renderToScreen viewports to the reported size into the default
framebuffer), so the buffer stayed at the browser default 300x150 while the
viewport was the full highway. Only the bottom-left ~300x150 of the pattern
was drawn, then CSS-stretched across the whole highway -- zoomed, soft, and
aspect-wrong, worse the larger the panel.

Add _bcApplySize(cssW, cssH): set the drawing buffer to the device-pixel
render size (round(css * min(DPR, 1.5))), confine every layer (canvas,
backdrop, scrim, tint) to the highway rect, and report the same device px
to setRendererSize so buffer == on-screen viewport. Seed the buffer at
create and switch createVisualizer to pixelRatio:1, textureRatio:1 (DPR is
now folded into the reported size, so buffer == viewport == internal
texsize, no double-counting). render() and resize() both route through it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>

* chore(highway_3d): bump to 3.31.5 (3.31.4 taken by #823 on main)

Rebased onto main; #823 already shipped 3.31.4 (per-panel camera), so this
Butterchurn buffer-sizing fix advances to 3.31.5.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Byron Gamatos <xasiklas@gmail.com>
2026-07-10 13:12:16 +02:00
f1bae9774c fix(gp2rs): write beat times at 6-decimal precision so imported tempo matches Guitar Pro (#819)
* fix(gp2rs): write beat times at 6-decimal precision

The editor/timeline derives per-bar BPM from beat spans
(bpm = beats*60/span), which amplifies rounding: at millisecond
(3-decimal) precision a constant-tempo GP import (e.g. 140 BPM) shows a
spurious per-bar "tempo drift" of ~0.05-0.7 BPM because most bar lengths
don't land on a ms boundary (worse for fast/odd meters). gp2rs computes
these beat times exactly from the GP tempo map, so the only precision
loss is the ebeat/startBeat format string. Writing them at 6 decimals
(microseconds) makes the derived tempo match GP's authored value.

Verified on GP5 imports (Highway to Hell 116, Equivalence 140, Living
After Midnight 138): the derived per-bar BPM collapses from two drifting
values to the single authored constant.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013JgxKh99UAeQqmhzSc73tv
Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>

* test(gp2rs): compare ebeat times by value, not string

The 6-decimal beat-time write makes _assert_ebeats' exact-string compare fail
("0.500" vs "0.500000"). These tests only assert spacing, so parse both sides
to float — precision-agnostic, no need to rewrite every parametrized list.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Byron Gamatos <xasiklas@gmail.com>
2026-07-10 13:12:10 +02:00
751209b80e Serve exact MIDI notes from GET /api/tunings (tuningMidis) (#829)
The tunings catalog is served as frequencies scaled to the reference pitch,
so every consumer that needs note identities (the v3 instrument badge's
TUNING_NOTE, plugins converging on the host profile model) reconstructs MIDI
numbers client-side via log2 — a rounding footgun at non-440 references, and
N copies of code the host can run once.

Add `tuningMidis` to the response: the same catalog keyed instrument-count →
name → absolute open-string MIDI notes (low → high). Built-ins come straight
from TUNING_PRESET_MIDIS (no float round-trip at all); provider-contributed
entries are inverted from their frequencies at the served reference via the
new freqs_to_midis() (the inverse of open_midis_to_freqs, garbage-guarded).
Purely additive — referencePitch/tunings are unchanged.

Tests: every built-in round-trips at 440; round-trip holds at 430/432/444/450
(the exact case client-side reconstruction drifts on); garbage rejected.


Claude-Session: https://claude.ai/code/session_01MS2YFb6UUSwJVV6CmEa25i

Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-10 13:05:39 +02:00
31 changed files with 7171 additions and 6125 deletions
+75
View File
@@ -7,6 +7,81 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
### Fixed
- **The packaged desktop app could not start (`ModuleNotFoundError: No module named
'appstate'`).** feedback-desktop's `scripts/bundle-slopsmith.sh` copies a *hardcoded
list* of core files into the app bundle — `server.py`, `VERSION`, `lib/`, `data/`,
`static/`, `plugins/__init__.py`. The root-level `appstate.py` and `routers/` added in
R3 shipped correctly in Docker and passed every test, and were then silently dropped
from the packaged app, which died at startup. Both now live under **`lib/`** — the one
core directory the Dockerfile (`COPY lib/`), `docker-compose.yml`, and the desktop
bundler (`cp -r lib`) all copy wholesale, and that all three put on `sys.path` (on
Windows via the embeddable-Python `._pth`, where `PYTHONPATH` is ignored). This needs no
change in feedback-desktop and no new release to take effect. Placing them there is also
correct under Principle V: with the injection seam, `appstate.py` constructs nothing and
does no import-time IO, and a route module only builds an `APIRouter`. The
`Dockerfile` / `.dockerignore` / `docker-compose.yml` entries added for the root layout
are reverted. New `tests/test_packaging.py` walks `server.py`'s module-level imports and
fails if any first-party module resolves outside a directory the packagers copy, so the
next root-level module can't ship broken.
### Added
- **`routers/` — extracting `server.py`'s route layer, cheapest-first (R3).** Each PR moves a cohesive route group into a `fastapi.APIRouter` under `lib/routers/`, mounted with `app.include_router(...)` at its original site (FastAPI matches in registration order; the full route table stays byte-identical). Bodies are verbatim — only the decorator receiver (`@app` → `@router`) and singleton reads (`meta_db` → `appstate.meta_db`, resolved at call time) change. So far: `audio_effects` (5), `artist_aliases` (5), `loops` (3), `playlists` (12 + covers), `ws_highway` (the 902-line highway chart WebSocket — the single largest handler). The DLC library-path resolution (`_get_dlc_dir`, pure `_resolve_dlc_path`) moved to `lib/dlc_paths.py`, reading paths through the seam; `config_dir`/`dlc_dir`/`dlc_dir_env` now ride the `appstate` seam (env-derived, so the pop-and-reimport fixtures reconfigure it for free), and the shared request-field sanitizer `_clean_str` moved to `lib/reqfields.py`. The next cut is picked by a dependency-closure scan that ranks groups by how many `monkeypatch.setattr(server, …)` targets they'd drag along.
- **`routers/` — the first extracted route module (R3).** The five audio-effects mapping
endpoints move out of `server.py` into `lib/routers/audio_effects.py` as a
`fastapi.APIRouter`, mounted with `app.include_router(...)` **at the point in the file
where they used to be defined** — FastAPI matches routes in registration order, so the
mount site preserves it. Verified: the full 143-route table (paths, methods, *and*
order) is byte-for-byte identical to `main`. Bodies are verbatim; the only edits are
the decorator receiver (`@app.get` → `@router.get`) and the singleton read
(`audio_effect_mappings` → `appstate.audio_effect_mappings`, a module attribute
resolved at call time). This proves the seam from #833 under a real consumer, including
the second slot. The `_demo_mode_guard` middleware still blocks all four moved write
routes with 403, and `Query(...)` validation still 422s — both checked against a running
server. `server.py`: **9,445 → 9,386 lines**.
- **`appstate.py` — the router seam (R3).** Route modules moving out of `server.py`
need `meta_db` and friends but must not `import server`, or the import graph goes
circular the moment `server` imports them back. So `server.py` keeps *constructing*
its singletons and now **injects** them once — `appstate.configure(meta_db=…,
audio_effect_mappings=…)` — and a router reads them back as module attributes at call
time (`import appstate; appstate.meta_db.…`). This is the Python analogue of the
frontend refactor's injected `configureX({…})` seams and of the plugin
`setup(app, context)` contract: dependencies flow one way, `server → routers →
appstate`. Two properties are load-bearing and pinned by `tests/test_appstate.py`:
(1) `import appstate` constructs nothing and touches no disk, so 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; (2) reads must be late-bound (`appstate.meta_db`, never
`from appstate import meta_db`), since a `from` import freezes the binding and defeats
both a later `configure()` and `monkeypatch.setattr` — the same read-only-binding trap
as ES `import`. `configure()` rejects an unknown slot rather than silently creating a
global nothing reads, and the suite asserts `server` actually calls it (a seam whose
wiring can no-op undetected is worse than no seam). Lives at `lib/appstate.py`.
### Changed
- **`AudioEffectsMappingDB` moved out of `server.py` into `lib/audio_effects_db.py`
(R3, move-only).** The core-owned song/tone → provider routing index follows
`MetadataDB` out of the host file, byte-identical apart from the same constructor
seam (`__init__` takes `config_dir`; `audio_effect_mappings = AudioEffectsMappingDB(CONFIG_DIR)`),
so the module does no IO at import. The singleton stays in `server.py`; no route,
no test and no `monkeypatch.setattr(server, …)` target moves. `server.py`:
**9,705 → 9,433 lines**.
- **`MetadataDB` moved out of `server.py` into `lib/metadata_db.py` (R3, 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) — now lives in its own flat
`lib/` module. `server.py` drops from **14,037 → 9,705 lines** and keeps the
`meta_db` singleton, so `server.meta_db` and `server.app` resolve exactly as before
and every route is untouched. The only non-verbatim change is the seam that lets the
class leave `server.py`: `MetadataDB.__init__` now takes `config_dir` explicitly
(`meta_db = MetadataDB(CONFIG_DIR)`) instead of reading the module-level `CONFIG_DIR`,
which also means `lib/metadata_db.py` performs no IO at import (Principle V). Logging
still goes through the `feedBack.server` logger, so existing log filters and `caplog`
assertions resolve to the same logger object. `tests/test_settings_export_library_db.py`
now imports `_apply_pending_db_restore` from `metadata_db` (the test moved with its
subject); no other test changed. Every moved block is byte-identical to its
`server.py` original.
### Added
- **Plugins can ship an ES-module `src/` tree (module-migration rails, R0).** The host gains three things so a plugin can move off a single global-scope `screen.js` IIFE onto native ES modules with **no build step**: (1) a new sandboxed `GET /api/plugins/{id}/src/{path}` route that serves a plugin's `src/` source subtree, containment-checked by the same `safe_join` guard as `assets/` (traversal/absolute/NUL → 404); (2) the live-edit cache contract — `Cache-Control: no-cache` + a weak mtime/size `ETag` + `If-None-Match`→`304` — applied to `src/`, `screen.js`, and `assets/` (previously `screen.js` sent no cache headers and `assets/` emitted an ETag but never revalidated), so an edited module reloads on refresh while unchanged ones `304`; and (3) `scriptType`/`minHost` passthrough from `plugin.json` to `/api/plugins`, with the loader injecting a plugin that declares `"scriptType":"module"` as `<script type="module">` (its screen.js becomes `import './src/main.js'`). A `<script type=module>` fires its load event only after its whole static-import graph evaluates, preserving the loader's completion-by-`onload` + `_loadingPluginId` contract. Classic plugins are unaffected; `minHost` is passthrough-only for now (enforcement deferred). Tests: `tests/test_plugin_src_route.py` (serve/media-type/traversal/304/no-stale-304/screen.js+assets conditional), `tests/js/plugin_loader_script_type.test.js` (guarded module injection).
- **Module-migration governance & rails (R0).** Constitution amended to **v1.2.0**: Principle II now names native ES modules as a first-class, *build-free* extension mechanism (the `scriptType:"module"` load path, both plugins and — over time — core's `static/js/`), keeping the no-bundler/no-transpiler/source-served rule intact; Operating Constraints gains a "Module load contract" clause (a `<script type=module>` load event awaits the whole static-import graph, so completion-by-`onload` is preserved; per-visit re-init comes from the `screen:changed` event, not screen.js re-execution). Mirrored into `CLAUDE.md`. New `docs/plugin-modules.md` (the migration playbook — layering, import-time purity, `import.meta.url` assets, the ETag live-edit loop) and `docs/size-exemptions.md` (the signed 1,500-line size-norm register; Byron signs core/bundled rows, Christian the authored virtuoso row). Adds a **maintainer/CI-only** ESLint gate (`eslint.config.js` + a `lint` CI job): `max-lines` warns at 1,500 as a non-blocking ratchet (ceilings for exempt files mirror the register), and `import-x/no-unresolved` + `import-x/no-cycle` hard-error on ES-module graphs — dormant until module code lands, never on the serve/Docker path.
+6 -2
View File
@@ -54,8 +54,12 @@ without a *signed* exemption" is unenforceable.
## Planned, NOT exempt (owned by split plans — listed so nothing falls between states)
core `static/app.js` (11,821) · `static/highway.js` (4,154, whole file) · `server.py`
(13,948) · `static/v3/songs.js` (4,134) · `static/capabilities/audio-session.js`
core `static/app.js` (11,852) · `static/highway.js` (4,168, whole file) · `server.py`
(8,003 — was 14,037; ratcheted by the R3 `MetadataDB` + `AudioEffectsMappingDB`
extractions and five `routers/` modules) ·
`lib/metadata_db.py` (4,373 — new in R3; the `MetadataDB` class alone is 4,018 lines
and is a monolith in its own right, to be split per-table once the router train
lands) · `static/v3/songs.js` (4,134) · `static/capabilities/audio-session.js`
(2,974) · `plugins/highway_3d/screen.js` (15,656) · `plugins/keys_highway_3d/screen.js`
(3,780) · `plugins/drum_highway_3d/screen.js` (3,597) — and every monolith with a PR
train in the refactor plan. Test files (e.g. `tests/test_plugins.py`) are out of scope
+102
View File
@@ -0,0 +1,102 @@
"""Shared application state — the seam that lets route modules reach core
singletons without importing ``server``.
``server.py`` is the host: it owns the FastAPI ``app``, constructs the DB
singletons, and runs the lifecycle. As routes move out into ``routers/`` (R3),
those modules need ``meta_db`` and friends but they must not ``import
server``, or the import graph goes circular the moment ``server`` imports them
back.
So ``server`` **injects** its singletons here once, at the point it builds them::
# server.py
meta_db = MetadataDB(CONFIG_DIR)
appstate.configure(meta_db=meta_db, ...)
and a router reads them back as **module attributes, at call time**::
# routers/artists.py
import appstate
@router.get("/api/artist/{name}/page")
def artist_page(name):
return appstate.meta_db.artist_page(name)
This is the Python analogue of the injected `configureX({...})` seams the
frontend refactor uses (stems' ``configureStreaming``, studio's
``configureAudioGraph``, the editor's ``src/host.js``), and of the plugin
``setup(app, context)`` contract in Principle III: dependencies flow one way,
``server -> routers -> appstate``, and nothing imports back up.
Two properties this shape buys, both load-bearing:
* **``import appstate`` performs no IO and constructs nothing.** ``server``
still owns construction, so the ~49 test fixtures that do
``sys.modules.pop("server")`` + re-import (to rebuild ``meta_db`` under a
patched ``CONFIG_DIR``) keep working untouched a singleton *owned* here
would survive that pop and go stale.
* **Reads are late-bound.** Routers must use ``appstate.meta_db``, never
``from appstate import meta_db`` a ``from`` import freezes the binding at
its current value, so a later ``configure()`` (or a
``monkeypatch.setattr(appstate, "meta_db", fake)``) would not reach the
router. This is the same read-only-binding trap as ES ``import``.
Defaults are ``None`` on purpose: they are inert but *type-honest*, so a router
that runs before ``configure()`` fails loudly on ``NoneType`` instead of
quietly operating on a stand-in.
Slots are added here only when a router actually needs one this is a seam,
not a grab-bag for everything in ``server.py``.
**Why this lives in ``lib/`` and not the repo root.** Because it constructs
nothing and does no import-time IO, it satisfies Principle V's rule for ``lib/``
modules and ``lib/`` is the only core directory every packaging path already
copies: the Dockerfile (``COPY lib/``), ``docker-compose.yml``, and
feedback-desktop's ``bundle-slopsmith.sh`` (``cp -r lib``). All three also put
both the bundle root and ``lib/`` on ``sys.path``. A root-level module ships in
Docker but is silently dropped from the packaged desktop app, whose bundler
copies a hardcoded file list that regression is what moved this file here.
"""
# The singletons routers may read. Every name here must also be a `_SLOTS` key.
meta_db = None
audio_effect_mappings = None
# Config paths. server.py derives these from the environment (fresh on every
# import, so the ~49 pop-and-reimport fixtures keep working) and injects them
# here. Routers read them as `appstate.config_dir` etc. — a module attribute at
# call time. NOTE: config_dir/dlc_dir are env-derived, so a `setenv`+reimport
# test reconfigures them for free; STATIC_DIR/SLOPPAK_CACHE_DIR are patched via
# `setattr(server, …)` in a few tests, so those slots (when added) need their
# tests retargeted to appstate in the same PR.
config_dir = None
dlc_dir = None # the DLC_DIR env value as a Path (Path("") if unset)
dlc_dir_env = None # the raw DLC_DIR env string, "" if unset — distinguishes
# "unset" from Path("")→"." (see dlc_paths._get_dlc_dir)
# Cache/asset dirs. static_dir + sloppak_cache_dir are patched via
# `setattr(server, …)` in a few tests, so a router reading them here needs those
# setattr sites retargeted to `setattr(appstate, …)` in the same PR (ws_highway
# retargets the 3 test_highway_ws_* SLOPPAK sites). config_dir-derived dirs are
# reconfigured for free on a setenv+reimport.
static_dir = None
sloppak_cache_dir = None
audio_cache_dir = None
_SLOTS = frozenset({
"meta_db", "audio_effect_mappings",
"config_dir", "dlc_dir", "dlc_dir_env",
"static_dir", "sloppak_cache_dir", "audio_cache_dir",
})
def configure(**kwargs) -> None:
"""Publish `server`'s singletons into this module. Called once per
`server` import (and again on re-import), so it must be idempotent."""
unknown = set(kwargs) - _SLOTS
if unknown:
raise TypeError(
f"appstate.configure() got unknown slot(s): {sorted(unknown)}. "
f"Known slots: {sorted(_SLOTS)}. Add the name to _SLOTS if a router "
f"genuinely needs it."
)
globals().update(kwargs)
+287
View File
@@ -0,0 +1,287 @@
"""Core-owned song/tone -> audio-effect-provider mapping index.
Extracted verbatim from ``server.py`` (R3). ``server.py`` still owns the
``audio_effect_mappings`` singleton; this module only supplies the class, so
nothing here touches config paths at import time the caller passes
``config_dir`` in.
"""
import json
import sqlite3
import threading
from pathlib import Path
class AudioEffectsMappingDB:
"""Core-owned public song/tone -> provider mapping index.
Providers own the preset/chain rows addressed by provider_ref. Core owns
the cross-provider routing index and the active mapping per song/tone.
"""
def __init__(self, config_dir: Path):
config_dir.mkdir(parents=True, exist_ok=True)
self.db_path = str(config_dir / "audio_effects.db")
self.conn = sqlite3.connect(self.db_path, check_same_thread=False)
self.conn.execute("PRAGMA journal_mode=WAL")
self.conn.execute("PRAGMA foreign_keys=ON")
self.conn.execute("""
CREATE TABLE IF NOT EXISTS audio_effect_mappings (
id INTEGER PRIMARY KEY AUTOINCREMENT,
song_key TEXT NOT NULL,
filename TEXT NOT NULL DEFAULT '',
tone_key TEXT NOT NULL,
provider_id TEXT NOT NULL,
provider_ref TEXT NOT NULL,
label TEXT NOT NULL DEFAULT '',
source TEXT NOT NULL DEFAULT 'manual',
metadata_json TEXT NOT NULL DEFAULT '{}',
created_at TEXT NOT NULL DEFAULT (datetime('now')),
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
UNIQUE(song_key, tone_key, provider_id)
)
""")
self.conn.execute("""
CREATE TABLE IF NOT EXISTS audio_effect_active_mappings (
song_key TEXT NOT NULL,
tone_key TEXT NOT NULL,
mapping_id INTEGER NOT NULL,
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
PRIMARY KEY (song_key, tone_key),
FOREIGN KEY (mapping_id) REFERENCES audio_effect_mappings(id) ON DELETE CASCADE
)
""")
self.conn.execute(
"CREATE INDEX IF NOT EXISTS idx_audio_effect_mappings_provider "
"ON audio_effect_mappings(provider_id)"
)
self.conn.execute(
"CREATE INDEX IF NOT EXISTS idx_audio_effect_mappings_filename "
"ON audio_effect_mappings(filename)"
)
self.conn.commit()
self._lock = threading.Lock()
@staticmethod
def _text(value, *, field: str, limit: int, allow_empty: bool = False) -> str:
if value is None:
text = ""
elif not isinstance(value, str):
raise ValueError(f"{field} must be a string")
else:
text = value.strip()
if not text and not allow_empty:
raise ValueError(f"{field} is required")
if len(text) > limit:
raise ValueError(f"{field} is too long")
return text
@staticmethod
def _mapping_id(value) -> int | None:
# Bind only values SQLite can store as an INTEGER; an out-of-range id is a
# clean miss (404), not a 500 at bind time.
if isinstance(value, int) and not isinstance(value, bool) and -(2 ** 63) <= value < 2 ** 63:
return value
return None
@staticmethod
def _field(data: dict, *keys):
# Select the first present snake/camel alias by key, not by truthiness, so a
# falsey non-string value (false/0) still reaches _text() and is rejected
# instead of being silently swallowed by an `or` chain.
for key in keys:
if key in data:
return data[key]
return None
@staticmethod
def _metadata(value) -> str:
if value is None:
return "{}"
if not isinstance(value, dict):
raise ValueError("metadata must be an object")
encoded = json.dumps(value, ensure_ascii=True, sort_keys=True)
if len(encoded) > 8192:
raise ValueError("metadata is too large")
return encoded
@staticmethod
def _row(row) -> dict | None:
if row is None:
return None
metadata = {}
try:
metadata = json.loads(row[8]) if row[8] else {}
except Exception:
metadata = {}
return {
"id": int(row[0]),
"song_key": row[1],
"filename": row[2] or "",
"tone_key": row[3],
"provider_id": row[4],
"provider_ref": row[5],
"label": row[6] or "",
"source": row[7] or "manual",
"metadata": metadata if isinstance(metadata, dict) else {},
"created_at": row[9] or "",
"updated_at": row[10] or "",
"active": bool(row[11]),
}
def _select_sql(self) -> str:
return """
SELECT m.id, m.song_key, m.filename, m.tone_key, m.provider_id,
m.provider_ref, m.label, m.source, m.metadata_json,
m.created_at, m.updated_at,
CASE WHEN a.mapping_id IS NULL THEN 0 ELSE 1 END AS active
FROM audio_effect_mappings m
LEFT JOIN audio_effect_active_mappings a
ON a.song_key = m.song_key AND a.tone_key = m.tone_key AND a.mapping_id = m.id
"""
def list(self, *, song_key: str = "", filename: str = "", tone_key: str = "", provider_id: str = "") -> list[dict]:
clauses: list[str] = []
params: list[str] = []
song_key = self._text(song_key, field="song_key", limit=240, allow_empty=True)
filename = self._text(filename, field="filename", limit=500, allow_empty=True)
tone_key = self._text(tone_key, field="tone_key", limit=160, allow_empty=True)
provider_id = self._text(provider_id, field="provider_id", limit=96, allow_empty=True)
if song_key and filename:
clauses.append("(m.song_key = ? OR m.filename = ?)")
params.extend([song_key, filename])
elif song_key:
clauses.append("m.song_key = ?")
params.append(song_key)
elif filename:
clauses.append("(m.song_key = ? OR m.filename = ?)")
params.extend([filename, filename])
if tone_key:
clauses.append("m.tone_key = ?")
params.append(tone_key)
if provider_id:
clauses.append("m.provider_id = ?")
params.append(provider_id)
sql = self._select_sql()
if clauses:
sql += " WHERE " + " AND ".join(clauses)
sql += " ORDER BY m.song_key COLLATE NOCASE, m.tone_key COLLATE NOCASE, m.provider_id COLLATE NOCASE"
with self._lock:
rows = self.conn.execute(sql, params).fetchall()
return [self._row(row) for row in rows]
def get(self, mapping_id: int) -> dict | None:
mapping_id = self._mapping_id(mapping_id)
if mapping_id is None:
return None
with self._lock:
row = self.conn.execute(self._select_sql() + " WHERE m.id = ?", (mapping_id,)).fetchone()
return self._row(row)
def upsert(self, data: dict) -> dict:
if not isinstance(data, dict):
raise ValueError("mapping body must be an object")
filename = self._text(data.get("filename", ""), field="filename", limit=500, allow_empty=True)
song_key_raw = self._field(data, "song_key", "songKey")
if song_key_raw is None or song_key_raw == "":
song_key_raw = filename
song_key = self._text(song_key_raw, field="song_key", limit=240)
tone_key = self._text(self._field(data, "tone_key", "toneKey"), field="tone_key", limit=160, allow_empty=True)
provider_id = self._text(self._field(data, "provider_id", "providerId"), field="provider_id", limit=96)
provider_ref = self._text(self._field(data, "provider_ref", "providerRef"), field="provider_ref", limit=240)
label = self._text(data.get("label", ""), field="label", limit=160, allow_empty=True)
source = self._text(data.get("source", "manual"), field="source", limit=40, allow_empty=True) or "manual"
metadata_json = self._metadata(data.get("metadata", {}))
with self._lock:
self.conn.execute(
"""
INSERT INTO audio_effect_mappings
(song_key, filename, tone_key, provider_id, provider_ref, label, source, metadata_json, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, datetime('now'))
ON CONFLICT(song_key, tone_key, provider_id) DO UPDATE SET
-- Only overwrite filename when a non-empty one was supplied; an
-- omitted/empty filename must preserve the stored value (it's an
-- alternate lookup key for list(..., filename=...)).
filename=CASE WHEN excluded.filename <> '' THEN excluded.filename ELSE audio_effect_mappings.filename END,
provider_ref=excluded.provider_ref,
label=excluded.label,
source=excluded.source,
metadata_json=excluded.metadata_json,
updated_at=datetime('now')
""",
(song_key, filename, tone_key, provider_id, provider_ref, label, source, metadata_json),
)
row = self.conn.execute(
"SELECT id FROM audio_effect_mappings WHERE song_key = ? AND tone_key = ? AND provider_id = ?",
(song_key, tone_key, provider_id),
).fetchone()
if row is None:
raise ValueError("failed to create audio-effects mapping")
mapping_id = int(row[0])
if data.get("active") is True:
self.conn.execute(
"""
INSERT INTO audio_effect_active_mappings (song_key, tone_key, mapping_id, updated_at)
VALUES (?, ?, ?, datetime('now'))
ON CONFLICT(song_key, tone_key) DO UPDATE SET
mapping_id=excluded.mapping_id,
updated_at=datetime('now')
""",
(song_key, tone_key, mapping_id),
)
self.conn.commit()
return self.get(mapping_id)
def delete(self, mapping_id: int, *, provider_id: str = "") -> bool:
mapping_id = self._mapping_id(mapping_id)
if mapping_id is None:
return False
provider_id = self._text(provider_id, field="provider_id", limit=96, allow_empty=True)
with self._lock:
if provider_id:
cur = self.conn.execute(
"DELETE FROM audio_effect_mappings WHERE id = ? AND provider_id = ?",
(mapping_id, provider_id),
)
else:
cur = self.conn.execute("DELETE FROM audio_effect_mappings WHERE id = ?", (mapping_id,))
self.conn.commit()
return cur.rowcount > 0
def activate(self, mapping_id: int, *, provider_id: str = "") -> dict | None:
mapping_id = self._mapping_id(mapping_id)
if mapping_id is None:
return None
provider_id = self._text(provider_id, field="provider_id", limit=96, allow_empty=True)
with self._lock:
row = self.conn.execute(
self._select_sql() + " WHERE m.id = ?",
(mapping_id,),
).fetchone()
mapping = self._row(row)
if not mapping or (provider_id and mapping["provider_id"] != provider_id):
return None
self.conn.execute(
"""
INSERT INTO audio_effect_active_mappings (song_key, tone_key, mapping_id, updated_at)
VALUES (?, ?, ?, datetime('now'))
ON CONFLICT(song_key, tone_key) DO UPDATE SET
mapping_id=excluded.mapping_id,
updated_at=datetime('now')
""",
(mapping["song_key"], mapping["tone_key"], mapping_id),
)
self.conn.commit()
selected = self.conn.execute(self._select_sql() + " WHERE m.id = ?", (mapping_id,)).fetchone()
return self._row(selected)
def clear_active(self, *, song_key: str, tone_key: str) -> bool:
song_key = self._text(song_key, field="song_key", limit=240)
tone_key = self._text(tone_key, field="tone_key", limit=160, allow_empty=True)
with self._lock:
cur = self.conn.execute(
"DELETE FROM audio_effect_active_mappings WHERE song_key = ? AND tone_key = ?",
(song_key, tone_key),
)
self.conn.commit()
return cur.rowcount > 0
+99
View File
@@ -0,0 +1,99 @@
"""DLC library path resolution — where the song files live, plus safe containment.
Extracted from ``server.py`` (R3). ``_resolve_dlc_path`` is pure and moved
verbatim. ``_get_dlc_dir`` reads the env-derived paths through the ``appstate``
seam (``server.py`` configures ``dlc_dir``/``dlc_dir_env``/``config_dir`` at
import, fresh on every re-import), so this module does no import-time IO and the
pop-and-reimport fixtures keep working. ``server.py`` re-exports both names, so
existing ``server._get_dlc_dir`` / ``server._resolve_dlc_path`` references
(tests, other handlers) resolve unchanged.
"""
import json
import os
from pathlib import Path
import appstate
def _get_dlc_dir(cfg: dict | None = None) -> Path | None:
# Only consider DLC_DIR if the env var was non-empty. `Path("")` collapses
# to `.` and reports `.is_dir() == True`, which would silently shadow the
# config.json fallback. Checking the raw env string preserves
# `DLC_DIR=.` as a valid opt-in for cwd while keeping unset/empty out.
if appstate.dlc_dir_env and appstate.dlc_dir.is_dir():
return appstate.dlc_dir
if cfg is None:
config_file = appstate.config_dir / "config.json"
if config_file.exists():
try:
cfg = json.loads(config_file.read_text(encoding="utf-8"))
except Exception:
pass
if isinstance(cfg, dict):
raw = str(cfg.get("dlc_dir", "")).strip()
if raw:
p = Path(raw)
if p.is_dir():
return p
return None
def _resolve_dlc_path(dlc: Path, filename: str) -> Path | None:
"""Resolve `filename` under DLC_DIR and refuse anything that escapes.
`filename` arrives from `:path` route params and can contain `..`
segments. The Sloppak and archive paths happen to fail safely later
because their loaders raise on missing/invalid files, but loose-
folder format detection (`is_loose_song`) globs and parses XML on
disk first, which lets a crafted path trigger filesystem reads
outside DLC_DIR before any guard fires. Centralise the containment
check so every filename-bound handler validates before touching the
filesystem.
Containment here is LEXICAL (normalize `.`/`..` WITHOUT following
symlinks), not `safe_join`'s `.resolve()`-based check — because users
commonly mount their song library through a directory JUNCTION/symlink
(a library shared across app installs; the desktop app's own mounts).
`.resolve()` follows that junction to its real target, sees it sits
outside DLC_DIR, and wrongly rejects every song reached through it the
scanner's `rglob` indexes those songs, but art/load then 403/404s (broken
covers, unplayable songs). Lexical normalization still rejects the only
escapes a `:path` filename can express `..` traversal and absolute
paths which the traversal tests pin. `safe_join` stays strict (it is
the zip-slip / plugin-asset guard, where following a symlink out IS the
defense); the loose-folder art handler keeps its own per-file symlink
re-check for defence-in-depth.
Returns the validated Path (not necessarily link-resolved), or None if
the filename is empty, contains a NUL, or escapes the DLC root.
"""
if not filename:
return None
# Backslashes → forward slashes so a Windows-style `..\\x` traversal is
# rejected identically on POSIX (mirrors safe_join's normalisation).
safe = filename.replace("\\", "/")
if "\x00" in safe:
return None
# Reject drive-letter / absolute paths in BOTH conventions. A POSIX "/x" is
# caught by the containment check below (the `/` operator discards `root`),
# but a Windows drive-absolute "C:/x" is treated as a relative "C:" dir on
# POSIX and would otherwise slip in as `<root>/C:/x` — so the contract must
# hold cross-platform (a shared library is reached from either OS).
from pathlib import PurePosixPath, PureWindowsPath
if (PurePosixPath(safe).is_absolute()
or PureWindowsPath(safe).is_absolute()
or PureWindowsPath(safe).drive):
return None
try:
root = dlc.resolve()
# normpath collapses `.`/`..`/duplicate separators purely lexically —
# it never touches the filesystem, so an in-library junction component
# is preserved (allowed) while `..`/absolute segments still escape and
# get caught by the containment check below.
candidate = Path(os.path.normpath(root / safe))
if not candidate.is_relative_to(root):
return None
except (ValueError, OSError):
return None
return candidate
+10 -3
View File
@@ -1114,7 +1114,7 @@ def _build_xml(
ET.SubElement(root, "arrangement").text = arrangement
ET.SubElement(root, "offset").text = f"{audio_offset:.3f}"
ET.SubElement(root, "songLength").text = f"{song_length:.3f}"
ET.SubElement(root, "startBeat").text = f"{beats[0].time:.3f}" if beats else "0.000"
ET.SubElement(root, "startBeat").text = f"{beats[0].time:.6f}" if beats else "0.000000"
ET.SubElement(root, "averageTempo").text = str(tempo)
ET.SubElement(root, "artistName").text = artist
ET.SubElement(root, "albumName").text = album
@@ -1139,10 +1139,17 @@ def _build_xml(
tuning_el.set(f"string{i}", str(tuning[i] if i < len(tuning) else 0))
ET.SubElement(root, "capo").text = "0"
# Ebeats
# Ebeats — write beat times at MICROSECOND (6-decimal) precision, not
# millisecond (3-decimal). The editor/timeline DERIVES per-bar BPM from beat
# spans (bpm = beats·60/span), which amplifies any rounding: at 3 decimals a
# constant-tempo GP (e.g. 140) shows a spurious ±0.050.7 BPM per-bar drift
# (worse for fast/odd meters) because most bar lengths don't land on a ms
# boundary. gp2rs computes these times exactly from the GP tempo map, so the
# only loss is this format string — 6 decimals makes the derived tempo match
# GP's authored value. (Everything else stays at :.3f; only beats drive tempo.)
ebeats = ET.SubElement(root, "ebeats", count=str(len(beats)))
for b in beats:
ET.SubElement(ebeats, "ebeat", time=f"{b.time:.3f}", measure=str(b.measure))
ET.SubElement(ebeats, "ebeat", time=f"{b.time:.6f}", measure=str(b.measure))
# Sections
sections_el = ET.SubElement(root, "sections", count=str(len(sections)))
+4373
View File
File diff suppressed because it is too large Load Diff
+13
View File
@@ -0,0 +1,13 @@
"""Request-field coercion helpers shared by the raw-`dict` POST handlers.
Extracted verbatim from ``server.py`` (R3). Pure no IO, no globals so it
imports cleanly from both ``server`` and any ``routers/`` module.
"""
def _clean_str(value) -> str:
"""Trim a request field to a string; non-strings (or missing) → ''.
Lets the raw-`dict` POST handlers treat wrong-typed JSON (an int/list/etc.
where a string was expected) as "empty" and answer 400, instead of raising
AttributeError/TypeError 500 on a later .strip()/`in`."""
return value.strip() if isinstance(value, str) else ""
+31
View File
@@ -0,0 +1,31 @@
"""FastAPI route modules extracted from ``server.py`` (R3).
Each module here exposes a module-level ``router`` (a ``fastapi.APIRouter``)
that ``server.py`` mounts with ``app.include_router(...)`` at the point in the
file where those routes used to be defined FastAPI matches routes in
registration order, so keeping the mount site preserves it.
**Routers must never ``import server``.** They reach core singletons through
the injected seam instead::
import appstate
@router.get("/api/thing")
def get_thing():
return appstate.meta_db.thing()
and always as a **module attribute, at call time** never
``from appstate import meta_db``, which freezes the binding and defeats both a
later ``appstate.configure()`` and ``monkeypatch.setattr``. See ``appstate.py``.
Dependencies flow one way: ``server -> routers -> appstate``.
**Why this lives under ``lib/``.** ``lib/`` is the only core directory every
packaging path already copies wholesale the Dockerfile (``COPY lib/``),
``docker-compose.yml``, and feedback-desktop's ``bundle-slopsmith.sh``
(``cp -r lib``) and all three put it on ``sys.path``. A root-level package
ships in Docker but is silently dropped from the packaged desktop app, whose
bundler copies a hardcoded file list. Route modules import nothing at module
scope beyond FastAPI and ``appstate``, so they do no import-time IO and satisfy
Principle V's rule for ``lib/``.
"""
+68
View File
@@ -0,0 +1,68 @@
"""Artist aliases / Tidy-up (P4) — canonicalize messy artist tags at DISPLAY
("ACDC" -> "AC/DC") without touching feedpak files or the scanner-derived
songs.artist. All DB-only.
Extracted verbatim from ``server.py`` (R3); only the decorator receiver
(``@app`` -> ``@router``) and the singleton read (``meta_db`` ->
``appstate.meta_db``) changed. The read stays a module attribute so a re-imported
``server`` re-publishes a fresh DB into the seam see ``appstate.py``.
"""
from fastapi import APIRouter
from fastapi.responses import JSONResponse
import appstate
router = APIRouter()
@router.get("/api/artist-aliases")
def list_artist_aliases():
"""Existing raw→canonical overrides (the Tidy-up 'current merges' list)."""
return {"aliases": appstate.meta_db.list_artist_aliases()}
@router.get("/api/artists/raw")
def list_raw_artists(limit: int = 2000):
"""Distinct RAW artist names + song counts + current canonical — the Tidy-up
picker (you merge raw variants into one canonical)."""
return {"artists": appstate.meta_db.raw_artists(limit)}
@router.post("/api/artist-aliases")
def set_artist_alias(data: dict):
"""Upsert one override: {raw_name, canonical_name, mb_artist_id?}. A self-alias
(raw == canonical) clears the row instead (un-merge)."""
raw = (data.get("raw_name") or "").strip()
canon = (data.get("canonical_name") or "").strip()
if not raw or not canon:
return JSONResponse({"error": "raw_name and canonical_name are required"}, 400)
result = appstate.meta_db.set_artist_alias(raw, canon, (data.get("mb_artist_id") or None))
if not result.get("ok"):
# Would form a cycle (raw → … → raw) — refuse rather than corrupt the chain.
return JSONResponse(
{"error": "alias would create a cycle", "raw_name": raw, "canonical_name": canon},
409)
return {"ok": True, "raw_name": raw, "canonical_name": result.get("canonical_name", canon)}
@router.post("/api/artist-aliases/merge")
def merge_artist_aliases(data: dict):
"""Merge several raw artist variants into one canonical:
{raw_names: [...], canonical_name}. The canonical's own self-alias is skipped.
Returns {merged: N}."""
canon = (data.get("canonical_name") or "").strip()
raws = data.get("raw_names")
if not canon:
return JSONResponse({"error": "canonical_name is required"}, 400)
if not isinstance(raws, list) or not raws:
return JSONResponse({"error": "raw_names must be a non-empty array"}, 400)
n = appstate.meta_db.merge_artists(raws, canon)
return {"merged": n, "canonical_name": canon}
@router.delete("/api/artist-aliases/{raw_name:path}")
def delete_artist_alias(raw_name: str):
"""Remove one override so that raw artist stands on its own again."""
appstate.meta_db.remove_artist_alias(raw_name)
return {"ok": True}
+80
View File
@@ -0,0 +1,80 @@
"""Audio-effects mapping API — the core-owned song/tone -> provider routing index.
Extracted verbatim from ``server.py`` (R3); only the decorator receiver
(``@app`` -> ``@router``) and the singleton read (``audio_effect_mappings`` ->
``appstate.audio_effect_mappings``) changed. The read must stay a module
attribute so a re-imported ``server`` re-publishes a fresh DB into the seam and
`monkeypatch.setattr` reaches this module see ``appstate.py``.
"""
from fastapi import APIRouter, Body, Query
from fastapi.responses import JSONResponse
import appstate
router = APIRouter()
def _audio_effects_error(exc: Exception):
return JSONResponse({"error": str(exc)}, status_code=400)
@router.get("/api/audio-effects/mappings")
def list_audio_effect_mappings(
song_key: str = Query(""),
filename: str = Query(""),
tone_key: str = Query(""),
provider_id: str = Query(""),
):
try:
return {
"mappings": appstate.audio_effect_mappings.list(
song_key=song_key,
filename=filename,
tone_key=tone_key,
provider_id=provider_id,
)
}
except ValueError as exc:
return _audio_effects_error(exc)
@router.post("/api/audio-effects/mappings")
def upsert_audio_effect_mapping(data: dict = Body(...)):
try:
mapping = appstate.audio_effect_mappings.upsert(data)
except ValueError as exc:
return _audio_effects_error(exc)
return {"ok": True, "mapping": mapping}
@router.delete("/api/audio-effects/mappings/{mapping_id}")
def delete_audio_effect_mapping(mapping_id: int, provider_id: str = Query("")):
try:
deleted = appstate.audio_effect_mappings.delete(mapping_id, provider_id=provider_id)
except ValueError as exc:
return _audio_effects_error(exc)
if not deleted:
return JSONResponse({"error": "mapping not found"}, status_code=404)
return {"ok": True}
@router.post("/api/audio-effects/mappings/{mapping_id}/activate")
def activate_audio_effect_mapping(mapping_id: int, data: dict = Body(default_factory=dict)):
try:
provider_id = data.get("provider_id") if "provider_id" in data else data.get("providerId")
mapping = appstate.audio_effect_mappings.activate(mapping_id, provider_id="" if provider_id is None else provider_id)
except ValueError as exc:
return _audio_effects_error(exc)
if not mapping:
return JSONResponse({"error": "mapping not found"}, status_code=404)
return {"ok": True, "mapping": mapping}
@router.delete("/api/audio-effects/active-mapping")
def clear_audio_effect_active_mapping(song_key: str = Query(...), tone_key: str = Query("")):
try:
cleared = appstate.audio_effect_mappings.clear_active(song_key=song_key, tone_key=tone_key)
except ValueError as exc:
return _audio_effects_error(exc)
return {"ok": True, "cleared": cleared}
+60
View File
@@ -0,0 +1,60 @@
"""Practice loops — saved A/B regions per song.
Extracted verbatim from ``server.py`` (R3); only the decorator receiver
(``@app`` -> ``@router``) and the singleton reads (``meta_db`` ->
``appstate.meta_db``) changed. See ``appstate.py`` for why the reads stay
module attributes.
"""
from fastapi import APIRouter
import appstate
router = APIRouter()
@router.get("/api/loops")
def list_loops(filename: str):
# Hold the DB lock for the read: the shared single connection
# (check_same_thread=False) is serialized through meta_db._lock by every
# writer, so an unlocked SELECT here can overlap a POST/DELETE commit.
db = appstate.meta_db
with db._lock:
rows = db.conn.execute(
"SELECT id, name, start_time, end_time FROM loops WHERE filename = ? ORDER BY start_time",
(filename,)
).fetchall()
return [{"id": r[0], "name": r[1], "start": r[2], "end": r[3]} for r in rows]
@router.post("/api/loops")
def save_loop(data: dict):
filename = data.get("filename", "")
name = data.get("name", "").strip()
start = data.get("start")
end = data.get("end")
if not filename or start is None or end is None:
return {"error": "Missing fields"}
db = appstate.meta_db
with db._lock:
# COUNT + INSERT under one lock so two unnamed POSTs can't read the same
# count and both mint "Loop N" (the count is only used to name the row).
if not name:
count = db.conn.execute(
"SELECT COUNT(*) FROM loops WHERE filename = ?", (filename,)
).fetchone()[0]
name = f"Loop {count + 1}"
db.conn.execute(
"INSERT INTO loops (filename, name, start_time, end_time) VALUES (?, ?, ?, ?)",
(filename, name, float(start), float(end))
)
db.conn.commit()
return {"ok": True, "name": name}
@router.delete("/api/loops/{loop_id}")
def delete_loop(loop_id: int):
with appstate.meta_db._lock:
appstate.meta_db.conn.execute("DELETE FROM loops WHERE id = ?", (loop_id,))
appstate.meta_db.conn.commit()
return {"ok": True}
+267
View File
@@ -0,0 +1,267 @@
"""Playlists + custom playlist covers (fee[dB]ack v0.3.0).
Extracted verbatim from ``server.py`` (R3). Edits: ``@app`` -> ``@router``,
``meta_db`` -> ``appstate.meta_db``, ``CONFIG_DIR`` -> ``appstate.config_dir``
(both read at call time through the seam), and ``_clean_str`` now imports from
``reqfields``. See ``appstate.py``.
"""
import logging
import os
import tempfile
from pathlib import Path
from fastapi import APIRouter
from fastapi.responses import FileResponse, JSONResponse
import appstate
from reqfields import _clean_str
log = logging.getLogger("feedBack.server")
router = APIRouter()
# Cache policy for the custom-cover file response: revalidate every time so a
# replaced cover is never served stale (pairs with the mtime-ns URL token).
_ART_CACHE_HEADERS = {"Cache-Control": "no-cache"}
def _playlist_cover_path(pid) -> Path | None:
"""Filesystem path of a playlist's optional custom cover image (PNG),
stored under CONFIG_DIR. Returns None for a non-integer id."""
try:
pid = int(pid)
except (TypeError, ValueError):
return None
return appstate.config_dir / "playlist_covers" / f"{pid}.png"
def _playlist_cover_url(pid) -> str | None:
cover = _playlist_cover_path(pid)
if not cover or not cover.exists():
return None
try:
# Nanosecond mtime so a same-second replace/remove/re-upload still
# changes the cache-bust token (int seconds could collide → stale image).
mt = cover.stat().st_mtime_ns
except OSError:
mt = 0
return f"/api/playlists/{pid}/cover?v={mt}"
@router.get("/api/playlists")
def api_list_playlists():
lists = appstate.meta_db.list_playlists()
for pl in lists:
pl["cover_url"] = _playlist_cover_url(pl["id"])
return lists
@router.post("/api/playlists")
def api_create_playlist(data: dict):
name = _clean_str(data.get("name"))
if not (1 <= len(name) <= 100):
return JSONResponse({"error": "Playlist name must be 1100 characters."}, status_code=400)
# kind='album' = a curated album (§7.2): hand-picked works, a chosen chart
# per slot, played front-to-back on the queue. Absent/None = a regular mix.
kind = _clean_str(data.get("kind")) or None
if kind not in (None, "album"):
return JSONResponse({"error": "kind must be 'album' or omitted"}, status_code=400)
return appstate.meta_db.create_playlist(name, kind=kind)
@router.get("/api/playlists/{pid}")
def api_get_playlist(pid: int):
pl = appstate.meta_db.get_playlist(pid)
if pl is None:
return JSONResponse({"error": "not found"}, status_code=404)
pl["cover_url"] = _playlist_cover_url(pid)
return pl
@router.patch("/api/playlists/{pid}")
def api_rename_playlist(pid: int, data: dict):
pl = appstate.meta_db.get_playlist(pid)
if pl is None:
return JSONResponse({"error": "not found"}, status_code=404)
if pl["system_key"]:
return JSONResponse({"error": "System playlists cannot be renamed."}, status_code=400)
name = _clean_str(data.get("name"))
if not (1 <= len(name) <= 100):
return JSONResponse({"error": "Playlist name must be 1100 characters."}, status_code=400)
appstate.meta_db.rename_playlist(pid, name)
return appstate.meta_db.get_playlist(pid)
@router.delete("/api/playlists/{pid}")
def api_delete_playlist(pid: int):
pl = appstate.meta_db.get_playlist(pid)
if pl is None:
return JSONResponse({"error": "not found"}, status_code=404)
if pl["system_key"]:
return JSONResponse({"error": "System playlists cannot be deleted."}, status_code=400)
if not appstate.meta_db.delete_playlist(pid): # vanished under us (concurrent delete)
return JSONResponse({"error": "not found"}, status_code=404)
cover = _playlist_cover_path(pid) # drop any custom cover with the playlist
if cover and cover.exists():
try:
cover.unlink()
except OSError:
pass
return {"ok": True}
@router.post("/api/playlists/{pid}/songs")
def api_add_playlist_song(pid: int, data: dict):
if appstate.meta_db.get_playlist(pid) is None:
return JSONResponse({"error": "not found"}, status_code=404)
filename = _clean_str(data.get("filename"))
if not filename:
return JSONResponse({"error": "filename required"}, status_code=400)
if appstate.meta_db.add_playlist_song(pid, filename) is None: # playlist vanished under us
return JSONResponse({"error": "not found"}, status_code=404)
pl = appstate.meta_db.get_playlist(pid)
return pl if pl is not None else JSONResponse({"error": "not found"}, status_code=404)
@router.patch("/api/playlists/{pid}/songs/{filename:path}")
def api_update_playlist_slot(pid: int, filename: str, data: dict):
"""Edit one curated-album slot: {"arrangement": name|null} pins/clears the
slot's arrangement; {"chart_filename": fn} swaps the slot to another chart
of the same work (position + pin kept). Albums only a mix has no slots."""
pl = appstate.meta_db.get_playlist(pid)
if pl is None:
return JSONResponse({"error": "not found"}, status_code=404)
if pl.get("kind") != "album":
return JSONResponse({"error": "Slot editing is for albums."}, status_code=400)
kwargs = {}
if "chart_filename" in data:
new_fn = _clean_str(data.get("chart_filename"))
if not new_fn:
return JSONResponse({"error": "chart_filename must be a filename"}, status_code=400)
kwargs["new_filename"] = new_fn
if "arrangement" in data:
arr = data.get("arrangement")
if arr is not None and not (isinstance(arr, str) and 1 <= len(arr.strip()) <= 100):
return JSONResponse({"error": "arrangement must be a name or null"}, status_code=400)
kwargs["arrangement"] = arr.strip() if isinstance(arr, str) else None
if not kwargs:
return JSONResponse({"error": "nothing to update"}, status_code=400)
if appstate.meta_db.update_playlist_slot(pid, filename, **kwargs) is None:
return JSONResponse(
{"error": "no such slot, or the chart isn't a version of this song"},
status_code=400)
return appstate.meta_db.get_playlist(pid)
@router.delete("/api/playlists/{pid}/songs/{filename:path}")
def api_remove_playlist_song(pid: int, filename: str):
if appstate.meta_db.get_playlist(pid) is None:
return JSONResponse({"error": "not found"}, status_code=404)
appstate.meta_db.remove_playlist_song(pid, filename)
pl = appstate.meta_db.get_playlist(pid)
return pl if pl is not None else JSONResponse({"error": "not found"}, status_code=404)
@router.post("/api/playlists/{pid}/reorder")
def api_reorder_playlist(pid: int, data: dict):
pl = appstate.meta_db.get_playlist(pid)
if pl is None:
return JSONResponse({"error": "not found"}, status_code=404)
order = data.get("order")
if not isinstance(order, list) or not all(isinstance(f, str) for f in order):
return JSONResponse({"error": "order must be a list of filenames"}, status_code=400)
# Require an exact permutation of the playlist's current songs: a list with
# duplicates, omissions, or extras would otherwise produce duplicate
# positions / a partial reorder while still returning 200.
current = [s["filename"] for s in pl["songs"]]
if len(order) != len(current) or sorted(order) != sorted(current):
return JSONResponse(
{"error": "order must be a permutation of the playlist's current songs"},
status_code=400,
)
appstate.meta_db.reorder_playlist(pid, order)
return appstate.meta_db.get_playlist(pid)
@router.post("/api/playlists/{pid}/cover")
async def api_set_playlist_cover(pid: int, data: dict):
"""Set a playlist's custom cover from a base64 / data-URL image (PNG/JPG).
Overrides the content-dependent (song-art) cover. Stored as a small PNG
thumbnail under CONFIG_DIR/playlist_covers/."""
if appstate.meta_db.get_playlist(pid) is None:
return JSONResponse({"error": "not found"}, status_code=404)
import base64
import io
b64 = data.get("image", "")
# Guard the type before the `","` membership test — a non-string image
# (e.g. {"image": 123} / null) would otherwise raise TypeError → 500.
# Mirrors the avatar/song-art upload guard.
if not isinstance(b64, str) or not b64:
return JSONResponse({"error": "No image data"}, status_code=400)
if "," in b64:
b64 = b64.split(",", 1)[1]
if not b64:
return JSONResponse({"error": "No image data"}, status_code=400)
try:
img_data = base64.b64decode(b64)
except Exception:
return JSONResponse({"error": "Invalid base64"}, status_code=400)
cover = _playlist_cover_path(pid)
cover.parent.mkdir(parents=True, exist_ok=True)
# Decode/validate the image — a bad payload is a CLIENT error (400), and the
# message stays generic so it can't echo internals.
try:
from PIL import Image
img = Image.open(io.BytesIO(img_data)).convert("RGB")
img.thumbnail((640, 640)) # covers stay small
except Exception:
return JSONResponse({"error": "Invalid image"}, status_code=400)
# Persist. A save/replace failure is a SERVER error (500, logged, no
# filesystem detail leaked) — the pre-split handler mislabeled these as 400
# and echoed the exception. A unique temp name in the cover dir (not a shared
# `{pid}.png.tmp`) means two concurrent uploads can't clobber each other's
# temp file; the atomic replace publishes. Re-check the playlist still exists
# just before publishing so a delete that raced the decode above can't leave
# an orphan cover — cheap belt-and-braces; FeedBack is single-user
# (Principle I), so a full per-playlist lock would be for a race the
# deployment model precludes.
tmp = None
try:
# mkstemp is inside the try too: an unwritable dir / full disk raises
# here, and that's the same class of persistence failure as save/replace.
fd, tmp_name = tempfile.mkstemp(prefix=f".{pid}.", suffix=".png.tmp", dir=str(cover.parent))
tmp = Path(tmp_name)
with os.fdopen(fd, "wb") as f:
img.save(f, "PNG")
if appstate.meta_db.get_playlist(pid) is None:
tmp.unlink(missing_ok=True)
return JSONResponse({"error": "not found"}, status_code=404)
tmp.replace(cover)
except Exception:
if tmp is not None:
tmp.unlink(missing_ok=True)
log.exception("playlist cover save failed (pid=%s)", pid)
return JSONResponse({"error": "could not save cover"}, status_code=500)
return {"ok": True, "cover_url": _playlist_cover_url(pid)}
@router.get("/api/playlists/{pid}/cover")
def api_get_playlist_cover(pid: int):
cover = _playlist_cover_path(pid)
if not cover or not cover.exists():
return JSONResponse({"error": "not found"}, status_code=404)
# no-cache (revalidate) like song art, so a replaced cover is never served
# stale — pairs with the mtime-ns cache-bust token on the URL.
return FileResponse(str(cover), media_type="image/png", headers=_ART_CACHE_HEADERS)
@router.delete("/api/playlists/{pid}/cover")
def api_delete_playlist_cover(pid: int):
cover = _playlist_cover_path(pid)
if cover and cover.exists():
try:
cover.unlink()
except OSError:
pass
return {"ok": True}
File diff suppressed because it is too large Load Diff
+1 -1
View File
@@ -3,7 +3,7 @@
This module is deliberately kept apart from ``server.py`` so that
``ProcessPoolExecutor`` workers can import and unpickle ``_scan_one``
without dragging in ``server.py``'s import-time side effects
(``configure_logging()``, ``meta_db = MetadataDB()`` opening/migrating
(``configure_logging()``, ``meta_db = MetadataDB(CONFIG_DIR)`` opening/migrating
SQLite, and ``register_plugin_api(app)`` registering routes).
The background scan spawns its pool with the ``spawn`` start method (see
+16
View File
@@ -98,6 +98,22 @@ def open_midis_to_freqs(midis: list[int], reference_pitch: float = DEFAULT_REFER
return [round(midi_to_freq(m, reference_pitch), 2) for m in midis]
def freqs_to_midis(freqs: list[float], reference_pitch: float = DEFAULT_REFERENCE_PITCH) -> list[int] | None:
"""Return absolute open-string MIDI notes for frequencies at the supplied
A4 reference the inverse of open_midis_to_freqs. None if any entry is
non-numeric or non-positive (a provider could hand us anything)."""
out: list[int] = []
for f in freqs:
try:
f = float(f)
except (TypeError, ValueError):
return None
if f <= 0:
return None
out.append(int(round(69 + 12 * math.log2(f / reference_pitch))))
return out
def tuning_offsets_from_midis(instrument_key: str, midis: list[int]) -> list[int] | None:
"""Return semitone offsets from the instrument's standard open strings."""
standard = STANDARD_OPEN_MIDIS.get(instrument_key)
+1 -1
View File
@@ -1,7 +1,7 @@
{
"id": "highway_3d",
"name": "3D Highway",
"version": "3.31.4",
"version": "3.31.5",
"type": "visualization",
"bundled": true,
"script": "screen.js",
+36 -11
View File
@@ -548,9 +548,20 @@
ctrl.ownsActx = !(fogAudio && fogAudio.ctx);
ctrl.actx = (fogAudio && fogAudio.ctx) || new Ctx();
if (ctrl.actx.state === 'suspended' && ctrl.actx.resume) ctrl.actx.resume().catch(() => {});
// Seed the DRAWING BUFFER (canvas.width/height) to the device-pixel
// render size and report that SAME size to Butterchurn. Its on-screen
// pass viewports to the reported size but never sizes the output canvas
// itself — leaving the buffer at the 300x150 default blits the whole
// visualizer into a corner that CSS then stretches across the highway.
// pixelRatio:1 because DPR is now folded into the reported size, so
// buffer == viewport == internal texsize (no double-counting).
const _bcRatio0 = Math.min(window.devicePixelRatio || 1, 1.5);
const _bcW0 = Math.max(1, Math.round((sz.w || 1280) * _bcRatio0));
const _bcH0 = Math.max(1, Math.round((sz.h || 720) * _bcRatio0));
canvas.width = _bcW0; canvas.height = _bcH0;
ctrl.viz = bc.createVisualizer(ctrl.actx, canvas, {
width: sz.w || 1280, height: sz.h || 720,
pixelRatio: Math.min(window.devicePixelRatio || 1, 1.5), textureRatio: 1,
width: _bcW0, height: _bcH0,
pixelRatio: 1, textureRatio: 1,
});
if (_bcIsDesktop()) {
try {
@@ -584,6 +595,27 @@
ctrl.actx = null; ctrl.viz = null; ctrl.dead = true;
_bcControllers.delete(ctrl);
});
// Size the Butterchurn output: set the canvas DRAWING BUFFER to the
// device-pixel render size AND report that same size, so buffer ==
// on-screen viewport == full fill. Butterchurn never sizes the output
// canvas itself; the previous code set only CSS size, leaving the buffer
// at the 300x150 default -> the viz showed a stretched lower-left corner
// (worse the larger the panel). Ratio reuses the highway's DPR budget.
function _bcApplySize(cssW, cssH) {
if (!(cssW > 0 && cssH > 0)) return;
ctrl.lastW = cssW; ctrl.lastH = cssH;
const ratio = Math.min(window.devicePixelRatio || 1, 1.5);
const bw = Math.max(1, Math.round(cssW * ratio)), bh = Math.max(1, Math.round(cssH * ratio));
if (canvas.width !== bw) canvas.width = bw;
if (canvas.height !== bh) canvas.height = bh;
const wpx = cssW + 'px', hpx = cssH + 'px';
// Confine ALL layers to exactly the highway-canvas rect so the opaque
// backdrop can't bleed over the transport bar above the highway.
[ctrl.canvas, ctrl.backdrop, ctrl.scrim, ctrl.tint].forEach((el) => {
if (el) { el.style.width = wpx; el.style.height = hpx; el.style.right = 'auto'; el.style.bottom = 'auto'; }
});
if (ctrl.viz && ctrl.viz.setRendererSize) { try { ctrl.viz.setRendererSize(bw, bh); } catch (e) {} }
}
return {
applySettings() { ctrl.applySettings(); },
dead() { return ctrl.dead; },
@@ -612,18 +644,11 @@
if (!ctrl.viz || !s.enabled) return; // skip GPU work when the bg is off
const sz = sizeProvider && sizeProvider();
if (sz && sz.w > 0 && sz.h > 0 && (sz.w !== ctrl.lastW || sz.h !== ctrl.lastH)) {
ctrl.lastW = sz.w; ctrl.lastH = sz.h;
const wpx = sz.w + 'px', hpx = sz.h + 'px';
// Confine ALL layers to exactly the highway-canvas rect so the opaque
// backdrop can't bleed over the transport bar above the highway.
[ctrl.canvas, ctrl.backdrop, ctrl.scrim, ctrl.tint].forEach((el) => {
if (el) { el.style.width = wpx; el.style.height = hpx; el.style.right = 'auto'; el.style.bottom = 'auto'; }
});
try { ctrl.viz.setRendererSize(sz.w, sz.h); } catch (e) {}
_bcApplySize(sz.w, sz.h);
}
try { ctrl.viz.render(); } catch (e) {}
},
resize(w, h) { if (ctrl.viz && ctrl.viz.setRendererSize) { try { ctrl.viz.setRendererSize(w, h); } catch (e) {} ctrl.lastW = w; ctrl.lastH = h; } },
resize(w, h) { _bcApplySize(w, h); },
destroy() {
ctrl.dead = true;
_bcControllers.delete(ctrl);
+81 -6096
View File
File diff suppressed because it is too large Load Diff
+153
View File
@@ -0,0 +1,153 @@
"""The router seam (`appstate.py`).
The load-bearing assertion here is `test_server_wires_the_seam`: that `server`
actually calls `appstate.configure(...)`. Every other test in this file would
pass just fine against a seam nothing ever wires up the same class of silent
no-op that bit the frontend refactor twice when a scripted `setHostHooks` edit
stopped matching its anchor. Unit tests cannot see wiring unless you make them
look at it.
"""
import importlib
import sys
import pytest
import appstate
def _close_server_dbs(mod):
conn = getattr(getattr(mod, "meta_db", None), "conn", None)
if conn is not None:
getattr(mod, "_join_background_db_threads", lambda: None)()
conn.close()
ae_conn = getattr(getattr(mod, "audio_effect_mappings", None), "conn", None)
if ae_conn is not None:
ae_conn.close()
@pytest.fixture()
def isolated_server(tmp_path, monkeypatch):
"""A freshly imported `server` bound to a throwaway CONFIG_DIR.
Importing `server` constructs `MetadataDB` + `AudioEffectsMappingDB` at
module level, so it MUST be re-imported under a patched CONFIG_DIR an
unguarded `import server` would create/mutate the developer's real
`~/.local/share/feedback` databases. Same idiom as the other ~49
server-importing suites.
Teardown restores the appstate slots as well as closing the connections:
leaving `appstate.meta_db` published but pointing at a closed sqlite handle
would hand a later test (or router) a live-looking, dead singleton.
"""
previous = (appstate.meta_db, appstate.audio_effect_mappings)
monkeypatch.setenv("CONFIG_DIR", str(tmp_path))
sys.modules.pop("server", None)
mod = importlib.import_module("server")
yield mod
_close_server_dbs(mod)
# Leave no half-torn-down `server` behind: the next fixture re-imports it.
sys.modules.pop("server", None)
appstate.configure(meta_db=previous[0], audio_effect_mappings=previous[1])
def test_import_is_side_effect_free():
"""`import appstate` must construct nothing and touch no disk.
This is why the ~49 fixtures that `sys.modules.pop("server")` and re-import
(to rebuild `meta_db` under a patched CONFIG_DIR) keep working untouched:
server owns construction, appstate only mirrors it. A singleton *owned*
here would survive that pop and go stale.
"""
sys.modules.pop("appstate", None)
fresh = importlib.import_module("appstate")
try:
assert fresh.meta_db is None
assert fresh.audio_effect_mappings is None
finally:
sys.modules["appstate"] = appstate
def test_configure_publishes_known_slots():
sentinel = object()
original = appstate.meta_db
try:
appstate.configure(meta_db=sentinel)
assert appstate.meta_db is sentinel
finally:
appstate.configure(meta_db=original)
def test_configure_is_idempotent():
"""server re-imports call configure() again; the last write must win."""
original = appstate.meta_db
try:
appstate.configure(meta_db="first")
appstate.configure(meta_db="second")
assert appstate.meta_db == "second"
finally:
appstate.configure(meta_db=original)
def test_configure_rejects_an_unknown_slot():
"""A typo'd or stale keyword must raise, not silently create a global that
nothing reads. A seam whose wiring can no-op undetected is worse than none."""
with pytest.raises(TypeError, match="unknown slot"):
appstate.configure(met_db="typo")
assert not hasattr(appstate, "met_db")
def test_late_bound_read_sees_a_later_configure():
"""Routers must read `appstate.meta_db`, never `from appstate import meta_db`.
This pins the property that makes that rule work."""
def router_style_read():
return appstate.meta_db # module attribute, resolved at call time
original = appstate.meta_db
try:
appstate.configure(meta_db="before")
assert router_style_read() == "before"
appstate.configure(meta_db="after")
assert router_style_read() == "after"
finally:
appstate.configure(meta_db=original)
def test_server_wires_the_seam(isolated_server):
"""The one that catches a dropped `appstate.configure(...)` call.
Identity, not truthiness, so a stray re-assignment or a half-applied edit
fails here rather than in some router months later.
"""
assert appstate.meta_db is isolated_server.meta_db
assert appstate.audio_effect_mappings is isolated_server.audio_effect_mappings
assert appstate.meta_db is not None
def test_reimporting_server_republishes_the_fresh_singletons(
isolated_server, tmp_path, monkeypatch
):
"""The 49-fixture contract, exercised end to end.
Those fixtures `sys.modules.pop("server")` + re-import to rebuild `meta_db`
under a new CONFIG_DIR, and know nothing about appstate. So the seam must
re-publish on that second import. This is the test that would fail if
`appstate` ever *owned* the singletons: a module-level `meta_db` there
survives the pop and the assertions below would still see the FIRST DB.
"""
first_db = isolated_server.meta_db
assert appstate.meta_db is first_db
assert str(tmp_path) in first_db.db_path
second_config = tmp_path / "second"
monkeypatch.setenv("CONFIG_DIR", str(second_config))
sys.modules.pop("server", None)
second_server = importlib.import_module("server")
try:
assert second_server.meta_db is not first_db # genuinely rebuilt
assert str(second_config) in second_server.meta_db.db_path
assert appstate.meta_db is second_server.meta_db # ...and re-published
assert appstate.audio_effect_mappings is second_server.audio_effect_mappings
finally:
_close_server_dbs(second_server)
sys.modules.pop("server", None)
+4 -1
View File
@@ -121,7 +121,10 @@ def _converter_ebeats(converter, numerator, denominator, tempo_changes=None):
def _assert_ebeats(converter, numerator, denominator, expected_times, tempo_changes=None):
ebeats = _converter_ebeats(converter, numerator, denominator, tempo_changes)
assert [ebeat.get("time") for ebeat in ebeats] == expected_times
# Compare by value, not string: beat times are written at 6-decimal
# (microsecond) precision so the derived per-bar tempo matches the authored
# GP value, but these tests only care about the spacing, not the format.
assert [float(ebeat.get("time")) for ebeat in ebeats] == [float(t) for t in expected_times]
assert [ebeat.get("measure") for ebeat in ebeats] == [
"1",
*["-1"] * (len(expected_times) - 1),
+8 -3
View File
@@ -16,6 +16,8 @@ import pytest
import yaml
from fastapi.testclient import TestClient
from routers.ws_highway import _sanitize_authors
# ── _sanitize_authors unit tests ────────────────────────────────────────────
@@ -35,7 +37,7 @@ def server_mod(monkeypatch, tmp_path):
def test_sanitize_authors_valid(server_mod):
out = server_mod._sanitize_authors(
out = _sanitize_authors(
{
"authors": [
{"name": "Azure", "role": "charter", "email": "a@b.c", "url": "x"},
@@ -53,7 +55,7 @@ def test_sanitize_authors_valid(server_mod):
def test_sanitize_authors_skips_malformed(server_mod):
out = server_mod._sanitize_authors(
out = _sanitize_authors(
{
"authors": [
{"name": ""}, # blank name → skipped
@@ -69,7 +71,7 @@ def test_sanitize_authors_skips_malformed(server_mod):
@pytest.mark.parametrize("manifest", [None, {}, {"authors": None}, {"authors": "x"}, "nope"])
def test_sanitize_authors_absent_or_nonlist(server_mod, manifest):
assert server_mod._sanitize_authors(manifest) == []
assert _sanitize_authors(manifest) == []
# ── song_info WS integration ────────────────────────────────────────────────
@@ -118,6 +120,9 @@ def make_client(tmp_path, monkeypatch):
monkeypatch.setattr(server, "load_plugins", lambda *a, **kw: None)
monkeypatch.setattr(server, "startup_scan", lambda: None)
monkeypatch.setattr(server, "SLOPPAK_CACHE_DIR", tmp_path / "cache")
# ws_highway reads the cache dir through the appstate seam now.
import appstate as _appstate
monkeypatch.setattr(_appstate, "sloppak_cache_dir", tmp_path / "cache")
return server
(tmp_path / "dlc").mkdir()
@@ -96,6 +96,9 @@ def make_client(tmp_path, monkeypatch):
monkeypatch.setattr(server, "load_plugins", lambda *a, **kw: None)
monkeypatch.setattr(server, "startup_scan", lambda: None)
monkeypatch.setattr(server, "SLOPPAK_CACHE_DIR", tmp_path / "cache")
# ws_highway reads the cache dir through the appstate seam now.
import appstate as _appstate
monkeypatch.setattr(_appstate, "sloppak_cache_dir", tmp_path / "cache")
return server
(tmp_path / "dlc").mkdir()
+3
View File
@@ -123,6 +123,9 @@ def make_client(tmp_path, monkeypatch):
monkeypatch.setattr(server, "load_plugins", lambda *a, **kw: None)
monkeypatch.setattr(server, "startup_scan", lambda: None)
monkeypatch.setattr(server, "SLOPPAK_CACHE_DIR", tmp_path / "cache")
# ws_highway reads the cache dir through the appstate seam now.
import appstate as _appstate
monkeypatch.setattr(_appstate, "sloppak_cache_dir", tmp_path / "cache")
return server
(tmp_path / "dlc").mkdir()
+84
View File
@@ -0,0 +1,84 @@
"""Concurrent unnamed loop saves must get unique names.
`save_loop` auto-names an unnamed loop `Loop {count+1}`. If the `COUNT(*)` runs
outside the DB lock (as it did before the fix), two simultaneous unnamed POSTs
read the same count and both mint the same name. A `threading.Barrier` releases
all workers into `save_loop` at once to force that interleave.
Single-user app, so this race is unlikely in practice but the fix is one lock
scope, and the test pins it.
"""
import threading
import pytest
import appstate
from metadata_db import MetadataDB
from routers import loops
@pytest.fixture()
def meta_db(tmp_path):
prev = appstate.meta_db
db = MetadataDB(tmp_path)
appstate.configure(meta_db=db)
yield db
db.conn.close()
appstate.configure(meta_db=prev)
def test_concurrent_unnamed_saves_get_unique_names(meta_db):
workers = 16
barrier = threading.Barrier(workers)
names, errors = [], []
lock = threading.Lock()
def save():
try:
barrier.wait() # release all at once
r = loops.save_loop({"filename": "song.feedpak", "start": 0.0, "end": 1.0})
with lock:
names.append(r["name"])
except Exception as e: # noqa: BLE001 — surface any thread error
with lock:
errors.append(e)
threads = [threading.Thread(target=save) for _ in range(workers)]
for t in threads:
t.start()
for t in threads:
t.join()
assert not errors, errors
assert len(names) == workers
# The load-bearing assertion: no two auto-named loops collide.
assert len(set(names)) == workers, f"duplicate loop names: {sorted(names)}"
# And the DB agrees — every insert landed.
stored = meta_db.conn.execute(
"SELECT COUNT(*) FROM loops WHERE filename = ?", ("song.feedpak",)
).fetchone()[0]
assert stored == workers
def test_list_and_save_do_not_error_under_interleave(meta_db):
"""A read overlapping writes must not raise (shared connection, one lock)."""
stop = threading.Event()
errors = []
def reader():
while not stop.is_set():
try:
loops.list_loops("song.feedpak")
except Exception as e: # noqa: BLE001
errors.append(e)
r = threading.Thread(target=reader)
r.start()
try:
for i in range(40):
loops.save_loop({"filename": "song.feedpak", "name": f"n{i}", "start": 0.0, "end": 1.0})
finally:
stop.set()
r.join()
assert not errors, errors
+112
View File
@@ -0,0 +1,112 @@
"""Guard: every first-party module `server.py` imports must be one the packagers copy.
feedback-desktop's `scripts/bundle-slopsmith.sh` copies a **hardcoded list** from
core into the app bundle `server.py`, `VERSION`, `lib/`, `data/`, `static/`,
`plugins/__init__.py`. A new root-level module (say `appstate.py`) ships fine in
Docker, imports fine under pytest, and is then *silently dropped* from the
packaged desktop app, which dies at startup with:
File ".../Resources/slopsmith/server.py", line 71, in <module>
import appstate
ModuleNotFoundError: No module named 'appstate'
That shipped once. This test is why it can't ship twice: it walks `server.py`'s
module-level imports, keeps the ones that resolve inside this repo, and asserts
each lives under a directory every packaging path already copies wholesale.
If you add a first-party module for `server.py`, put it in `lib/` the one core
directory the Dockerfile (`COPY lib/`), `docker-compose.yml`, and the desktop
bundler (`cp -r lib`) all copy, and that all three put on `sys.path`. If you
genuinely need it at the repo root, you must also teach `bundle-slopsmith.sh`,
the `Dockerfile`, `.dockerignore`, and `docker-compose.yml` about it and then
update `BUNDLED_ROOTS` below.
"""
import ast
import importlib.util
import pathlib
import pytest
REPO_ROOT = pathlib.Path(__file__).resolve().parent.parent
# Directories every packaging path copies wholesale, plus the files copied by name.
BUNDLED_ROOTS = ("lib", "plugins", "data", "static")
BUNDLED_FILES = ("server.py", "main.py")
def _server_toplevel_imports():
"""Module names imported at `server.py`'s top level (not inside a function)."""
tree = ast.parse((REPO_ROOT / "server.py").read_text())
names = set()
for node in tree.body: # top level only — lazy imports are fine
if isinstance(node, ast.Import):
names.update(a.name.split(".")[0] for a in node.names)
elif isinstance(node, ast.ImportFrom) and node.level == 0 and node.module:
names.add(node.module.split(".")[0])
return sorted(names)
# `spec.origin` is not always a path: built-in and frozen stdlib modules use
# these sentinels. `Path("frozen").resolve()` would land inside the repo and
# report `os` as first-party — so filter them before touching the filesystem.
_NON_PATH_ORIGINS = {"built-in", "frozen", "namespace"}
def _first_party_origin(name):
"""Path of `name` if it resolves inside this repo, else None (stdlib/site-package)."""
try:
spec = importlib.util.find_spec(name)
except (ImportError, ValueError):
return None
if spec is None:
return None
if spec.origin and spec.origin not in _NON_PATH_ORIGINS:
origin = pathlib.Path(spec.origin)
else:
# Namespace/frozen: fall back to the first search location, if any.
locations = list(getattr(spec, "submodule_search_locations", None) or [])
if not locations:
return None
origin = pathlib.Path(locations[0])
if not origin.is_absolute():
return None # a sentinel, not a real path
origin = origin.resolve()
try:
origin.relative_to(REPO_ROOT)
except ValueError:
return None # outside the repo → a dependency
return origin
@pytest.mark.parametrize("name", _server_toplevel_imports())
def test_server_import_is_bundled(name):
origin = _first_party_origin(name)
if origin is None:
return # stdlib or an installed dependency
rel = origin.relative_to(REPO_ROOT)
if rel.as_posix() in BUNDLED_FILES or rel.parts[0] in BUNDLED_ROOTS:
return
raise AssertionError(
f"server.py imports `{name}` from {rel}, which no packager copies.\n"
f"The desktop bundler (scripts/bundle-slopsmith.sh) copies only "
f"{BUNDLED_FILES} and {BUNDLED_ROOTS}/, so the packaged app would die "
f"at startup with ModuleNotFoundError: No module named '{name}'.\n"
f"Move it under lib/, or teach bundle-slopsmith.sh + Dockerfile + "
f".dockerignore + docker-compose.yml about it and update BUNDLED_ROOTS."
)
def test_the_seam_and_routers_live_under_lib():
"""Pin the two that already caused a shipped break."""
for name in ("appstate", "routers"):
origin = _first_party_origin(name)
assert origin is not None, f"{name} does not resolve inside the repo"
assert origin.relative_to(REPO_ROOT).parts[0] == "lib", (
f"{name} resolved to {origin.relative_to(REPO_ROOT)}; it must live "
f"under lib/ or the packaged desktop app will not ship it"
)
+107
View File
@@ -0,0 +1,107 @@
"""Playlist cover upload: client vs server errors, no temp litter, no leak.
The pre-split handler caught decode AND persistence failures in one `except`,
returned 400 for both, and echoed the exception (`Invalid image: {e}`) so a
disk/permission failure was mislabeled as a client error and could leak a
filesystem path. These pin the split.
"""
import base64
import io
import pytest
from fastapi.testclient import TestClient
from PIL import Image
import appstate
from metadata_db import MetadataDB
from routers import playlists
@pytest.fixture()
def client(tmp_path):
prev = (appstate.meta_db, appstate.config_dir)
db = MetadataDB(tmp_path)
appstate.configure(meta_db=db, config_dir=tmp_path)
app_ = __import__("fastapi").FastAPI()
app_.include_router(playlists.router)
try:
yield TestClient(app_), tmp_path
finally:
db.conn.close()
appstate.configure(meta_db=prev[0], config_dir=prev[1])
def _png_b64():
buf = io.BytesIO()
Image.new("RGB", (8, 8), (10, 20, 30)).save(buf, "PNG")
return base64.b64encode(buf.getvalue()).decode()
def _make_playlist(client):
return client.post("/api/playlists", json={"name": "P"}).json()["id"]
def test_valid_cover_saves_and_leaves_no_temp(client):
c, tmp_path = client
pid = _make_playlist(c)
r = c.post(f"/api/playlists/{pid}/cover", json={"image": _png_b64()})
assert r.status_code == 200
cover_dir = tmp_path / "playlist_covers"
assert (cover_dir / f"{pid}.png").exists()
# The atomic-publish temp file must not linger.
assert not list(cover_dir.glob("*.tmp"))
def test_undecodable_image_is_a_400_without_leaking(client):
c, _ = client
pid = _make_playlist(c)
# valid base64, not a valid image
r = c.post(f"/api/playlists/{pid}/cover", json={"image": base64.b64encode(b"not an image").decode()})
assert r.status_code == 400
body = r.json()["error"]
assert body == "Invalid image" # generic — no exception detail echoed
assert "playlist_covers" not in body # no filesystem path leak
def test_save_failure_is_a_500_not_a_400(client, monkeypatch):
"""A persistence failure (here: Image.save raising) must be a logged 500,
not a 400 the whole point of the decode/persist split. Negative-checks
against the pre-fix behavior, which returned 400 for exactly this."""
c, tmp_path = client
pid = _make_playlist(c)
payload = _png_b64() # build BEFORE patching save
def boom(self, fp, *a, **k):
raise OSError("disk full")
monkeypatch.setattr(Image.Image, "save", boom)
r = c.post(f"/api/playlists/{pid}/cover", json={"image": payload})
assert r.status_code == 500
assert "disk full" not in r.json()["error"] # no internal detail
assert not list((tmp_path / "playlist_covers").glob("*.tmp")) # temp cleaned up
def test_temp_creation_failure_is_a_500(client, monkeypatch):
"""mkstemp raising (unwritable dir / full disk) must hit the same logged 500
path as a save failure, not escape as an unhandled server error."""
import tempfile as _tempfile
c, _ = client
pid = _make_playlist(c)
payload = _png_b64()
def boom(*a, **k):
raise OSError("read-only file system")
monkeypatch.setattr(_tempfile, "mkstemp", boom)
r = c.post(f"/api/playlists/{pid}/cover", json={"image": payload})
assert r.status_code == 500
assert "read-only" not in r.json()["error"]
def test_upload_to_missing_playlist_is_404(client):
c, _ = client
r = c.post("/api/playlists/9999/cover", json={"image": _png_b64()})
assert r.status_code == 404
+6 -2
View File
@@ -6,6 +6,10 @@ import sys
import pytest
from fastapi.testclient import TestClient
# Moved to routers/playlists in R3; reads appstate.config_dir, which the
# `server` fixture configures via CONFIG_DIR before this is called.
from routers.playlists import _playlist_cover_path
@pytest.fixture()
def server(tmp_path, monkeypatch, isolate_logging):
@@ -168,6 +172,6 @@ def test_cover_rejects_non_string_image_with_400_not_500(client):
def test_deleting_playlist_removes_custom_cover(client, server):
pid = client.post("/api/playlists", json={"name": "Doomed"}).json()["id"]
client.post(f"/api/playlists/{pid}/cover", json={"image": _png_b64()})
assert server._playlist_cover_path(pid).exists()
assert _playlist_cover_path(pid).exists()
client.delete(f"/api/playlists/{pid}")
assert not server._playlist_cover_path(pid).exists()
assert not _playlist_cover_path(pid).exists()
+1 -1
View File
@@ -50,7 +50,7 @@ def client(tmp_path, monkeypatch):
# Point CONFIG_DIR at a per-test temp path BEFORE server's
# import-time side effects run. server.py reads CONFIG_DIR from the
# environment at module load (line 35) and immediately constructs
# `meta_db = MetadataDB()` at module level, which calls
# `meta_db = MetadataDB(CONFIG_DIR)` at module level, which calls
# CONFIG_DIR.mkdir(...) and opens a sqlite file — a plain
# post-import monkeypatch on server.CONFIG_DIR wouldn't catch those
# side effects, and the real user config dir would get written to.
+9 -4
View File
@@ -22,6 +22,11 @@ from pathlib import Path
import pytest
from fastapi.testclient import TestClient
# The startup DB swap lives with the DB layer it guards, not with server.py.
# metadata_db reads no environment at import, so a plain module import is safe
# alongside the env-patched `server_mod` fixture below.
from metadata_db import _apply_pending_db_restore
@pytest.fixture()
def server_mod(tmp_path, monkeypatch):
@@ -219,7 +224,7 @@ def test_apply_pending_db_restore_swaps_and_clears_sidecars(server_mod, tmp_path
(tmp_path / "web_library.db-shm").write_bytes(b"OLD-SHM")
(tmp_path / "web_library.db.restore").write_bytes(new_db)
server_mod._apply_pending_db_restore(tmp_path)
_apply_pending_db_restore(tmp_path)
assert main.read_bytes() == new_db # swapped in
assert not (tmp_path / "web_library.db.restore").exists()
@@ -234,7 +239,7 @@ def test_apply_pending_db_restore_discards_corrupt_keeps_live(server_mod, tmp_pa
main.write_bytes(b"LIVE-GOOD-DB")
(tmp_path / "web_library.db.restore").write_bytes(b"SQLite format 3\x00" + b"\xff" * 64)
server_mod._apply_pending_db_restore(tmp_path)
_apply_pending_db_restore(tmp_path)
assert main.read_bytes() == b"LIVE-GOOD-DB" # live DB preserved
assert not (tmp_path / "web_library.db.restore").exists() # bad restore dropped
@@ -242,7 +247,7 @@ def test_apply_pending_db_restore_discards_corrupt_keeps_live(server_mod, tmp_pa
def test_apply_pending_db_restore_noop_without_staging(server_mod, tmp_path):
(tmp_path / "web_library.db").write_bytes(b"LIVE")
server_mod._apply_pending_db_restore(tmp_path) # nothing staged
_apply_pending_db_restore(tmp_path) # nothing staged
assert (tmp_path / "web_library.db").read_bytes() == b"LIVE"
@@ -266,7 +271,7 @@ def test_full_db_backup_restore_round_trip(client, server_mod, tmp_path):
# Simulate a restart: close the live conn, apply the staged restore,
# reopen — the song is back.
server_mod.meta_db.conn.close()
server_mod._apply_pending_db_restore(tmp_path)
_apply_pending_db_restore(tmp_path)
conn = sqlite3.connect(str(tmp_path / "web_library.db"))
try:
rows = conn.execute(
+27
View File
@@ -249,3 +249,30 @@ def test_flat_string_count_patch_resets_incompatible_named_tuning():
patched = apply_flat_instrument_patch_to_profiles(settings, {"string_count": 7})
assert patched["string_count"] == 7
assert patched["tuning"] == "Standard"
# ── freqs_to_midis (the /api/tunings tuningMidis inverse) ────────────────────
def test_freqs_to_midis_round_trips_every_builtin_at_440():
from tunings import freqs_to_midis
for key, presets in TUNING_PRESET_MIDIS.items():
for name, midis in presets.items():
assert freqs_to_midis(open_midis_to_freqs(midis)) == midis, f"{key}/{name}"
def test_freqs_to_midis_round_trips_at_nonstandard_reference():
# The consumer footgun this exists to kill: frequencies served at a 432/450
# reference must recover the SAME integer midis when inverted at that
# reference (client-side log2-at-440 reconstruction drifts here).
from tunings import freqs_to_midis
for ref in (430.0, 432.0, 444.0, 450.0):
for midis in (TUNING_PRESET_MIDIS["guitar-8"]["Standard"], TUNING_PRESET_MIDIS["bass-5"]["Standard"]):
freqs = open_midis_to_freqs(midis, ref)
assert freqs_to_midis(freqs, ref) == midis, f"ref={ref}"
def test_freqs_to_midis_rejects_garbage():
from tunings import freqs_to_midis
assert freqs_to_midis([82.41, 0]) is None # non-positive
assert freqs_to_midis([82.41, "x"]) is None # non-numeric
assert freqs_to_midis([]) == [] # vacuously fine