mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-21 20:31:21 +00:00
d217ffecd8
65 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
byrongamatos
|
d217ffecd8 |
refactor(server): extract XP + per-song stats into routers/stats.py (R3)
The last of the progression cluster. XP award (1 route) + per-song practice stats
(record/recent/best/top/per-song, 5 routes) — meta_db-only apart from the two
seam accessors record_stats uses (get_progression_content, builtin_diagnostic_
filename, both landed by shop/progression). Bodies verbatim; @app -> @router,
meta_db -> appstate.meta_db, _as_int from metadata_db, _clean_str from reqfields.
Three scattered source blocks (xp, the stats block, and the separated
/api/stats/{filename:path} which the /api/library/practice-suggestions route
splits off) are assembled into one module and mounted once. Registration order
is preserved WHERE IT MATTERS: the /api/stats/{filename:path} catch-all is
assembled LAST inside the router, so it still can't shadow the fixed /recent
/best /top paths — verified against the live route table (recent/best/top all
precede the catch-all) and the route SET is identical to origin/main (143).
server.py: 7,478 -> 7,275.
Verified: pyflakes clean; route set identical + catch-all-last; pytest 2401
passed (113 across song_stats/profile/progression); eslint 0. Boot smoke:
/stats/recent /best /top all 200 (not shadowed), xp/award 200.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
4cc8fa3b4d
|
refactor(server): extract the profile routes into routers/profile.py (R3) (#854)
6 routes (get/set profile, bundled+custom avatars, avatar upload/serve, progress) + the exclusive _list_bundled_avatars helper. Bodies verbatim; @app -> @router, meta_db -> appstate.meta_db, CONFIG_DIR/STATIC_DIR -> appstate.config_dir/ static_dir (seam), _clean_str from reqfields, _get_progression_content() -> appstate.get_progression_content(). No STATIC_DIR test retarget: _list_bundled_avatars reads appstate.static_dir but test_profile_api doesn't patch STATIC (only the sloppak/audio/traversal suites do, for handlers that stay in server.py). server.py: 7,594 -> 7,478. Verified: pyflakes clean; route table IDENTICAL (143); pytest 2401 passed (61 in test_profile_api); eslint 0. Boot smoke: GET /api/profile 200 (drives get_progression_content), /avatars lists via appstate.static_dir, /progress 200. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
f9f33320ac
|
refactor(server): extract the progression routes into routers/progression.py (R3) (#853)
4 routes (overview/add-paths/onboarding/events, spec 010) + their EXCLUSIVE helpers (_goal_ui_progress, _progression_overview 112L) + the _PROGRESSION_EVENT_TYPES whitelist. Bodies verbatim; @app -> @router, meta_db -> appstate.meta_db, _clean_str from reqfields. The two SHARED server accessors read through the seam: get_progression_content (added for shop #851) and builtin_diagnostic_filename (new slot — a trivial const-returning fn shared with the stats router's api_record_stats). Both are injected via the second appstate.configure() after their defs (the import-top configure runs before them). The cache + fns stay in server.py, so test_progression_api's server._progression_content patch is untouched — 0 retarget. server.py: 7,798 -> 7,594. Verified: pyflakes clean; route table IDENTICAL (143); both seam accessors wired; pytest 2401 passed (63 in test_progression_api); eslint 0. Boot smoke: GET /api/progression 200 (drives _progression_overview + both accessors), events 400 on bad body. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
5f58af4faa
|
refactor(server): extract the shop routes + inject get_progression_content into the seam (R3) (#851)
The progression-content substrate: `_get_progression_content` (a lazy, double-checked-locking content cache) is now published into the appstate seam as a CALLABLE. The cache global + lock + the function stay in server.py (startup uses it, and test_progression_api patches `server._progression_content` directly), so ZERO test retargeting — routers just call `appstate.get_progression_content()`. Because the accessor is defined at server.py:1152 but the import-top configure() runs at :346, a second `appstate.configure(get_progression_content=...)` publishes it right after the def (configure is idempotent/additive). First consumer: routers/shop.py (3 routes: buy/equip/list). Bodies verbatim; @app -> @router, meta_db -> appstate.meta_db, _clean_str from reqfields, _get_progression_content() -> appstate.get_progression_content(). This unblocks stats/progression/profile next (all share the accessor). server.py: 7,880 -> 7,845. Verified: pyflakes clean; route table IDENTICAL (143); pytest 2401 passed (test_progression_api's server._progression_content patch still works via the kept cache); packaging guard; eslint 0. Boot smoke: GET /api/shop 200 (drives appstate.get_progression_content), buy 400 on bad body. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
ea8834862d
|
refactor(server): batch the small meta_db user-state endpoints into routers/library_extras.py (R3) (#850)
work keeper-chart prefs (3) + favorites toggle + tags list + saved toggle + session/continue — 7 routes across 5 domains, all meta_db-only (0 setattr, 0 helpers). Bodies verbatim; @app -> @router, meta_db -> appstate.meta_db, _clean_str from reqfields. These were scattered singletons with no natural neighbor, so they're grouped as "small library/user-state endpoints" and mounted once. All paths are distinct and non-overlapping, so registering them together doesn't change routing: verified the route SET is identical to origin/main (143) AND that no moved path shadows or is shadowed by another (order-independence check). server.py: 7,880 -> 7,833. Verified: pyflakes clean; route set identical + order-independent; pytest 2401 passed; packaging guard 45; eslint 0. Boot smoke: tags/session GET 200, favorites/saved toggle (400 on missing filename), work/charts 200. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
cbc65458e3
|
refactor(server): extract the wishlist routes into routers/wanted.py (R3) (#847)
Free after _clean_str moved to lib (#841): wanted's deps are JSONResponse, _clean_str (reqfields), app + meta_db (seam). 3 routes (list/add/remove), 0 setattr targets, 0 helpers to relocate. Bodies verbatim; @app -> @router, meta_db -> appstate.meta_db. include_router at the original site; 143-route table identical. No test retargeting. server.py: 8,003 -> 7,974 (this branch is independent of the chart PR). Verified: pyflakes clean; route table identical; pytest 2401 passed (52 in test_wanted_api); eslint 0. Boot smoke: add wishlist entry -> list -> delete, 400 on missing artist+title (_clean_str path). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
7c87538d6b
|
refactor(server): extract the chart routes into routers/chart.py (R3) (#846)
Unblocked by the DLC-path substrate (#843): chart's only server-module deps are now app + meta_db (both seam); _get_dlc_dir/_resolve_dlc_path come from dlc_paths, sloppak/loose detection from the shared lib modules. 4 routes (split/unsplit/ work/fileinfo), meta_db-only otherwise. 0 setattr targets, 0 helpers to relocate. Bodies verbatim; @app -> @router, meta_db -> appstate.meta_db. include_router at the original site; 143-route table identical to origin/main. No test retargeting. server.py: 8,003 -> 7,909. Verified: pyflakes clean on the router; no new undefined/dead in server.py; route table identical; pytest 2401 passed (74 across work_charts/context_menu/ group_filter/packaging); eslint 0. Boot smoke: chart/work 200, chart/fileinfo resolves the real pack path through _resolve_dlc_path. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
c8701991cb
|
fix(ws_highway): swallow a mid-stream WebSocketDisconnect quietly (#845)
* fix(ws_highway): swallow a mid-stream WebSocketDisconnect quietly Pre-existing (byte-identical to origin/main), flagged by CodeRabbit on #844: the outer try's only guard was `except Exception as e: log.exception("highway_ws unhandled error")`. The inner `except WebSocketDisconnect` covers ONLY the post-`ready` keep-alive loop, and the drum_tab/notation sends have their own localized guards — but the ~13 other streamed sends (beats, sections, notes, chords, anchors, …) fall through to the blanket handler. Since WebSocketDisconnect is an Exception subclass, a routine tab-close mid-load was logged as an error at whichever send was in flight. Fix: a dedicated `except WebSocketDisconnect: return` before the blanket handler, matching the two localized guards and the lib/ coding guideline. tests/test_ws_highway_disconnect.py drives highway_ws with a fake websocket that raises WebSocketDisconnect on the first streamed send (`loading`), over a real minimal sloppak. Negative-checked: removing the guard makes it fail (the disconnect is logged as `highway_ws unhandled error`); the fix passes. Uses a raw handler on `feedBack.server` rather than caplog, since configure_logging() reroutes that logger through structlog where caplog doesn't observe it. Full suite green. Closes the review thread on #844. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ws_highway): assert streaming stops after disconnect (exactly one send) — CodeRabbit The stub now raises only on the first send and the test asserts ws.sends == 1, so a handler that caught the disconnect and kept streaming would fail. Still negative-checked: removing the guard fails the test. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
514461167e
|
refactor(server): extract the highway WebSocket into routers/ws_highway.py (R3) (#844)
The single largest handler in server.py — the 902-line /ws/highway/{filename}
chart streamer — plus its 3 exclusive helpers (_pick_smart_arrangement,
_sanitize_authors, _sanitized_song_offset). server.py: 9,008 -> 8,003 (-1,005,
the biggest single R3 cut).
Clean move despite the size: the handler's only server-module deps are the 4
path constants + 3 exclusive helpers + app + log. Everything else it uses is
either NESTED inside the handler (_evict_audio_cache, _fill_scale_degree,
_manifest_entries, _tone_names, _xml_rank, _send_keepalives) or imported from
the shared lib modules (song/audio/sloppak/drums/notation/dlc_paths/metadata_db).
- Path constants read through the appstate seam: added static_dir /
sloppak_cache_dir / audio_cache_dir slots (config_dir already there);
server.py configures them. Bodies otherwise verbatim (@app.websocket ->
@router.websocket, PATHS -> appstate.*, log -> module logger).
- sloppak_cache_dir IS setattr-patched, so the 3 test_highway_ws_* suites now
also `setattr(appstate, "sloppak_cache_dir", ...)` next to their existing
server patch. _sanitize_authors unit tests import it from routers.ws_highway
(it moved). No other test churn.
- Removed 23 now-dead imports from server.py (song/audio/drums/notation/
bisect/contextvars/structlog/WebSocket*/_arr_smart_sort_key) — diffed against
the origin/main unused-import baseline so only NEWLY-dead ones went.
owns_tmp (assigned, never read) moved verbatim — it's pre-existing dead on
origin/main too; left as-is to keep the move faithful.
Verified: route table identical to origin/main (143, paths/methods/order);
handler body verbatim spot-checked; pyflakes clean (server has no new
undefined/dead); pytest 2400 passed; packaging guard 53; eslint 0. Boot smoke:
the highway WS streams the full chart (song_info/beats/sections/notes/chords/
notation/anchors/drum_tab -> ready) BYTE-for-byte the same message sequence as
origin/main across arrangements 0/1/2, zero tracebacks.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
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> |
||
|
Byron Gamatos
|
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> |
||
|
Byron Gamatos
|
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> |
||
|
Byron Gamatos
|
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> |
||
|
Byron Gamatos
|
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> |
||
|
Byron Gamatos
|
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> |
||
|
Byron Gamatos
|
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>
|
||
|
Byron Gamatos
|
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> |
||
|
Byron Gamatos
|
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> |
||
|
ChrisBeWithYou
|
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> |
||
|
ChrisBeWithYou
|
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> |
||
|
ChrisBeWithYou
|
5cb4ea0623
|
feat(drums): capture velocities alongside times in unmapped-percussion reporting (#808)
* feat(drums): capture velocities alongside times in unmapped-percussion reporting Both drum converters opt-in out_unmapped capture (convert_drum_track_from_midi, convert_drum_track_to_drumtab) gain an index-aligned `velocities` list next to `times`, carrying each dropped note real dynamics — MIDI velocity verbatim; GP velocity with the same 1-127 gate as mapped hits, falling back to the 100 import default. A hand-mapping UI (the editor unmapped-notes dialog) can then restore mapped notes at their source dynamics instead of flattening to v:100 (editor-side consumer: feedBack-plugin-editor#111). The GP path chronological sort now reorders times and velocities in LOCKSTEP so multi-voice measures cannot silently reassign dynamics. Additive: callers that ignore the new key are unaffected. Tests: extended tests/test_midi_import_drums.py + tests/test_gp2rs_drums.py (alignment, lockstep sort, out-of-range fallback) — 26 passing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu * docs(gp2rs): clarify velocity-default comment, mark dead-path fallback - The mapped-GP velocity comment conflated GP's authoring default (95, Velocities.default) with the drumtab render default (100, DEFAULT_VELOCITY in lib/drums.py) used when `v` is omitted. Clarify both defaults and that only the latter applies to omitted hits. - Mark the `else: times.sort()` fallback in the unmapped-percussion time/velocity sort as belt-and-suspenders — times and velocities are always appended together under the same len<100 guard, so lengths can't actually diverge. No behavior change; comment-only maintainability nits from PR review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: ChrisBeWithYou <chris@rifflarr.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com> |
||
|
Byron Gamatos
|
115c3529e9
|
fix(midi): guard non-positive division in the legacy inline tempo path (#805)
Some checks are pending
ship-ci / ci (push) Waiting to run
convert_midi_track_to_keys_wire builds its own inline tempo map and divides by the raw midi.ticks_per_beat at two sites (the tempo-table precompute and the tick_to_seconds closure). A malformed header with division == 0 raised ZeroDivisionError, and an SMPTE division (which mido returns as a NEGATIVE signed short) produced negative/garbage note times. Guard the divisor with `ticks_per_beat if ticks_per_beat > 0 else 480` so both the zero and negative cases fall back to the SMF default. The `> 0` form (not `or 480`) is required because a negative value is truthy and would slip past `or`. Positive-division behavior is unchanged. Follow-up to #796, which fixed the same class of bug in the newer convert_midi_tempo_map / _build_tick_to_seconds path. Adds two focused tests: division == 0 no longer crashes and emits a non-negative time, and a negative/SMPTE division yields sane non-negative times. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
ChrisBeWithYou
|
1bccb8a9e8
|
feat(midi): convert_midi_tempo_map — extract tempos, time signatures, beat grid (#796)
* feat(midi): convert_midi_tempo_map — extract tempos, time signatures, beat grid
The keys/drums MIDI converters always built a tempo-aware tick->seconds
map internally (to bake note times) and then discarded it — and never
read time_signature meta at all — so every MIDI import landed with no
bars, no measures, and an implied 4/4 no matter what the file said.
New lib/midi_import.py helper convert_midi_tempo_map(midi_path,
track_index) extracts the grid a .mid actually carries:
- tempos: {time, bpm} per tempo event (deduped per tick, 120 default)
- time_signatures: {time, ts: [num, den]} — the song-timeline shape
- beats: one row per beat on the editor grid shape — numbered downbeats
with a den hint, measure:-1 interior beats; the beat unit follows the
active signature (6/8 = six eighth-note rows per bar)
Event scope mirrors _build_tick_to_seconds: SMF type 0/1 merge meta
from all tracks, type 2 reads ONLY the chosen track (independent
timelines — callers must never share one grid across type-2 tracks).
Mid-bar signature events (ill-formed but seen in the wild) apply at the
next bar boundary. All times compute from absolute ticks through the
cumulative tempo table and round once at emit — rounding error never
accumulates with song length. A bar-count safety valve guards malformed
files. Consumer: the editor's multitrack MIDI import (tempo-seed
dialog, feedBack-plugin-editor roadmap Phase 3).
Tests: tests/test_midi_tempo_map.py — 10 cases driving the REAL
function against real .mid files built with mido (no stubs): default
grid, tempo bends, 500-bar rounding-drift check, 4/4->3/4 and 6/8
signatures, mid-bar signature deferral, duplicate-tick last-wins,
type-2 meta isolation from a bogus sibling track, empty files, grid
coverage bounds. Full MIDI-adjacent suite green: 55 passed
(test_midi_tempo_map + test_midi_import + test_midi_import_drums +
test_gp2midi) under the project venv.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu
* fix(midi): make convert_midi_tempo_map robust — real division guard, symmetric tempo default, single-pass meta
- Push the ticks-per-beat fallback into _build_tick_to_seconds (the single
place ticks route through), guarding on `> 0` so a division==0 (malformed)
or negative SMPTE-division header no longer raises ZeroDivisionError or
walks the beat grid into negative times. Mirror the guard at the
convert_midi_tempo_map beat_ticks site. The local `or 480` was cosmetic
before — the closure still divided by the raw division.
- Seed tempos_out with a 120 BPM row at time 0 when the first set_tempo
lands after tick 0, symmetric with the (0, 4, 4) time-signature default,
so the sidecar matches the grid the head of the song actually used.
- Collapse the duplicated meta_source/note_source lists into one
source_tracks walked in a single pass (meta collection + end_tick).
- Fix a weak assert in test_mid_bar_signature_applies_at_the_next_boundary
(operator-precedence `(A and B) or C`) to assert den == 4 outright.
- Add tests: non-positive division (0 + negative SMPTE), first tempo after
start seeds 120@0, explicit SMF type-0 file, and the _TEMPO_MAP_MAX_BARS
safety valve.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: ChrisBeWithYou <chris@rifflarr.local>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
|
||
|
topkoa
|
7cbf9824b1 |
Drop dead whisperx entry from allowed lyrics_source set
The whisperx->transcribed alias runs before the membership check, so the literal whisperx never reaches _ALLOWED_LYRICS_SOURCES (same reason sng is omitted). Remove the dead entry. Per CodeRabbit review on #799. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: topkoa <topkoa@gmail.com> |
||
|
topkoa
|
33146cc7f6 |
Accept spec lyrics_source values (authored, transcribed)
The feedpak spec (§7.1) defines the lyrics_source vocabulary as {authored, transcribed, user}, but the reader only accepted the legacy {xml, notechart, whisperx, user} set and silently downgraded anything else to "xml". A spec-compliant writer (e.g. the stem_splitter plugin, which emits transcribed for WhisperX-produced lyrics) therefore lost its provenance badge.
Widen the allowed set to the union of the spec vocabulary and the legacy values so both validate, and alias the legacy whisperx engine name to the spec transcribed so existing packs normalise to the spec badge.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: topkoa <topkoa@gmail.com>
|
||
|
Byron Gamatos
|
1a8540935b
|
fix(gp_autosync): slope-constrained DTW steps — stop path collapse on riff-based songs (#791)
librosa.sequence.dtw's default step sizes permit pure horizontal/vertical moves; on songs whose chroma is self-similar for long stretches the flat cost surface let the path collapse (minutes of score onto one audio frame), so auto-sync produced monotonic-but-garbage sync points and the per-bar warp imported charts badly out of sync while reporting success. Use the standard music-sync step pattern [[1,1],[1,2],[2,1]] (local tempo ratio bounded to 0.5x-2x), falling back to unconstrained steps if the global length ratio makes it infeasible. Validated on the reported song (138 BPM tab, YouTube audio): coarse points now track 1:1, refine holds slopes 0.77-1.04, warped downbeats hit onset peaks at 3.3x background energy. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Byron Gamatos
|
92dc321fdf
|
feat(gp_autosync): piecewise time-warp helpers + refine_sync onset pass (#787)
* feat(gp_autosync): piecewise time-warp helpers + refine_sync onset pass auto_sync computes per-bar sync points but consumers only ever applied the scalar bar-1 audio_offset, so any tempo drift between the recording and the tab's authored tempo accumulated over the song. Add the librosa-free helpers needed to apply the full piecewise mapping: - bar_start_times(gp_path): per-bar score times sharing auto_sync's axis (GPIF bar-resolution map, GP3/4/5 per-tick integration) - build_warp_anchors(points, bar_starts): monotonic (score, audio) anchors - warp_time(t, anchors): piecewise-linear map with edge-slope extrapolation - warp_song_times(song, warp): retime a lib.song.Song in place (notes, sustains, chords, beats, sections, anchors, handshapes, phrase levels, tone changes, tempo overrides) - gp_has_expandable_repeats(gp_path): detects GP3/4/5 repeat/volta/direction markup whose playback expansion auto_sync's as-written points cannot map Also implement refine_sync() — the editor's refine-sync endpoint has imported it since the snapshot but it never existed in lib, so the Refine button 500'd. It densifies the DTW points to every Nth bar and re-times each with a local onset phase sweep (radius clamped under half a beat to avoid one-beat locks, short scoring grid + median residual snap against the first beats). Synthetic click-track validation: ~13ms mean / ~40ms max error from ±180ms coarse input across 117-123 BPM recordings of a 120 BPM tab. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * review: Copilot round 1 — normalize bar_start_times GP3/4/5 parse failures to ValueError, document ImportError Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
ChrisBeWithYou
|
74cff4e0d6
|
feat(enrichment): alias-aware scoring — auto-confirm non-Latin-primary artists (#772)
Some checks are pending
ship-ci / ci (push) Waiting to run
* feat(enrichment): loose MusicBrainz search fallback (find aliased artists)
The MB text search used a strict field-phrase query
(`recording:"<title>" AND artist:"<artist>"`). A field phrase only matches
MusicBrainz's *primary* artist/title — it never searches ALIASES — so a
recording stored under a non-Latin primary name (大橋純子) whose romanized
form ("Junko Ohashi") is only an alias returns ZERO results, even though MB
has it. Whole swaths of a community library (e.g. romanized J-pop / city-pop
charts) were unsearchable.
- `build_recording_query(..., loose=True)` drops the field scoping + phrases
for plain AND-ed term groups (`(telephone number) AND (junko ohashi)`),
which searches the whole document incl. aliases.
- `_mb_search_recordings` runs the strict query first (unchanged, high
precision) and only on an EMPTY result retries once with the loose query —
so mainstream matches are untouched and the extra throttled request is spent
only on a miss. Results are re-scored by rank_candidates, so recall goes up
without lowering match quality (auto-accept still needs the per-field floors).
Verified live: "Junko Ohashi / Telephone Number" and "Anri / Windy Summer"
(both 0 under the strict query) now surface the real records; "AC/DC /
Highway to Hell" still hits strict at score 1.0 with no loose retry.
Follow-up (separate): alias-aware SCORING so these can auto-confirm, not just
appear as manual candidates.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(enrichment): alias-aware scoring (auto-confirm non-Latin-primary artists)
Builds on the loose-search fallback: that surfaces a recording stored under a
Japanese primary name (大橋純子) via its romanized alias, but the SCORER still
compared the reference ("Junko Ohashi") against the primary only → artist
similarity 0 → below the auto floor, so it could only ever be a manual
candidate, never an auto-fill.
- mb_match: `cand_artist_sim` takes the best similarity over the candidate's
primary name AND its `artist_aliases`; score_candidate + classify use it.
- server: `_mb_artist_aliases(id)` fetches an artist's aliases (one throttled
lookup, process-cached — a one-artist discography costs ONE request) and
`_alias_enrich` attaches them ONLY to promising near-misses (title agrees,
primary artist doesn't) so a normal pass spends zero extra requests. Wired
into both the auto-matcher (_enrich_one) and the manual search proxy.
Verified live: "Junko Ohashi / Telephone Number" → 大橋純子 candidate goes from
score 0.5 (loose-only) to 1.0 (auto-confirmable), ranked #1; "AC/DC / Highway
to Hell" unchanged at 1.0 with no alias lookup.
Stacks on #771 (feat/mb-loose-search-fallback).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(enrichment): keep live exclusion in the loose search fallback
The loose fallback dropped the strict path's -secondarytype:Live filter, so a
studio chart whose strict query missed could fall back to — and, since
score_candidate doesn't penalize live takes, auto-confirm — a live-only
recording. Apply the same live gate to the loose query (skipped only when the
source title is itself a live take).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
|
||
|
ChrisBeWithYou
|
18c4e229e1
|
feat(enrichment): loose MusicBrainz search fallback (find aliased/romanized artists) (#771)
* feat(enrichment): loose MusicBrainz search fallback (find aliased artists)
The MB text search used a strict field-phrase query
(`recording:"<title>" AND artist:"<artist>"`). A field phrase only matches
MusicBrainz's *primary* artist/title — it never searches ALIASES — so a
recording stored under a non-Latin primary name (大橋純子) whose romanized
form ("Junko Ohashi") is only an alias returns ZERO results, even though MB
has it. Whole swaths of a community library (e.g. romanized J-pop / city-pop
charts) were unsearchable.
- `build_recording_query(..., loose=True)` drops the field scoping + phrases
for plain AND-ed term groups (`(telephone number) AND (junko ohashi)`),
which searches the whole document incl. aliases.
- `_mb_search_recordings` runs the strict query first (unchanged, high
precision) and only on an EMPTY result retries once with the loose query —
so mainstream matches are untouched and the extra throttled request is spent
only on a miss. Results are re-scored by rank_candidates, so recall goes up
without lowering match quality (auto-accept still needs the per-field floors).
Verified live: "Junko Ohashi / Telephone Number" and "Anri / Windy Summer"
(both 0 under the strict query) now surface the real records; "AC/DC /
Highway to Hell" still hits strict at score 1.0 with no loose retry.
Follow-up (separate): alias-aware SCORING so these can auto-confirm, not just
appear as manual candidates.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(enrichment): keep live exclusion in the loose search fallback
The loose fallback dropped the strict path's -secondarytype:Live filter, so a
studio chart whose strict query missed could fall back to — and, since
score_candidate doesn't penalize live takes, auto-confirm — a live-only
recording. Apply the same live gate to the loose query (skipped only when the
source title is itself a live take).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
|
||
|
ChrisBeWithYou
|
bde25c0bc8
|
fix(gpx): clamp partial final BCFS sector so GP6 .gpx import works (#749)
Every real Guitar Pro 6 (.gpx) file failed to import with "GPX BCFS sector pointer out of range (malformed file)". A real .gpx's BCFZ-declared decompressed size isn't 0x1000-aligned, so its last (small) container file lands in a partial trailing sector. _parse_bcfs raised whenever a sector read would run past the buffer end, rejecting the whole container before score.gpif could be extracted -- so no GP6 file could be charted in the song editor. (GP7/GP8 .gp files take the ZIP path, not BCFS, which is why this wasn't caught earlier.) Clamp the final sector read to the buffer end (the per-file size field trims the padding anyway), matching canonical GPX readers (alphaTab / PyGuitarPro). A sector whose start is past the end still raises, so the malformed-file guard is preserved. Verified against two real GP6 files -- both now unpack to valid GPIF with all tracks. Adds the previously-missing positive BCFS round-trip coverage: partial-final-sector, multi-file, sector-aligned baseline, and the preserved out-of-range guard. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com> |
||
|
ChrisBeWithYou
|
73c5ab149e
|
feat(enrichment): AcoustID audio-fingerprint identification (opt-in) (#759)
* feat(enrichment): AcoustID audio-fingerprint identification (opt-in)
Text search can only guess the version; the definitive fix is content-based —
fingerprint the actual audio with Chromaprint (fpcalc) and look it up on
AcoustID, which maps the fingerprint to the EXACT MusicBrainz recording (the
approach Lidarr uses). Sidesteps the studio-vs-live ambiguity entirely.
- lib/acoustid_match.py: pure response parsing + config gating (unit-tested);
normalizes AcoustID hits into the same candidate shape as mb_match so the
review UI + editor Match popup render fingerprint and text hits identically.
- server.py: _fpcalc (Chromaprint subprocess), _acoustid_lookup (throttled,
offline-guarded HTTP), _identify_by_fingerprint (also available to the
library-enrichment pipeline), and POST /api/enrichment/identify (upload the
master audio → candidates).
- Fully OPT-IN and graceful: absent the fpcalc binary or an ACOUSTID_API_KEY
the whole path is a no-op / 503 and the text matcher runs unchanged.
Requires (both optional): the `fpcalc` (Chromaprint) binary on PATH/$FPCALC,
and a free AcoustID application key in $ACOUSTID_API_KEY. Pure parsing/gating
is unit-tested; the fpcalc + live-lookup path needs those two to exercise.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(enrichment): make AcoustID self-serve — opt-in toggle + API key in settings
Fingerprinting was env-var only (ACOUSTID_API_KEY), so only an operator could
enable it. Add two core settings so a user can turn it on themselves:
- acoustid_enabled (bool, default OFF — opt-in)
- acoustid_api_key (string, ≤128 chars, trimmed; env var stays a fallback)
_acoustid_available()/_acoustid_lookup() now resolve (enabled, key) from
settings via _acoustid_settings(). /api/enrichment/identify distinguishes
"not set up" (412 needs_setup — the UI nudges the user to enable it) from
"set up but fpcalc/network missing" (503) so the client never fakes a match.
Verified: default off; POST round-trips + trims; 412 vs 503 gating; bad
types/over-length rejected. acoustid_match unit tests green (8/8).
* fix(enrichment): POST the AcoustID lookup instead of GET
A Chromaprint fingerprint is multi-KB (a 3.5-min track ≈ 3.5k chars), so
sending it as a GET query param overflows the request URL for longer songs and
fails spuriously. AcoustID accepts the same params form-encoded — POST them.
* fix(acoustid): space-separate the lookup `meta` (was silently dropping metadata)
The meta value was `+`-joined ("recordings+releasegroups+compress"). Sent over
the wire the literal `+` percent-encodes to %2B, which AcoustID does NOT split
into flags — so every hit came back with an empty `recordings` array and the
parser produced zero candidates (a fingerprint match that resolved to nothing).
AcoustID wants the flags space-separated. Verified against real fingerprints:
`+`-joined → 0 recordings; space-joined → 28, resolving Highway to Hell and
Living After Midnight to their canonical studio albums as the top hit.
* feat(acoustid): resolve the canonical original album + year from the fingerprint
AcoustID hits resolved the right recording but a weak album/blank year: the
album picker took the first studio-typed group (a later comp/soundtrack typed
"Album" could win) and the year took an arbitrary release (often a reissue).
Request the `releases` meta (which carries per-release dates) and use them to
(1) pick the EARLIEST original studio album among the groups and (2) fill the
year from that album's earliest release. Verified against real fingerprints:
Smoke on the Water → Machine Head (1972) not a later comp; Highway to Hell →
1979; Living After Midnight → British Steel (1980). +2 unit tests.
* feat(acoustid): per-song "Identify by audio" for the library metadata tooling
Add POST /api/enrichment/identify/{filename} — fingerprints an EXISTING library
song's own master audio (resolves the sloppak's original_audio or a loose
folder's audio), the library counterpart to the upload-based /identify used by
the editor. Wire an "Identify by audio" action into the match-review / Fix-match
modal: it renders fingerprint hits in the same candidate list and pins the pick
via the existing /review/{f}/pick. Shared _acoustid_gate() (412 needs_setup /
503) for both endpoints; 404 when a pack has no full mix. Both identify routes
added to the demo-mode block list (they spend fpcalc + the AcoustID budget) —
fixes a pre-existing miss on the upload route.
* fix(acoustid): regenerate stale tailwind CSS + cap identify upload
- static/tailwind.min.css was stale vs a fresh rebuild (ci/tailwind-fresh red);
regenerated with the pinned tailwindcss@3.4.19 (byte-stable).
- /api/enrichment/identify read the whole multipart upload into memory before
writing it; stream it to the temp file with a 256 MB cap (413 over) so an
oversized upload can't balloon RAM. fpcalc reads from the temp file anyway.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(acoustid): pre-parse upload guard + settings UI to enable it
- /api/enrichment/identify is now async: a pre-parse Content-Length check +
request.form(max_part_size=…) reject an oversized body BEFORE Starlette spools
the multipart to temp disk (mirrors the song-upload endpoint), and the blocking
fpcalc subprocess + AcoustID HTTP run off the event loop via run_in_executor.
- The v3 Metadata-matching settings card gains an 'Identify by audio' opt-in
toggle (acoustid_enabled, default OFF) + an AcoustID key input
(acoustid_api_key), wired in match-review.js — so the advertised feature is
reachable from the UI instead of only via a manual settings POST. Reuses
existing classes only; committed tailwind.min.css stays fresh.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
ChrisBeWithYou
|
a65d8cfa13
|
fix(enrichment): rank the canonical studio take over live/comp versions (#758)
* fix(enrichment): rank the canonical studio take over live/comp versions A flat MusicBrainz /recording text search ties every take of a song at the same score, so "AC/DC — Highway to Hell" returns a wall of live bootlegs and compilations with the 1979 studio version buried (or below the fetch limit). - build_recording_query: drop live-ONLY recordings (`-secondarytype:Live`). Compilations are deliberately kept — they REUSE the studio recording, so filtering them cuts the very recording we want (verified against MB). - _best_release / parse_recording_doc: pick the canonical studio album (primary Album, no Live/Compilation/Remix/... secondary type) for the displayed album/year, and expose a `studio` flag. - rank_candidates: since the combined score caps at 1.0 (perfect text match ties), break ties on the studio flag and — when the caller knows the audio length — on duration proximity, so the studio take wins over live/extended cuts. The studio distinction is intentionally NOT scored (a live take is still the right SONG), only re-ordered. - /api/enrichment/search: accept an optional `duration` param so a caller that has the audio but no library row (the editor's create modal) can pass the master-track length for the duration tiebreak. Verified end-to-end against live MusicBrainz: AC/DC "Highway to Hell" now returns the 1979 studio recording at #1 with the correct album + year. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(enrichment): official releases outrank unofficial studio albums _best_release sorted (clean, status_ok, date), so an UNofficial bootleg Album outranked an official Single/EP/comp — regressing canonical album/year and seeding cover-art from a bootleg for single-only songs. Order status_ok before clean: official first, then prefer a clean studio album among the official releases (still surfaces the studio album over an official live/comp album). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(enrichment): keep live recordings for genuinely-live charts build_recording_query unconditionally added -secondarytype:Live, but denoise() strips a '(Live at …)' qualifier from the query — so a chart that IS a live take had its only correct recording filtered out (both background enrichment and manual search). Skip the live filter when the source title carries a parenthetical live marker; a bare title word ('Live and Let Die') still filters. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(enrichment): drop the studio tiebreak when the chart is a live take Follow-through on keeping live recordings for live charts: rank_candidates still ranked the studio take ahead of a tied live one, so a live chart would auto-match the studio recording. Skip the studio tiebreak when the source title has a live marker — duration proximity + score then pick the right live version. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
ChrisBeWithYou
|
a86abadb14
|
settings: add host instrument profiles (#753)
* settings: add host instrument profiles Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com> * settings: add instrument pathway selection Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com> * fix(settings): profile-aware saves/resets/switch, provider tunings, bass-5 Five regressions from the instrument-profiles rework: 1. save_settings canonicalized profiles on EVERY save -> empty/unrelated POST froze default profiles into config.json (broke test_empty_post_preserves_all_existing_keys). Gate on the save touching instrument settings; GET already virtualizes profiles. 2. pathway is profile-mirrored, so the Gameplay reset (flat-key delete) was a no-op. reset_settings now resets pathway inside the persisted profiles too. 3. Per-profile tuning validation rejected provider/custom tunings (tuner plugin, /api/tunings). _valid_tuning_for_key now accepts a name unknown to every built-in table while still rejecting a built-in misapplied to the wrong key. 4. First-migration overwrote an explicit active_instrument_profile with the legacy-inferred one, so a fresh-config switch to 'bass' was lost. Use setdefault so an explicit request wins. 5. Pre-existing test_instrument_fields_persist used bass-5 + 'Drop D' (a 4-string tuning). Updated to the valid 'Drop A'. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(settings): partial-merge instrument_profiles; clamp tuning on string-count switch Two partial-update follow-ups: - save_settings normalized a POSTed instrument_profiles by FILLING every omitted profile with defaults and replacing wholesale, so a one-profile update reset the others. Validate each PROVIDED profile individually and merge the partial over the persisted set inside the lock — /api/settings is partial-merge. - the string-count picker posted only string_count, so the backend silently reset a now-invalid tuning to Standard while the UI kept the old value (settings/tuner desync). Clamp + post the valid tuning too, mirroring the instrument-switch path. 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> |
||
|
ChrisBeWithYou
|
41e907fa52
|
fix(library): serve art/load for songs mounted through a library junction (#766)
* fix(library): serve art/load songs mounted through a library junction A song library mounted through a directory JUNCTION/symlink subfolder (a library shared across app installs; the desktop app's own mounts) had broken album art and couldn't load: the scanner's rglob follows the junction and indexes the songs, but _resolve_dlc_path (via safe_join's .resolve()) followed the junction to its real target, saw it outside DLC_DIR, and rejected every song reached through it → 403 on /art, 404 on /art/candidates, broken covers. - _resolve_dlc_path now uses LEXICAL containment (os.path.normpath, no symlink following) so an in-library junction is allowed, while `..` traversal and absolute paths are still rejected (the traversal tests pin this). - safe_join is left STRICT (.resolve()-based) — it is the zip-slip / plugin- asset / avatar guard, where following a symlink out IS the defense — but gains an explicit NUL guard (on Python 3.13/Windows resolve() no longer raises on an embedded NUL, so the byte was leaking through; strictly-more- rejection, no effect on the zip-slip contract). Tests: test_dlc_junction (junction allowed; `..`/absolute/NUL rejected; the safe_join-stays-strict contrast). Existing traversal/safepath/art-candidates suites stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(library): reject Windows drive-letter paths in _resolve_dlc_path The new test_absolute_path_rejected pins 'C:/Windows/system32/x' → None, but on POSIX a drive-letter path isn't absolute, so Path(dlc)/'C:/…' becomes the contained relative dir '<dlc>/C:/…' and slipped through the lexical containment check (red on the Linux CI). Not an escape, but the traversal contract should hold cross-platform (a shared library is reached from either OS). Reject a path that is absolute or drive-qualified in either POSIX or Windows semantics before the containment check. Legitimate relative/junction paths are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
286c59707b
|
fix(tests): isolate plugin routes modules + redact .feedpak filenames (#736)
Two pre-existing failures the segfault had been masking (the run aborted at ~25%, so they never ran until #735 let the suite complete): 1) Tuner group (~24): plugins ship a bare-named routes.py, so sys.modules['routes'] leaked between plugin test dirs (achievements ran first, tuner got its module). Each plugin conftest now pops the stale 'routes' and an autouse fixture binds sys.modules['routes'] to that plugin's module for the duration of its tests (covers runtime 'import routes' in test bodies). 2) Diagnostics group (5): _SONG_FILENAME_RE never matched the tests' .feedpak/.archive filenames — it also lacked 'feedpak' (the current primary format), a real redaction gap. Added feedpak to the regex and switched the tests off the fake .archive to the real .feedpak. Verified: full suite 2183 passed, 0 failed. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
ChrisBeWithYou
|
8e953e8bc4
|
library: opt-in gap-fill — write confirmed missing metadata into the pack (R4a) (#724)
* library: opt-in gap-fill — write confirmed missing metadata into the pack (R4a)
The write-back contract agreed with the spec chair (alignment doc §7),
made executable, now that feedpak-spec 1.14.0 (mbid/isrc) is merged:
opt-in + user-initiated, adds ABSENT keys only, spec'd-keys allowlist,
values only from a CONFIRMED identity, atomic write + .bak. Single-song
only — batch write-back stays an open question with the chair.
- songmeta.gap_fill_sloppak: append-only manifest writer. Every added
key is absent by definition, so the new lines are APPENDED — the
author's existing bytes (key order, comments, formatting) survive
verbatim, unlike the metadata editor's full re-serialize. Directory
form gets a one-time manifest.yaml.bak + temp + atomic replace; zip
form reuses the editor's backup/temp/replace rewriter. Raises on any
already-present key: the never-clobber rule lives in the writer, not
just the callers.
- GET /api/song/{fn}/gap-fill: read-only preview — which of
album/year/genres/mbid/isrc are missing from the file (absent or
empty; year 0 = empty), with the values the enrichment match
supplies. Only a CONFIRMED identity is eligible (matched or a user
pin); review-tier rows are refused until a human confirms —
wrong-match > fast, same as everywhere else in the enrichment layer.
- POST /api/song/{fn}/gap-fill {keys}: writes the user-confirmed
subset. Proposals are RECOMPUTED under _song_io_lock, so a key that
gained an author value between preview and confirm is skipped, never
replaced. mbid/isrc written in canonical form only (validated).
DB stays scanner-consistent (album/year/genre columns + mtime/size
re-stat, cache invalidation + scan kick — the metadata editor's
contract). Demo mode blocks the write.
- Details drawer (Identity section): "Write missing info to file…" →
per-key checkbox confirm ("Only adds what's missing — nothing already
in the file is changed. A backup (.bak) is kept.") → written
confirmation; not-eligible states explain themselves. v3 only; no
new tailwind classes.
- Rides along: _manifest_exact_ids now strips ISRC display separators
(spec 1.14.0's strip rule) — a hand-authored "AU-AP0-90-00045" hits
the exact-match tier instead of silently falling back to text.
Tests: tests/test_gap_fill.py (10) — preview eligibility incl.
review-refusal + empty-as-gap, author-bytes-preserved-verbatim on dir
AND zip (with .bak content pinned), skip-not-replace on the mixed
request, the writer's ValueError guard, key validation, demo block,
DB sync; +1 hyphenated-ISRC test in test_mb_enrichment.py. 46 targeted
green; full-suite failure set A/B-identical to the main base (39
env/pre-existing).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN
* gap-fill: align preview with append-only writer (no cleared-value 500)
The R4a preview offered present-but-empty manifest values (album: '',
genres: [], year: 0) as gaps, but the append-only writer's never-clobber
guard raises on ANY key already present — so a user-confirmed POST for
those keys turned into a 500 "write failed" instead of filling the gap.
Appending can't fill an empty-but-present key anyway (it would duplicate
the YAML key).
Fix: _gap_fill_manifest_absent now treats only genuinely-MISSING keys as
gaps; a present-but-empty value is left to the metadata editor (which
re-serializes and can replace in place). This closes the preview→POST
mismatch — the preview never offers what the writer would refuse.
Tests: test_preview_treats_empty_values_as_gaps replaced by
test_preview_excludes_present_but_empty_keys (present-but-empty not
offered; genuinely-absent still offered) + test_write_present_but_empty_
key_is_refused_not_500 (POST → clean 409, file untouched, no .bak; a
genuinely-absent key alongside still writes). Closes the write-path blind
spot in the original empty-value test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
|
||
|
ChrisBeWithYou
|
7ef52cdd66
|
fix: Edit Metadata persists into .feedpak files (suffix gate predated the rename) (#725)
write_song_metadata dispatched zip-form packages on `suffix == ".sloppak"` only, while core reads both suffixes everywhere else (sloppak.SONG_EXTS, .feedpak being the current write extension). Editing a zip-form .feedpak's title/artist/album/year therefore silently fell back to a DB-only update, which looked fine until the next full library rescan re-derived metadata from the file and reverted the edit — the exact failure this module exists to prevent. Directory-form packages were unaffected (manifest-presence dispatch, not suffix). Gate on SONG_EXTS, add TestWriteSongMetadata regression coverage (both zip suffixes, mixed-case suffix, directory form, unknown-suffix fallback), and correct the stale scan_worker comment claiming .sloppak-suffix-only detection (the code already accepts both via is_sloppak). Claude-Session: https://claude.ai/code/session_01H1ZBEcZoJinde9ms5fAjwc Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
ChrisBeWithYou
|
0a8c8945ea
|
v3 library: Albums view — the client half of the album-condense work (#716)
* v3 library: Albums view — the client half of the album-condense work Follow-up to the query_albums endpoint: the UI that consumes it, plus the track-order plumbing the endpoint's track list needs. - Albums view (a fourth view toggle next to grid/tree/folder): album cards (cover / title / artist / track count) from /api/library/albums, respecting the active filter drawer; clicking one opens the track list with per-track play and a Play-album button that feeds the play queue (falls back to plain playSong when the queue plugin is absent). - Track order: the scanner now reads the feedpak `track`/`disc` fields (spec 1.12.0) into new nullable songs columns (idempotent ALTERs), and the album track list orders by the new `track` sort — disc, then track number, unauthored charts to the bottom by title. Charts without authored numbers keep working; they just sort alphabetically. - The albums view persists like the other view choices. 3 new tests: manifest track/disc extraction (and unauthored -> None), the disc->track->title sort order over /api/library, and the put() round-trip. Full-suite failure set matches the known env baseline (one tuner-config name swapped inside the suite-ordering flake family — the file passes 25/25 in isolation on clean main). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN * v3 albums: honour Genre/Match filters in album grid + detail (review fixes) The Albums view only partially respected the filter drawer: - /api/library/albums silently dropped the `genre` and `match` params the client sends via queryParams(), so with a Genre or Match filter active the album grid surfaced albums with zero matching tracks. Thread match_states/ genre through the endpoint -> query_albums -> _build_where, mirroring the /api/library grid route. (SmartCollection/pass-through providers keep their existing kwarg handling.) - The album-detail track list built its own params (provider/artist/album/ sort only), so it ignored ALL active filters — the track list and the Play-album queue could include songs the user had filtered out. Reuse queryParams({...}, {catalog: true}) so detail honours the same filters as the grid while pinning this album's artist/album and track order. +1 regression test (albums endpoint honours the Genre filter). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com> |
||
|
ChrisBeWithYou
|
80caf78306
|
v3 library: genre filter facet (reads feedpak genres) (#690)
* feat(v3): genre filter facet (reads the feedpak genres field) Adds a Genre facet to the library Filters drawer, populated from each song's primary genre. Server: a genre column (idempotent ALTER, indexed) written from the sloppak manifest's genres list on scan (primary = genres[0]); a genre band in _build_where (OR within the selected set); GET /api/library/genres for the facet's distinct list. Client: a multi-select Genre section mirroring the tuning/mastery facets. Follows the merged spec 1.12.0 genres field (#40). v1 stores only the PRIMARY genre (genres[0]); secondary genres aren't filterable yet. Threaded like the mastery filter (separate query_page kwarg, so query_artists /query_stats are unaffected) -- genre filters the grid view. Needs a rescan to backfill genre on existing packs (only packs whose manifest carries genres). Verified live: a sloppak tagged genres:[Metal, Rock] -> /api/library/genres returns [Metal]; ?genre=Metal returns it; ?genre=Rock (secondary) returns none; a plain song stays ungenred. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF * fix(v3): scope genre facet to local provider (PR #690 review) The /api/library/genres facet always read the local meta DB, so a remote provider showed local genres while the `genre` filter was a no-op on that provider's grid. Make the endpoint provider-aware: return an empty facet for remote providers (kind != "local") and keep serving genres for the local library and its smart collections, which share the local DB. The v3 client now passes the active provider, mirroring the tuning-names facet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com> |
||
|
ChrisBeWithYou
|
74cd08f765
|
library: MusicBrainz text matching + Match-Review UI (P8) (#710)
* library: MusicBrainz text matching + Match-Review UI — P8
Replaces the enrichment plumbing's no-op matcher (P7) with the real
pipeline, per the library-metadata design: a wrong match is worse than a
slow one, so medium confidence goes to a human review queue and never
straight to canonical values.
- lib/mb_match.py (new, pure — no network/DB/server imports): denoise
(author credits, (440Hz)/(Live)/(No Lead)/(v2) parentheticals,
diacritics/punctuation, ACDC / AC DC / AC/DC folding via compacted
token equality), token-set similarity, scoring with year/duration
corroboration bonuses, tier classification (auto needs combined
>= 0.95 AND per-field floors — a perfect-title cover by the wrong
artist, or a chart with no artist, can never auto-match), Lucene
query building, MusicBrainz response normalization.
- Matcher precedence in _enrich_one: content-hash cache copy (another
chart of the same recording matches with no network) -> manifest
mbid (tier 0) / isrc (tier 1) exact keys, feature-detected and
strictly shape-validated, read-only -> text search tiers
(auto / review / failed).
- Lifecycle: review rows store their ranked candidate list (JSON) and
write NO canonical fields until a human accepts; failed rows retry on
an exponential backoff (1 h doubling, 7 d cap) via the attempts
column; user-rejected rows never auto-retry; an identity edit
re-queues anything and resets the backoff; never-overwrite-manual is
enforced inside the single writer (apply_enrichment_match) so no call
path can forget it.
- Network: _mb_http_get is the one transport seam — throttled to
<= 1 req/s through P7's _enrich_throttle, identified with a real
User-Agent from VERSION, and a 503 pauses the whole pass without
burning attempts. Offline guard: no sockets under
FEEDBACK_ENRICH_OFFLINE or FEEDBACK_SKIP_STARTUP_TASKS, so pytest can
never reach MusicBrainz; the pass still stamps identity hashes
(two-phase), which is why every P7 test passes unchanged.
- Routes: GET /api/enrichment/review, POST
/api/enrichment/review/{filename}/accept|reject|pick, GET
/api/enrichment/search (throttled manual-search proxy). All four are
demo-mode blocked.
- Match facet: match= CSV accepted by /api/library AND
/api/library/stats (the A-Z rail's letter counts stay lockstep with
the grid) — review / matched (incl. manual) / unmatched / pending,
the same EXISTS idiom as the mastery facet.
- UI: static/v3/match-review.js (new, self-contained) — an ambient
"N to review" chip beside the song count (rendered only when
non-zero; silent on success, no toasts), and a review drawer on the
filter-drawer slide idiom (Escape + focus trap; row click accepts,
"Not a match" rejects, "Search instead" is the fix-match escape
hatch). songs.js gets the chip mount, a Match filter section, and
session-only match state; also fixes the latent applySavedPrefs bug
where restored filters dropped the mastery key, which made the
filter drawer throw for anyone with saved prefs.
- static/tailwind.min.css regenerated (scripts/build-tailwind.sh) for
the new utility classes; conflicts with sibling PRs resolve by
re-running the script.
Nothing is ever written to pack files — canonical values live only in
the song_enrichment display cache. Cover art caching and acoustic
fingerprinting are follow-up slices.
22 pure unit tests + 19 server tests (fake transport injected over the
_mb_http_get seam) + demo-mode route cases; full-suite failure set
A/B-identical with the change stashed vs applied.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN
* library: match-review modal + configurable auto-apply confidence (P8 R0)
Follow-up to the initial P8 commit, folding in the first round of tester
feedback on the review surface and the matcher's knobs:
- Review GUI is a centred MODAL now, not a sidebar — one chart at a
time (the scraper-review model from media-server / emulation-frontend
apps): the chart's current metadata with explicit amber
"Missing: album / year / cover art" chips (art detected via the art
request failing), candidates each carrying "Adds: year - genres -
ISRC" / "Shows as: ACDC -> AC/DC" per-field chips, and Skip /
Not a match / Search instead / Use selected with prev-next + arrow-key
navigation. Chip + window API surface unchanged, so songs.js needed no
edits for the rework.
- Auto-apply confidence is a SETTING: default drops 0.95 -> 0.90
(mb_match.AUTO_MIN; classify() takes an auto_min override). The
per-field floors are untouched and threshold-independent — a
perfect-title cover by the wrong artist still can't auto-match at any
setting. New validated settings keys: enrich_enabled (bool) +
enrich_auto_threshold (0.5–1.01; >1.0 = "Always review", since a
capped score can equal exactly 1.0). Read once per pass; disabling
gates only the BACKGROUND matcher — manual search/fix stays available.
- Settings -> Library -> "Metadata matching" card: enable toggle,
confidence select (85 / 90 / 95 / Always review), a Match Now button
(new POST /api/enrichment/kick, single-flight like every other kick,
demo-mode blocked), and a live status line fed by the same fetch as
the review chip. Markup in index.html per the v3 settings pattern,
wired by match-review.js, null-guarded so v2 no-ops.
- Review queue orders missing-data charts first — confirming those has
the most to gain; complete charts only stand to be re-labelled.
Tests: threshold moves the auto/review boundary via settings; the
enable toggle gates matching but not the manual proxy; settings
validation; kick route; queue ordering; classify(auto_min=...) floors.
Full-suite failure set byte-identical to the pre-change baseline.
tailwind.min.css regenerated for the modal's utility classes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN
* fix(library): lock MusicBrainz throttle across sleep + de-dup enrich queue (PR #710 review)
Hold a module-level lock across _enrich_throttle's read/sleep/write so the
background daemon and threadpooled sync search route serialize outbound MB
requests instead of bursting past the 1 req/s limit. De-dup the enrich queue
by filename so a changed-hash failed row isn't processed twice per pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
|
||
|
OmikronApex
|
749af31cc3
|
fix: correctly import and notate multi-staff (piano/keys) tracks from GP8 (#692)
* fix: correctly import and notate multi-staff (piano/keys) tracks from GP8 Fixes bass stave being dropped on import (bar-column enumeration bug) and wrong hand-split heuristic in notation_lift for chords straddling middle C. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: byrongamatos <xasiklas@gmail.com> * fix(gp-import): fold all grand-staff staves, per-stave tuning, playable hand-splits Addresses review on #692 (topkoa): - split_hands: only use the middle-C boundary when both resulting hands are within HAND_SPLIT_SPAN_SEMITONES, else fall back to the largest-gap heuristic — a hard middle-C split otherwise put a 19-semitone (unplayable) span in one hand for bass-under-treble voicings (e.g. E2+B3 under an Em7 shape). - Treat any multi-stave (grand-staff) track as keys end-to-end, so the stave-0 and folded stave-1+ notes share one encoding and note_count (which sums every stave column) matches what actually imports — closing the phantom-count case for grand-staff instruments the name/program heuristics miss (harp, celesta, marimba). - Fold *every* extra stave (stave_columns[1:]), not just stave 1. - Per-staff tuning fall-back to the track-level Tuning property so an untuned staff never yields an empty pitch list (silent note loss); via a shared _parse_tuning helper. - Extract _collect_column_notes / _merge_lh_notes so the GPX LH/RH pair merge and the GP8 grand-staff fold share one implementation and can't drift in tie/timing/dedup handling. - Rebuild filtered_to_raw from the already-computed stave_columns (one source of truth for the counting rule) and drop the dead num_raw_tracks/raw_tracks. Tests: grand-staff fold + bar-column offset (test_gp2notation.py); both middle-C split cases (test_notation_lift.py). CHANGELOG updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: byrongamatos <xasiklas@gmail.com> --------- Signed-off-by: byrongamatos <xasiklas@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com> |
||
|
topkoa
|
7713ca92f4 |
Merge remote-tracking branch 'origin/main' into feat/feedpak-jsonc
Signed-off-by: topkoa <topkoa@gmail.com> # Conflicts: # CHANGELOG.md |
||
|
Byron Gamatos
|
c8e0ad3f75
|
fix(gp-import): correct bass string count, lead/rhythm roles, preview note count (#601)
Four tester-reported GP-import issues, all in the converter/parse layer: * String count (bugs 2 & 4): <tuning> is padded to 6 slots, so a 4-string bass, 5-string bass and 6-string guitar were byte-identical and the real count was lost — a 5-string bass played on 4 strings and a 4-string bass showed a phantom B in the editor. Record the authoritative count in a new <tuning stringCount=N> attribute (gp2rs._build_xml) and trim the padded tail back to it on read (song.parse_arrangement). All consumers already trust a non-6 tuning length (arrangement_string_count, the editor's _stringCountFor and build-time _normalize_tuning_to_count), so this fixes the create-mode preview AND the built sloppak with no consumer changes. * Lead/Rhythm reversed (bug 3): guitar arrangements were named by appearance order (first guitar -> Lead), swapping roles for files that list Rhythm before Lead. Honor 'lead'/'rhythm' in the GP track name; unhinted tracks keep positional fallback. Applied to both convert_file's fallback (the editor's track_indices-without-names path) and _auto_select_gpx, with cross-role dedup so name-based and positional labels can't collide. * Preview note count (bug 1): the importer's per-track count included tie-continuation notes, which are folded into the previous note's sustain and never become separate RS notes (260 shown vs 241 imported). Exclude tie destinations so the preview matches the imported result. Adds regression tests for all three. Bug 5 (no stems from synced audio) is environment-dependent (best-effort demucs backend) and not addressed here. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
f3a5cb9ed3
|
feat(sloppak): expose full-mix original_audio alongside stems (#583)
Lets a .sloppak ship the single pre-separation full mixdown next to its
per-instrument stems, so the player can use the pristine original when nothing
is isolated (demucs recombination is lossy) and switch to separated stems only
when a slider drops below unity.
- lib/sloppak.py::load_song parses the optional manifest `original_audio:` key
into a new LoadedSloppak.original_audio field, with the same path-traversal
guard + permissive "missing → disabled" posture as the drum_tab loader.
- The highway WS song_info frame additively carries original_audio_url (served
by the existing /api/sloppak/{filename}/file/{rel_path} endpoint, None for
stems-only packs), has_original_audio, and has_stems.
- A stem-less, full-mix-only sloppak now sets audio_url to the full mix (plays
natively) instead of emitting audio_error.
Message shape stays a stable contract — all additions are purely additive.
Tests: tests/test_sloppak_original_audio_load.py (6 passing).
Closes #580
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Bret Mogilefsky
|
af2949677a
|
rename: slopsmith → feedBack, byron → got-feedBack (#537)
* Update GitHub repo references from feedback* to feedBack* * rename: slopsmith -> feedBack, byron -> got-feedBack Renames across the entire codebase: - slopsmith/Slopsmith/SLOPSMITH/SlopSmith -> feedBack/FeedBack/FEEDBACK/FeedBack - byron/Byron/Byrongamatos -> got-feedBack/got-feedBack/got-feedBack - /home/byron/ -> /opt/got-feedBack/ - byron@ougsoft.com -> hi@got-feedBack.org - github.com/byrongamatos/ -> github.com/got-feedback/ - com.byron. -> com.got-feedback. - SLOPSMITH_ env vars -> FEEDBACK_ with backward-compat fallback - Protocol/storage strings migrated with read-old/write-new pattern - window.slopsmith JS API -> window.feedBack (canonical) + backward-compat alias Refs: #rename-slopsmith * rename: complete regen against current main + fix backward-compat alias Regenerated the slopsmith->feedBack / byron->got-feedBack rename on top of current main (3 commits had landed since the branch: #572/#554/#574), resolving the four content conflicts in favour of main's newer content (autoplay/auto-exit, accuracy-badge, Virtuoso re-home, feedpak badge). Completion fixes on top of the mechanical rename: - Re-apply rename to post-branch content the original rename never saw: window.slopsmith(.Tour) consumers in lessons.js / notifications.js / onboarding-tour.js, and the matching JS + python tests (autoplay_exit, progression_*, test_feedpak_extension FEEDBACK_* env vars). The test env vars now match server.py (which reads FEEDBACK_SYNC_STARTUP / FEEDBACK_SKIP_STARTUP_TASKS), so the sync-startup test exercises the real path again. - Restore the window.slopsmith backward-compat alias dropped during conflict resolution, and move the bus aliases to AFTER the _feedBackExisting merge block so they reference the fully-assembled object (also fixes the loop_api.test.js API-surface regex, which the original PR latently broke). - Drop the stray empty data/web_library.db (runtime DB lives in CONFIG_DIR) and gitignore it. - Fix stale tone-source test: feed[dB]ack -> fee[dB]ack to match shipped source labels. Verified locally (org CI billing-blocked): JS 819/819 pass; pytest 1669 passed / 1683 collected with 0 import errors; zero residual slopsmith/byron except the two intentional window.slopsmith aliases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * rename: implement advertised backward-compat + prune dead community plugins Address gaps where PR #537's "Backward compatibility" section was advertised but not implemented, and clean up the community plugin list. Env vars (FEEDBACK_* canonical, legacy SLOPSMITH_* honoured): - New lib/env_compat.py (getenv_compat / env_flag_compat) + tests. server.py (_env_flag + all FEEDBACK_* reads), diagnostics_hardware, gp2midi and tailwind_rebuild now resolve the legacy alias, so existing SLOPSMITH_UI / SLOPSMITH_PLUGINS_DIR / etc. deployments keep working. - Fix the rename collapsing plugins/__init__.py and minigames/routes.py from `FEEDBACK_PLUGINS_DIR or SLOPSMITH_PLUGINS_DIR` into a redundant `FEEDBACK_ or FEEDBACK_` (the fallback was silently lost). Storage (app.js update-channel): - Read feedBack-update-channel, fall back to legacy slopsmith-update-channel, and clear the legacy key on write — so a user's update-channel preference survives the rename instead of resetting to "stable". Community plugin list (README): the rename rewrote third-party repo URLs we don't own. Probed every one; their owners never renamed, so: - Restore the 13 live community plugins to their real slopsmith-* names. - Prune 6 that are 404 to the public (topkoa splitscreen/stems, OmikronApex tuner, Jafz2001 nam-rig-builder, DeathlySin song-preview, Erikcb91 shuffle). - Fix a pre-existing Guitar Theory clone-command typo (nam-tone -> guitar-theory). Verified: env_compat 7/7, JS 819/819, pytest 1690 collected / 0 import errors, rename-sensitive + startup suites green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: byrongamatos <xasiklas@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Sin
|
37aedd4251
|
Restore GP6/7/8 tremolo picking on import (#572)
* Map GP7/8 tremolo picking on import GP7/8 (GPIF) encodes tremolo picking as a beat-level <Tremolo> element, which the importer ignored — so tremolo was silently dropped on .gp import, while note vibrato and the GP3-5 path were unaffected. Read the beat-level <Tremolo> and set the note tremolo flag across the beat, independent of vibrato (a note can carry both). Signed-off-by: Sin <deathlysin@outlook.com> * test: cover GP6/7/8 tremolo-picking import Extract the beat-level <Tremolo> detection into a pure _beat_has_tremolo helper (mirroring the tested _note_has_vibrato) so it's unit-testable in this suite's fixture-free style, then add: - 4 unit tests on _beat_has_tremolo: direct <Tremolo> child detected (rate-agnostic), absent -> False, direct-child-only (nested Tremolo ignored), independent of the VibratoWTremBar whammy property. - 1 end-to-end test driving convert_file via a crafted GPIF (monkeypatched _load_gpif): a tremolo-picked beat's note serializes tremolo="1" while a plain beat stays "0". Both the detection and integration tests fail without the fix; full GP suite 238 passed. Refactor is behavior-identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Signed-off-by: Sin <deathlysin@outlook.com> Co-authored-by: Byron Gamatos <xasiklas@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
K. O. A.
|
32127bc70b
|
feat: save as .feedpak; discover and load both .feedpak and .sloppak (#553)
* feat: save songs as .feedpak; discover and load both .feedpak and .sloppak The open song format was renamed sloppak -> feedpak (public spec lives in the feedback-feedpak-spec repo), but the server still wrote and recognized only `.sloppak`. The two are byte-identical on disk. Read both suffixes everywhere songs are discovered, uploaded, and loaded; writing the new `.feedpak` suffix is handled in the editor plugin repo. Keep the internal `format` tag `sloppak` so existing feature gates (stems, drums, keys) are untouched, matching the "internal rename not landed yet" stance. - lib/sloppak.py: add FEEDPAK_EXT / SLOPPAK_EXT / SONG_EXTS; is_sloppak() now matches either suffix (covers all 7 callers). - server.py: union scan glob over SONG_EXTS; widen loose-folder exclusion, settings DLC count, upload gate (_ALLOWED_SONG_EXTS) and zip-magic check; refresh user-facing messages to .feedpak. - static: library format filter relabeled Sloppak -> Feedpak (value stays sloppak, matches both); badge text SLOPPAK -> FEEDPAK in v2 + v3; filename-suffix detection and upload drag-drop filter accept both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: topkoa <topkoa@gmail.com> * test: cover .feedpak/.sloppak dual-suffix support Add tests/test_feedpak_extension.py pinning the four paths PR #553 widened so a refactor can't drop .sloppak back-compat or stop accepting .feedpak: - is_sloppak / SONG_EXTS suffix detection (file + dir form, case-insensitive) - _background_scan discovery glob unions over both suffixes - POST /api/songs/upload accepts both, rejects wrong suffix + non-zip - save_settings DLC count includes both suffixes 19 tests, all passing; reuses the existing scan_module / TestClient / isolate_logging fixtures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Signed-off-by: topkoa <topkoa@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com> |
||
|
Byron Gamatos
|
73e3fe2226
|
fix(song): sanitize caged + guideTones on emit, not just decode (#544 follow-up) (#547)
Post-merge Codex review of #544 found chord_template_to_wire emitted ct.caged and ct.guide_tones raw — so a directly-constructed ChordTemplate(caged="X") or guide_tones=[99] would write a schema-invalid value to the feedpak wire, even though the decoder guards on input. The spec constrains caged to C/A/G/E/D and guideTones to 0..11. Run the same _sanitize_caged / _sanitize_guide_tones guards on emit: caged is written only when a valid enum value, guideTones only as the in-range ints (empty result -> key omitted). +1 test (invalid caged dropped, mixed guideTones filtered to the valid in-range subset, wholly-invalid list omitted). Codex-reviewed: clean. 154 song tests pass. Part of got-feedback/feedback#334. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
4195b73877
|
feat(song): wire caged + guideTones chord-template fields (§6.6) (#544)
Mirror the voicing field for the two deferred FEP #24 harmony annotations on ChordTemplate: - caged: str ("C"/"A"/"G"/"E"/"D", "" = unset) - guideTones: list[int] (semitone offsets 0..11 above the root, [] = unset) Both are default-omitted on the wire and sanitized on decode (caged enum-guarded, guideTones filtered to in-range ints, rejecting bool) so a malformed value can't round-trip. GP import is untouched — GP carries no CAGED / guide-tone data. Teaching annotations only; never fed to a grader. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
ea22791984
|
feat(core): carry chord harmony fn + template voicing on the wire (§6.3.1, §6.6) (#540)
Add two OPTIONAL per-chord harmony annotations (feedpak 1.7.0), mirroring the
teaching-marks (fg/ch/sd) wire work:
- Chord.fn (instance): {rn, q, deg} harmonic-function object, key-dependent.
Validated by _validate_fn on BOTH decode and emit so a partial / out-of-range
fn (which would fail the schema's required-keys rule) never rides the wire.
Default-omitted, mirroring bend bnv.
- ChordTemplate.voicing (template): key-independent voicing-type string
("open", "triad", "shell", "drop2", "barre", ...). Emitted only when
non-empty; non-string wire values fall back to "".
Display/teaching only — never fed to a grader (honesty rule). fn auto-derivation
is DEFERRED (carry-only): a complete rn/q needs chord-quality analysis, and a
deg-only fn would be schema-invalid, so server.py carries author-provided fn
unchanged. GP import unchanged (no reliable per-chord function/voicing).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|