mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-23 21:32:33 +00:00
fix/loops-lock-atomicity
156 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
byrongamatos
|
ea6a79767b |
fix(loops): take the DB lock for the count+insert and the list read
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
|
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
|
d6f2df14f7
|
feat(server): add appstate.py, the router seam (R3) (#833)
* feat(server): add appstate.py, the router seam (R3)
Routes moving out of server.py need `meta_db` and friends, but must not
`import server` -- that goes circular the moment server imports them back.
server.py keeps CONSTRUCTING its singletons and now injects them once via
`appstate.configure(...)`; routers read them back as module attributes at call
time (`import appstate; appstate.meta_db`). The Python analogue of the frontend
refactor's `configureX({...})` seams and of the plugin `setup(app, context)`
contract: dependencies flow one way, server -> routers -> appstate.
Two properties are load-bearing, both pinned by tests/test_appstate.py:
1. `import appstate` constructs nothing and touches no disk. This is why the
~49 test fixtures that `sys.modules.pop("server")` + re-import (to rebuild
meta_db under a patched CONFIG_DIR) keep working UNTOUCHED. A singleton
owned by appstate would survive that pop and go stale -- verified.
2. Reads must be late-bound. `from appstate import meta_db` freezes the binding
and defeats both a later configure() and monkeypatch.setattr -- the same
read-only-binding trap as ES imports.
configure() raises on an unknown slot instead of silently creating a global
nothing reads, and the suite asserts server ACTUALLY calls it. Negative-checked:
dropping the configure() call fails exactly the two wiring tests while the other
five stay green -- those five are the false-green a seam test must not be.
The new suite imports server through an `isolated_server` fixture that patches
CONFIG_DIR to tmp_path and closes both DB connections on teardown. An unguarded
`import server` constructs MetadataDB + AudioEffectsMappingDB under the real
`~/.local/share/feedback` (reproduced: running the file alone created
web_library.db + audio_effects.db there). The full suite now leaves the real
config dir untouched.
Packaging: `COPY appstate.py /app/` plus a .dockerignore allowlist entry. That
file opens with a blanket `*` exclusion, so root-level Python must be re-allowed
explicitly -- without it the image build fails on the COPY. Verified against the
real docker daemon (build context reaches /app/appstate.py). docker-compose.yml
gains the dev bind-mount; docker-compose.nas.yml runs the baked image, so the
COPY covers it. `routers/` will need the same two entries when it lands.
Verified: pyflakes clean; pytest 2348 passed (2341 + 7 new); eslint 0 errors;
boot smoke serves /api/version, /api/library, /api/audio-effects/mappings, and
all three migrated plugins' src/ graphs, with `appstate.meta_db is server.meta_db`
asserted against the live import.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(appstate): address CodeRabbit — restore slots on teardown, really re-import
Two real findings on #833, both fixed:
(1) `isolated_server` closed server's DB connections but left `appstate.meta_db`
and `appstate.audio_effect_mappings` published and pointing at the closed
handles -- a live-looking, dead singleton for any later test. Teardown now
snapshots and restores both slots.
(2) `test_reimporting_server_republishes_the_fresh_singletons` never performed a
second import: it only re-asserted what `test_server_wires_the_seam` already
covers, so it could not detect the very staleness it names. (I introduced
that regression while fixing Codex's CONFIG_DIR isolation finding.) It now
pops `server`, re-imports under a SECOND CONFIG_DIR, and asserts the seam
republishes -- `second_server.meta_db is not first_db` and
`appstate.meta_db is second_server.meta_db`.
Negative-checked both directions: simulating an appstate-OWNED singleton
(configure() only-first-wins) now fails the re-import test, and dropping
server's configure() call still fails exactly the two wiring tests.
NB CodeRabbit's committable suggestion inserted the snapshot above the
fixture docstring, which would have demoted it from __doc__; written by hand
instead.
pytest 2348 passed; the full suite leaves the real ~/.local/share/feedback
untouched.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
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> |
||
|
OmikronApex
|
1c1a0e0268
|
feat(audio): renderer-bus feeder — song audio into engine output under exclusive mode (Phase 2) (#828)
Some checks are pending
ship-ci / ci (push) Waiting to run
* feat(audio): route feedpak full-mix natively under exclusive output Song playback runs through the renderer, which WASAPI-exclusive (and ASIO) output silences. Route single-mix feedpaks (stem-less original_audio packs AND single-stem packs) onto the engine's backing transport when the output device type is exclusive-style, and migrate back to HTML5 when it isn't. Extends /api/audio-local-path to resolve /api/sloppak/.../file/... URLs via the same containment guards as serve_sloppak_file. Multi-stem packs stay on the WebAudio path (Phase 2). Includes [feedpak-route] transition-gated diagnostics logging. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(audio): renderer-bus feeder — mix renderer song audio into engine output (Phase 2) Under exclusive-style output the native backing transport (Phase 1, #824) carries loose /audio/ songs and feedpak full-mixes, but not the stems plugin's multi-stem WebAudio graph or tracks JUCE rejected. The feeder taps the renderer-side master with an AudioWorklet, re-points the owning AudioContext at a null sink so it keeps rendering without a device, and pushes ~10 ms chunks over IPC into the desktop engine's renderer bus (feedBack-desktop#90 follow-up). Inert in the Docker sphere and in shared mode. Validated by the fix12 tester spike: null-sink rendering works, clocks hold, no overflow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
OmikronApex
|
1b3178037b
|
feat(audio): route feedpak full-mix natively under exclusive output (#824)
Song playback runs through the renderer, which WASAPI-exclusive (and ASIO) output silences. Route single-mix feedpaks (stem-less original_audio packs AND single-stem packs) onto the engine's backing transport when the output device type is exclusive-style, and migrate back to HTML5 when it isn't. Extends /api/audio-local-path to resolve /api/sloppak/.../file/... URLs via the same containment guards as serve_sloppak_file. Multi-stem packs stay on the WebAudio path (Phase 2). Includes [feedpak-route] transition-gated diagnostics logging. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Byron Gamatos
|
950e348357
|
R0: module-migration rails (src/ serving, live-edit cache, scriptType loading, governance) (#812)
Some checks failed
ship-ci / ci (push) Has been cancelled
Host enablement for the plugin ES-module migration: sandboxed /api/plugins/{id}/src/ serving, no-cache+weak-ETag/304 live-edit caching on src/+screen.js+assets, scriptType:module loader injection + scriptType/minHost manifest passthrough; constitution v1.2.0 + module playbook + signed size-exemptions register + maintainer/CI-only ESLint gate; rerunnable perf-baseline harness. Reviewed by Codex (local), Copilot, and CodeRabbit.
|
||
|
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> |
||
|
Matthew Harris Glover
|
fadaa154e9
|
feat(library): sort and badge by personal difficulty rating (#810)
* feat(library): sort and badge by personal difficulty rating
Adds sort=difficulty/difficulty-desc to the library API (correlated
subquery over song_user_meta.user_difficulty, unrated songs pushed to
the bottom either direction, same pattern as the existing mastery
sort) and surfaces the rating as a badge on library cards in both the
v2 grid/tree views and the v3 grid. The rating itself already existed
(song_user_meta) — this just makes it sortable and visible, so it's
no longer only readable in the per-song edit drawer.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(library): escape difficulty badge, wire tree view, add changelog+tests
- Wrap song.user_difficulty in esc() at both badge call sites
(static/app.js ~2082 and ~2283) for XSS-consistency with the
sibling tuning badge, which already uses esc().
- server.py: query_artists (the classic tree view's data source, used
by /api/library/artists) never batch-attached user_difficulty the
way query_page does for the grid, so the tree-view difficulty badge
added in
|
||
|
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>
|
||
|
byrongamatos
|
9fb63fd3b5 |
fix(tuner): anchor injectPlayerButton to a direct-child button (feedBack#800)
`injectPlayerButton()` anchored the injected Tuner button with
`controls.querySelector('button:last-child')`, which can match a NESTED
button that is not a direct child of `#player-controls`. `insertBefore(btn,
nestedButton)` then throws `NotFoundError` (the reference node must be a
direct child); since injection runs from the tuner's `screen:changed`
handler, the throw propagated out of the player-screen transition and
stalled its render. The v3 path was already safe (plugin-control slot);
only the classic anchor was bad.
Use `:scope > button:last-of-type` (direct child only) with a
`parentNode === controls` guard before insertBefore, falling back to
appendChild. Bump plugins/tuner 1.3.3 → 1.3.4.
Test: tests/plugins/tuner/js/inject_player_button.test.js — extracts the
real function and runs it over a faithful DOM model whose insertBefore
enforces the direct-child invariant; the nested-last-button case reproduces
the throw on the old anchor and passes on the new one (5 tests).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: byrongamatos <xasiklas@gmail.com>
|
||
|
Byron Gamatos
|
69c8ad4e0c
|
fix(settings): don't let a stale DLC path block saving the Demucs server address (#795)
Some checks are pending
ship-ci / ci (push) Waiting to run
The v3 Settings "Save" button posts dlc_dir together with demucs_server_url, default_arrangement and av_offset_ms in one request. POST /api/settings validated dlc_dir first and early-returned "DLC directory not found" before it ever processed demucs_server_url, so on a machine whose DLC path doesn't resolve (fresh install, unplugged/network drive, a path carried over from another machine) setting the Demucs server address silently failed — reported in got-feedBack/feedBack-demucs-server#3 (macOS 07-05 nightly). - server: a non-resolving dlc_dir is now recorded as a warning and skipped rather than aborting the whole POST, so the co-submitted keys still persist. The bad path is surfaced via a new additive `warnings` field and folded into `message` so the settings status line still shows it. - client (v3): the Demucs input now autosaves on blur/enter via a single-key persistSetting POST, like every other v3 setting, so it never depends on the coupled Save button. - tests: cover the decoupling and the unchanged happy path. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
ChrisBeWithYou
|
612b1f2e0d
|
feat(v3): choose handedness in the instrument selector + onboarding (give lefties a break) (#793)
Some checks are pending
ship-ci / ci (push) Waiting to run
* feat(v3): add a handedness (left-handed) choice to the instrument selector + onboarding Left-handed players could already mirror the highway, but only via a buried Settings toggle they had to find AFTER setup -- so a lefty went through the tour, the tuner and calibration all right-handed first (community callout). Add a "Handedness: Right / Left" row to the v3 instrument badge popover, alongside Instrument / Strings / Tuning (all player-orientation choices). It writes the same lefty preference -- highway.setLefty when a live highway exists (flips it immediately + persists), else the 'lefty' localStorage key the highway reads on init -- and keeps the Settings "Left-handed" checkbox in sync. The first-run tour's "Choose your instrument" step, which runs before the tuner/audio- calibration steps, now calls it out so lefties flip it up front. Frontend-only, additive. Full core JS suite green (938). Tests: tests/js/badges_handedness.test.js. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UR2Cr7GEu3yMY7SrfxH6c1 * docs: split the spliced Handedness/Colorblind CHANGELOG entries A rebase pasted the Handedness bullet over the Colorblind preset entry's bold lead, merging two unrelated Added entries into one run-on bullet. Restore them as two separate bullets. 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
|
4f6dc233f1
|
feat(player): seed editor region handoff state (#762)
Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com> |
||
|
ChrisBeWithYou
|
6aaa2dcf47
|
feat(v3 library): batch→popup handoff + English-base romaji (metadata-curation capstone) (#782)
* feat(v3 library): click a "No match" badge to fix it — batch → popup handoff
Connects the two halves: the "No match" badge (the unmatched pile) now opens the
Fix-metadata popup for that song in one click, instead of right-click → menu.
The resting badge becomes interactive (pointer-events-auto + hover), carrying a
data-meta-fix hook; wireCards opens window.__fbFixMatch(playTarget) on click and
stops propagation so it doesn't also play the card. Batch tile states stay
non-interactive. Loop becomes: Unmatched filter → see the pile → click one →
fix it. tailwind.min.css regenerated for the badge's hover classes.
* feat(v3 library): show the author's romaji, not blank/native script (English base)
Two changes so an English-speaking base never sees a blank name or native script:
- Filename romaji fallback: a blank-artist CDLC pack ("Artist_Title_v1_p") shows
nothing useful (artist blank; title = the raw filename), and a match fills it
with kanji/kana. query_page + pack_fields now surface the author's own romaji
parsed from the filename ("Junko Yagami — BAY CITY") when the pack has no
artist of its own — display-only, keyset-safe (raw title stashed for the
cursor), a real pack artist or a user override still wins.
- Smart adopt: "Use these values" now KEEPS the readable romaji name + title the
card already shows and takes only album/year/genre (+ art via the pin) from the
match, so identifying a Japanese song gives "Junko Yagami — BAY CITY — FULL MOON"
with the right cover, never native script.
Tests: romaji fallback fires for a blank-artist CDLC pack (grid + pack_fields
agree) and is left alone when the pack has a real artist.
|
||
|
ChrisBeWithYou
|
b914612f9d
|
fix(highway_3d): recover from WebGL context loss instead of crashing on alt-tab (#790)
Switching the active window / alt-tabbing away (most often on Windows) can trigger a GPU context reset. The 3D highway's WebGL renderer had no webglcontextlost handler, so a lost context was left to escalate into a render-process crash -- matching the intermittent "randomly crashes when I change windows" desktop reports. The renderer now binds webglcontextlost/webglcontextrestored on its own WebGL canvas (ren.domElement): the loss is preventDefault()'d so the browser keeps the context restorable, draw() bails while the context is down so no GL work runs on a dead context, and on restore the viewport is re-applied and rendering resumes (Three re-uploads scene resources on the next frame). Listeners are removed in teardown. Root cause is a strong hypothesis -- the crash is intermittent and unreproducible -- but the fix is low-risk and additive and closes a real gap: there was no context-loss handling anywhere in the renderer. plugins/highway_3d 3.31.2 -> 3.31.3. Tests: tests/js/highway_3d_context_loss.test.js (source-contract, like the other highway_* tests). The sibling keys_highway_3d / drum_highway_3d renderers share the same gap -- follow-up in their repos. Claude-Session: https://claude.ai/code/session_01UR2Cr7GEu3yMY7SrfxH6c1 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
ChrisBeWithYou
|
3100d68a45
|
feat(v3 library): genre field in the Fix-metadata popup Details tab (#780)
Adds Genre as a fifth Details field (edit / lock / revert / Yours-Pack provenance), backed by the existing override store. To make it actually useful, the genre FILTER and FACET now resolve the per-song override (effective genre = override else scanned pack genre) — guarded so the common no-override case stays on the plain indexed column — so a corrected/added genre is immediately browsable. Genre stays a library-only overlay: it is NOT a write-to-file field (split WRITE_FIELDS = the four file-safe fields from the five DETAIL_FIELDS), so Write to file leaves the genre override in place and the copy says so. The Match→Details bridge also carries a candidate's first genre. Tests: effective-genre facet + filter, and that a value-less lock doesn't invent an effective genre. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
ChrisBeWithYou
|
af1170cec3
|
feat(v3 library): "Fix metadata" popup — per-song override + lock, cover picker, MusicBrainz + AcoustID (#777)
* feat(library): metadata override + lock store, enforced by enrichment (popup slices 1–2)
Backend foundation for the Fix-metadata popup. Not yet surfaced in the UI (the
display + 3-tab popup are the next slices); no PR until it's user-visible.
Slice 1 — the store:
- `song_field_override(filename, field, value, locked)` table: a reversible
DISPLAY overlay (never written to the pack), filename-keyed so it survives a
rescan (never purged by delete_missing) and is dropped only with the song.
- DB methods (partial upsert that drops empty+unlocked rows; batch map) +
`GET`/`PUT /api/song/{fn}/overrides` (field allowlist title/artist/album/
year/genre; clearing rides PUT since DELETE /api/song/{path} shadows sub-
routes; PUT demo-blocked).
Slice 2 — locks respected by enrichment:
- The auto-matcher composes a per-song `_compose_lock_filter` onto the global
apply-filter, so a match still applies IDENTITY (mbid/release → art) but never
re-canonicalizes a LOCKED display field.
- Gap-fill (write-to-file) skips locked album/year/genre — writing the matched
value would be exactly the clobber the lock exists to prevent.
- Review/manual picks bypass the filter (an explicit confirm overrides a lock).
Tests: store semantics + rescan-survival + API; the lock filter + reader; an
auto-match leaving a locked field un-canonicalized; gap-fill excluding locked
keys.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(library): show per-song overrides in the grid (popup slice 3)
The grid now displays the user's per-song title/artist/album/year override
in place of the pack value ("grid shows only overrides") — a matched
MusicBrainz canon never silently re-titles a card; canon stays in the
Details drawer + art. Overlaid in Python over the visible window, keyset-safe
like the P4 artist-alias re-label: the seek still runs on the raw column, and
the one overridable keyset column (title) stashes its raw value for the cursor
so paging never skips/dupes. The private stash is dropped from the payload.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(library): 3-tab Fix-metadata popup — Details / Cover art / Match (slice 4)
Turns the thin single-song fix-match modal into the Plex-style metadata
editor reached from a card's "Fix metadata…" menu:
- Details tab: type + lock the displayed title/artist/album/year. Values
ride the reversible override store (GET/PUT /api/song/{fn}/overrides); each
field sits on its pack value (Yours/Pack provenance + revert-to-pack), a lock
pins it against auto-match, and Save repaints the grid via library:changed
(slice-3 overlay). This is the real tool for the blank-artist city-pop pile
MusicBrainz can't surface — you just type the right title.
- Cover art tab: hands off to the shared image picker (its own modal); the
pick refreshes the thumbnail everywhere.
- Match tab: the existing MusicBrainz search + candidate/pick flow, refactored
into shared body/footer helpers (the queue-review flow is untouched).
Backend: GET /overrides now also returns the pack baseline so the Details tab
can pre-fill + show provenance. tailwind.min.css regenerated (build-tailwind.sh)
for the popup's new utility classes.
Identify-by-audio (AcoustID) is deferred: it lives in unmerged PR #759, off
main — the Match tab gains the button once #759 lands and this branch rebases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(library): wire "Identify by audio" in the tabbed popup's Match tab
The AcoustID Identify button (#759) merged in referencing an out-of-scope
`panel` in the wiring — a leftover from the pre-popup fix-match modal that my
tab refactor renamed to `root`. Under strict mode that threw, so the handler
never attached and the button did nothing. Scope it to `root` (the tab body),
which is where the search-results area it renders into lives.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(library): make "Identify by audio" outcomes unmistakable
An empty AcoustID result read the same as a broken button. Each state now says
plainly which outcome it is — ✓ fingerprinted-but-no-match vs no-audio vs off vs
unavailable — and, in the popup, points at the manual fallback (Search, or set
the album in Details + cover in Cover art by hand). A ✓ marks the states that
actually ran, so "worked, found nothing" no longer looks like a failure.
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
|
3e036e3db6
|
feat(v3 library): persistent "no match" badge + Unmatched quick filter (#781)
* feat(v3 library): persistent "no match" badge + Unmatched quick filter The Refresh-Metadata batch (#764) shows a transient per-tile "no match" only while a pass runs, so the unmatched pile goes quiet at rest. Two additions make it visible + reachable: - Persistent per-card "No match" badge: query_page now marks each row `unmatched` (a cheap failed-set membership like favs/estd), and enrichBadge paints a subtle resting marker for those cards — tracked in a `_unmatched` set so a batch tile clearing falls back to it instead of wiping it. A live batch tile still wins while a pass runs. - "Unmatched" toolbar toggle (local-only): one click applies the same filter as the drawer's Match → Unmatched (match_state='failed'), so the no-match pile is a click away right after a batch. Re-queries + reflects active state. Test: query_page flags a failed row + the match=unmatched filter returns it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(v3 library): repaint persistent no-match badge after metadata tile-clear _clearMetaTiles removed every .v3-meta-tile node — including the new persistent 'No match' resting badge, which derives from _unmatched rather than _metaTile. A metadata rescan's tile-clear therefore dropped the badge until the next scroll re-rendered the card. Repaint it from _unmatched. 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> |
||
|
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
|
c7aa5a10b0
|
fix(v3): recycle library grid cards on scroll instead of rebuilding the window (#742)
The virtualized v3 Songs grid rebuilt its entire visible window (grid.innerHTML = _renderCardsRange(...) + a full wireCards pass) every time it slid by one row. Each row-boundary crossing was therefore a heavy synchronous frame — reparse ~60 cards, re-attach hundreds of listeners, reflow — that stalled the main thread and buffered held-arrow key-repeats, flushing them in a burst. Testers saw the library "go super fast for a second then slow down," skipping "every so many scrolls," up or down, at the same spots each time. It hitched scrolling back up over already-loaded songs too, because the cost was DOM teardown, not fetching. renderWindow() now reconciles the window in place: it reuses the card nodes that stay on-screen and builds only the row that enters/leaves (~6 nodes per slide instead of ~60). Nodes are keyed by absolute index (data-idx) with a real-vs-skeleton + select-mode signature (data-sig) so hole-fills after a page fetch and select-mode toggles still rebuild exactly the nodes that changed. wireCards()'s existing data-wired guard then wires only the freshly-built nodes, so per-slide listener churn drops with it. Everything keyed off data-fn (favorites, ⋮ menu, right-click, selection, accuracy badges, A–Z rail) is unaffected. Follow-up to the stage-2 virtualized grid (#636 item 3). Frontend-only. Tests: tests/js/v3_songs_window_recycle.test.js — window stays [start,end) contiguous and in-window node identity is reused across a down-then-up scroll; select-mode toggle and a rail-seek jump rebuild correctly. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com> |
||
|
ChrisBeWithYou
|
fa2d12222a
|
feat(v3 library): "Refresh Metadata" button + per-view re-match + filename-artist seed (#764)
Adds a media-server-style "Refresh Metadata" control to the Songs toolbar
(beside "⟳ Refresh") — the metadata counterpart to a file scan.
- Re-matches the songs currently SHOWN (the visible grid window) against
MusicBrainz: a per-view refresh that's visible even on an already-matched
library. The button doubles as Stop while a pass runs; a batch progress bar
+ per-tile queued→working→done badges show what's happening. User-pinned
`manual` matches are never re-matched.
- Backend: POST /api/enrichment/{cancel,states,rematch}; /status gains
total/matched/current/cancelling; a cooperative cancel Event is checked
between songs in the match + art phases so Stop halts without waiting for
the whole queue. `states` is read-only (open); `cancel`/`rematch` are
demo-blocked.
- Matcher: when a pack's `artist` field is blank (common in community
charts), derive artist/title from the CDLC `Artist_Song-Title` filename
convention as a SEARCH SEED so text matching can identify it — the displayed
values still come only from the confirmed MusicBrainz match, nothing
estimated is shown as author-set. Rescues blank-artist packs that otherwise
always failed.
Tests: enrichment_states_for, the three new routes, cancel-halts-a-pass,
kick-clears-stale-cancel, filename parse, blank-artist seeding, and
present-artist-not-overridden.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.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
|
14eaad09e9
|
feat(library): add Star-Spangled Banner + Ode to Joy starter content (#744)
Add two more public-domain starter songs alongside Für Elise, wired into _BUILTIN_STARTER_SOURCES so they seed into DLC_DIR/starter/ on first run: - The Star-Spangled Banner (lead) — John Stafford Smith; cleaned the "Unknown" artist / placeholder album, author "Fee[dB]ack". - Ode to Joy (lead/rhythm/bass + drums) — Beethoven. Replaces the raw "Ode to Joy (VST Cover)_The Adicts.feedpak" that was committed to main but never added to the seed list (so it bundled 23 MB of dead weight and never appeared). Fixed metadata (artist Beethoven, year 1824, author "Fee[dB]ack"), and repointed the stem from the 22 MB editor WAV to the byte-identical-render full.ogg (both exactly 85.324 s), shrinking the pack 23.8 MB -> 1.7 MB. Add guard tests asserting every _BUILTIN_STARTER_SOURCES entry has its file committed and that a seed run lands them all — this catches exactly the listed-but-missing (or committed-but-unlisted) mismatch that left Ode to Joy un-seeded. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
7c873f5cc2
|
feat(library): seed bundled starter content into the library on first run (#743)
Ship a public-domain Für Elise (keys) feedpak as starter content so a fresh install isn't an empty library. server._seed_builtin_starter_content() copies bundled packs into DLC_DIR/starter/ exactly once, guarded by a marker in CONFIG_DIR — unlike the always-reseeding diagnostic seed, a user who deletes the starter song does not get it back. `starter/` is deliberately outside the diagnostics/tutorials library carve-out so the song surfaces as a normal library entry. Extract the shared symlink-safe, mtime-aware copy loop into _copy_builtin_packs() and route both the diagnostic and starter seeds through it (diagnostic behavior unchanged; existing tests green). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Byron Gamatos
|
68e29a8b6e
|
fix(plugins): don't treat transient absence from /api/plugins as uninstall (#741)
* fix(plugins): don't treat transient absence from /api/plugins as uninstall The backend clears its plugin registry at the start of load_plugins() and repopulates it incrementally while HTTP stays up, so every backend restart (desktop: Audio Quality soundfont switch, LAN toggle, update restart) serves a window of partial — even empty — /api/plugins responses. loadPlugins() treated absence from the current response as an uninstall, with three destructive consequences for still-loaded plugins: 1. Their settings-panel and screen DOM were wiped while their _loadedPluginScripts entry survived, so the NEXT refetch failed the DOM-existence check and re-evaluated the plugin's screen.js mid-session. For the desktop audio_engine plugin that re-ran init() against the surviving native audio chain and exactly duplicated every VST/NAM/IR stage (the alpha testers' "chain duplicates after leaving the Audio menu" / blown-out gain reports). 2. _reconcilePluginStyles dropped their stylesheet, leaving them visible but unstyled until they reappeared. 3. The stale-contribution sweep unmounted their UI contributions and unregistered their capability participant with no re-registration path (plugin scripts don't re-run thanks to the loadedScripts guard). Absence is now a non-signal everywhere in loadPlugins: the DOM wipe and style reconcile are scoped to plugins the response actually names, and the absence sweep is removed. Present plugins still fully re-sync via _registerLegacyPluginUiContributions each round; failed plugins are present in the response and still cleaned up; nav is rebuilt from the response so genuinely uninstalled plugins drop out of it, and their (un-unloadable) already-evaluated scripts keep their DOM until reload. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: update idempotence contract to the absence-is-not-uninstall invariant The removed-plugin sweep contract pinned the old behavior this branch deletes; pin the new invariant instead (no absence sweep + respondedIds scoping on the DOM/style reconcilers). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
OmikronApex
|
d2b2a7e9f7
|
fix(tests): re-green the JS suite — 18 stale source-shape tests + 1 real seek-reason violation (#740)
main's JS suite has been red since the recent v3-library and player refactors landed. 17 of 18 failures were test harnesses/regexes that went stale behind real, intentional code changes; one was a genuine contract violation in the code. Code fix: - session-resume seek passed 'resume' as its _audioSeek reason; the documented contract (enforced by song_seek.test.js) requires multi-word kebab-case. Renamed to 'session-resume' — no consumer string-matches specific reasons, so this is rename-safe. Test updates (each pins the CURRENT contract): - highway_colors_facade: inject HWC_PRESETS + applyHighwayStringPreset (new preset feature); lock presets/applyPreset into the surface test - loop_api: stub _updateEditRegionBtn (new edit-region UI hook) - song_close: sandbox gets window.feedBack.playQueue; assert a real close abandons the queue (the new queue-aware behavior) - v3_keep_practicing: the shelf moved from client-side /api/stats/recent dedupe+gating to the server-side practice-suggestions recommender — tests now pin that (fetch, arrangement-aware card click, Promise.all) - v3_songs_tuning: card row variable renamed song → shown (grouped cards) - live_guitar_tone_source: accept literal ’ where ’ drifted in copy - legacy_shim_hits: normalize CRLF before fixed-width region() slicing (Windows-only failure; char windows shrank by one char per line) Suite: 987/987 locally (Windows), previously 968/987 (and 18 red on CI). Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
OmikronApex
|
b6442dda75
|
Merge pull request #737 from got-feedback/feat/playlist-shuffle
feat(v3): playlist shuffle toggle |
||
|
OmikronApex
|
425f72b33f |
feat(v3): playlist shuffle toggle
Crossing-arrows toggle next to Play all / Play album on the playlist detail page. When on, playQueue.start Fisher-Yates-shuffles the queue once at start — on a copy, so the stored playlist order is untouched — swapping per-slot album arrangements in lockstep so each slot keeps its pinned arrangement (#685 contract preserved). Prev-less queue semantics are unchanged: auto-advance simply walks the shuffled order. Preference is global, persisted as localStorage v3PlaylistShuffle. Co-Authored-By: Claude Fable 5 <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> |
||
|
Byron Gamatos
|
97a941c45d
|
fix(tests): join background scan/enrich workers before closing the DB (#735)
Root cause of the flaky pytest segfault (exit 139): the background scan and enrichment daemon threads (_scan_runner/_enrich_runner) use the shared MetadataDB connection, but test fixtures closed that connection in teardown without stopping them. A daemon thread mid-query on a freed SQLite conn is a native use-after-free → SIGSEGV. The app's startup kicks a scan, so almost any app-booting fixture was vulnerable. It only surfaced now because got-feedback/feedBack#728 added a push trigger, so ci/test runs on every push to main. Fix: server.py retains the scan/enrich thread handles and adds _join_background_db_threads(); every test fixture now joins the workers before conn.close(). Verified: the full suite runs to completion (no segfault) where it previously crashed at ~25%. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
ChrisBeWithYou
|
be9e965001
|
v3 library: artist pages — in-your-library view, similar-in-library, links-only web links (#731)
* v3 library: artist pages — in-your-library view, similar-in-library, links-only web
The greenlit artist-pages feature. Every page renders from LOCAL data;
an optional, opt-in external-links strip is the only network surface.
Server:
- artist_enrichment table (mb_artist_id PK, url_rels JSON, genres JSON,
fetched_at) — never purged; one row per matched MusicBrainz artist.
- GET /api/artist/{name}/page — all-local: canonical name (+ raw alias
variants), song/album/mastered counts, album list, similar-in-library
(top artists by shared genre, self excluded, empty is fine), and the
artist's MB id when any matched/manual song carries one. THE DENOMINATOR
LAW: "N mastered" counts songs you OWN (best_accuracy >= 0.9 across the
artist's library songs), never a global discography — a stored score for
a song no longer in the library does not count.
- GET /api/artist/{name}/links — lazy, cached-forever: returns the cached
row, else (external links enabled + network + a known MB artist id) ONE
throttled artist lookup (inc=url-rels+genres+tags), whitelisted into
{official, tour, video, social[], wikipedia}. Every URL passes the same
http(s) scheme gate as art redirects, so a hostile javascript:/data:/file:
can never reach an href. POST .../links/refresh re-fetches. Offline /
no-mbid / links-disabled → empty, no error. Both routes demo-blocked.
- Settings keys artist_pages_enabled (default ON — local-only) and
artist_external_links (default OFF — opt-in per the dev-chat thread).
Frontend (static/v3/songs.js): an in-place sub-render mirroring openAlbum()
with a "← Song Library" back + scroll restore. 2x2 album-art mosaic header
(borrows the playlist-cover renderer), canonical name + "also shown as"
variants + a Matched·MusicBrainz pill when known; stats strip that omits
the mastered segment at zero (invitational, never "0 mastered"); Play all /
Shuffle (playQueue) + Save as smart playlist (collections rule {artist});
album rail → openAlbum; song list via the artist filter + wireCards;
"Similar in your library" chips → open that artist; and the external-links
row under an "On the web · opens your browser" divider, each link
target=_blank rel=noopener noreferrer with its domain shown — rendered only
when external links are on AND links exist. Empty modules hide.
Entry points: card ⋮ "Go to artist", the grid card artist line, and a
"View artist page" link in the Details drawer — all via
window.__fbOpenArtistPage.
Tests: tests/test_artist_page.py — page counts/albums/alias folding, the
denominator law (owned-only, best-across-arrangements), similar ranking +
empty, mb-id only from matched rows, links whitelist + scheme gate (a
javascript: and an ftp:// URL both rejected), disabled-by-default no
network, cache-no-second-fetch, refresh, demo block. 21 pass (35 with
artist_alias). node --check clean; tailwind rebuilt.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN
* fix(v3 artist pages): unfiltered album view + select-mode row guard on artist page
Artist-page album click no longer applies the global library filters: openAlbum
gains an ignoreFilters option that builds the /api/library request scoped only to
artist+album (no drawer/genre/tuning/search params), so the album view and
Play-album match the artist page's full-shelf counts. The normal albums-view
click path is unchanged (ignoreFilters defaults off).
Select-mode row clicks on the artist page now toggle selection instead of playing.
Extracted the grid/tree capture-phase select guard into a shared bindSelectGuard()
and attach it to the persistent artist-page host too. Each host is bound once at
shell build; innerHTML re-renders reuse the same element, so there is no
double-binding.
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
|
64a499975e
|
v3 library: multi-candidate cover picker — select from a populated list (#732)
* v3 library: multi-candidate cover picker — select from list, media-server style
Auto-best already ships; this adds the "pick from a populated list" surface
Christian asked for, as ONE reusable component (song covers now; album/artist
art reuse the same picker later).
Server:
- GET /api/song/{filename}/art/candidates — assembles WITHOUT hoarding: the
Current image + its provenance, Pack art when present, and Cover Art Archive
candidates for the matched release (and any release ids stored on a review
row's candidates). New _caa_release_index() fetches the CAA release INDEX
json (image list + types + thumb sizes) through the existing throttle +
offline gate, cached as caa_index_{id}.json beside the covers (indexes are
stable). Capped at 12; fetched on demand. Demo-blocked (it spends the rate
budget) and offline → instant tiles only, no error.
Frontend: new static/v3/image-picker.js — window.__fbOpenImagePicker({filename,
title}), a body-appended singleton modal (match-review anatomy: overlay, focus
trap, Esc). Current image + provenance badge on the left; a tile grid on the
right whose instant tiles — Current, Pack original, Upload, Paste URL — work
immediately even offline, while CAA candidates load behind ONE /art/candidates
fetch with skeleton tiles + a "the source is rate-limited" caption. The fetch
is tied to an AbortController and cancelled when the modal closes.
Applying a pick reuses EXISTING routes so there's no new write path and the
design's key trick holds: a chosen cover POSTs to …/art/url (the override
lane — never evicted by the art-cache LRU, survives a re-match); "Pack
original" DELETEs the override; Upload POSTs …/art/upload (GIF stays
upload-only + local-only). Silent-on-success; the drawer/card art refreshes
via the existing cache-buster.
Entry points: the Details drawer art click (the old direct file dialog is now
the Upload tile) and a card ⋮ "Change cover…" action.
Tests: tests/test_art_candidates.py (matched row lists index images; review
row pulls in candidate releases; unmatched/offline → instant tiles only;
index cached, no second fetch; demo blocked) over a fake index seam.
30 pass with test_art_layer green (same seams). node --check clean; tailwind
rebuilt for the new file.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN
* fix(v3 cover picker): uiPrompt over window.prompt, visible-only focus trap, gate CAA to matched rows, index-cache lock, abort-on-reopen; +traversal tests
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
|
8c7cde5d5c
|
v3 library: first-hour polish — zero-states, match progress, provenance, alias search (#730)
* v3 library: first-hour polish — zero-states, match progress, provenance, alias search
Six launch-eve fixes for a brand-new user's first hour with a fresh,
being-matched library. Each is small and reuses shipped idioms.
- Invitational repertoire meter: with no practice data yet, the home meter
no longer reads "0 of N mastered" (debt framing) — it shows an empty bar
with "grows as you master songs". A count of 0 read as failure on day one.
- "Start here" starter shelf: growth_edge_suggestions() distinguishes two
empties — attempts exist but all mastered (honest empty shelf) vs nothing
attempted yet (day one) → new starter_suggestions() returns up to 8
approachable songs (90–480s, shortest first) flagged starter:true, and the
client renders a "Start here" shelf instead of a blank home.
- Library-visible match progress: while the background pass runs, a quiet
"Matching your library — X of Y" line sits by the review chip (5s poll,
single guarded interval, cleared the moment the pass stops — no leak,
no toast, silent completion).
- One-time transparency toast: the first time an install is seen matching a
real library, one fbNotify names what's contacted (MusicBrainz / Cover Art
Archive), that results are stored locally, that files aren't changed
without you, and where the switch is. localStorage-gated, wrapped so a
blocked notifier can't break the chip.
- Empty-library dead-end card: a genuinely empty local library (no songs, no
query/filter) shows "Your library is empty" + drop-files hint + Open
Settings, instead of a bare grid under dead dropdowns.
- Alias-aware search: searching a canonical name ("AC/DC") now also finds
songs whose raw tag is a merged variant ("ACDC"), via the artist_alias
table. Probe-guarded so a no-aliases library keeps the exact original
3-term query; pure predicate, keyset-safe.
- Details-drawer provenance line: matched/manual rows show "Matched:
<artist — title> (source) · Fix match" under the Identity fields — the
wrong-match escape hatch at the point of the data, wired to the same
fix-match flow the card menu uses. New read-only GET
/api/enrichment/song/{filename} backs it.
Tests: tests/test_starter_suggestions.py (starter vs normal-shelf behaviour,
length window, attempts-exist path unchanged) + alias-search cases added to
tests/test_artist_alias.py. 34 targeted pass; node --check clean; no new
Tailwind classes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN
* fix(v3 library): stop match-progress poll when leaving the library screen
The 5s enrichment poll (_pollTimer) was cleared on pass completion and on
fetch error, but not when the user navigated away from the library. Leaving
v3-songs mid-pass left the interval pinging /api/enrichment/status in the
background until the pass ended. Subscribe to the existing feedBack
'screen:changed' event: clear the poll when any non-v3-songs screen shows,
and refresh (re-arming if a pass is still running) on returning to v3-songs.
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>
|
||
|
Byron Gamatos
|
f5c9c34291
|
library: scraper options — per-source + per-field auto-apply, review-queue order (R1) (#726)
* library: scraper options — per-source + per-field auto-apply toggles, review-queue order (R1) Grows the Settings→Library "Metadata matching" card into the full scraper-options panel — not everybody needs the same things out of a scraper: - Sources: enrich_src_musicbrainz gates the background matcher (phase 2; identity hashes still stamp, and manual Fix-match/search stays available — same contract as the master toggle). enrich_src_caa gates the Cover Art Archive fetch (phase 3). Rows skipped by an off toggle stay unevaluated, so re-enabling picks them up on the next pass — nothing is permanently forfeited. - Auto-apply fields: enrich_apply_names/year/genres filter what an AUTOMATIC match may canonicalize (_enrich_field_filter, applied on all three automatic paths: cache copy, mbid/isrc exact keys, text auto). MusicBrainz ids always stamp — they're identity, not display; the art fetch and future re-matching need them. A match the user confirms in the review modal applies in full. enrich_apply_art gates the art fetch alongside the CAA source toggle (two axes, one behaviour today — future art sources slot in without re-teaching the panel). - Review queue order: enrich_review_order = missing_first (default, today's behaviour) | artist | recent, read by GET /api/enrichment/review; unknown stored values degrade to the default. - Settings card: Sources / Auto-apply / Review-queue-order groups wired in match-review.js; the master toggle is relabelled "Match songs automatically" so it doesn't read the same as the new MusicBrainz source toggle. No tailwind rebuild needed — every class was already scanned from core source. Tests: tests/test_scraper_options.py (9) — settings validation, MB-source-off stamps-without-matching + re-enable, per-field stripping on auto matches with ids preserved, review-accept full-apply despite toggles, CAA gating on both axes, review-order modes incl. the unknown-value fallback. Full-suite failure set A/B-identical to the base (39 env/pre-existing). Stacked on feat/enrichment-art (#715) — merge that first. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN * library: per-field auto-apply honours "nothing forfeited" (backfill + no partial seeding) The R1 per-FIELD auto-apply toggles settled a `matched` row with the disabled fields stripped, but enrichment_pending() never revisits an unchanged-hash matched row — so re-enabling a field never backfilled it, and enrichment_cache_lookup() (which gates only on mb_recording_id) could seed sibling charts with the stripped blanks. This broke the same "nothing is permanently forfeited" contract the source (mb_on) and art (art_on) toggles already keep. Fix: persist an `apply_mask` marker (sorted blocked apply-keys) on every AUTOMATIC match: - migration: additive `apply_mask TEXT` column (idempotent ALTER). - enrichment_pending(allowed_keys=...): re-queues a `matched` row whose apply_mask names a field that is now re-enabled → backfill on re-enable, converges (a fully-applied row is never re-queued). - enrichment_cache_lookup: only fully-applied donors (apply_mask empty/NULL) may seed siblings; a partial row is skipped and the sibling falls through to its own re-filtered match. - _enrich_apply_mask()/_enrich_blocked_apply_keys() helpers; threaded through _enrich_one → apply_enrichment_match. Review/manual writers leave it NULL (a confirmed pick applies in full). Tests: re-enable-backfills-and-converges; partial row is not a cache donor (fully-applied one is). 13 scraper-options tests pass; 170 enrichment/ settings tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: ChrisBeWithYou <christian.a.cowan@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
ChrisBeWithYou
|
7ca736d525
|
library: cover-art layer — CAA auto-fetch + user overrides, GIF local-only (P9) (#715)
* library: cover-art layer — CAA auto-fetch + user overrides, GIF local-only (R3/P9)
Third slice of the enrichment series (stacked on the matcher): covers.
- Serve chain for GET /api/song/{fn}/art: USER OVERRIDE -> PACK ART ->
COVER ART ARCHIVE cache -> 404. Behaviour change, deliberate: a
user-uploaded cover now OVERRIDES pack art (previously the upload only
filled the no-art gap, which made custom art look broken on any song
that already shipped a cover).
- GIF is allowed as an override and kept VERBATIM (animation intact) —
a local-only bonus. Everything else normalizes to RGB PNG as before.
One override per song (saving either kind removes the other), and
nothing ever writes art INTO a pack file — test-pinned: the pack's
cover.jpg is byte-identical after a GIF upload.
- Art by URL: POST /api/song/{fn}/art/url fetches server-side (http(s)
only, 10 MB cap enforced while streaming) into the same override slot.
DELETE /api/art/{fn}/override drops it — under /api/art because the
greedy DELETE /api/song/{path} catch-all shadows anything beneath it
(the same dodge the chart split/unsplit routes use).
- Cover Art Archive fetch as phase 3 of the enrichment pass: matched
songs that LACK pack art get their release's front cover, throttled +
identified + offline-guarded exactly like the MusicBrainz client
(pytest can never reach the network; a transport error pauses the
pass without burning the row). The cache is keyed by RELEASE MBID —
ten charts of one album cost one fetch — and every outcome writes an
art_state (pack/user/caa/none/error) so a row is evaluated once.
- LRU cap (200 MB) on the CAA side of the cache only; user overrides
are never evicted, and evicted rows reset so a later pass may
re-fetch. Deleting a song removes its override files (CAA files stay
— they may be shared by other charts of the release).
No frontend changes: the grid, the review modal, and the player pick
the new art up through the same route they already use. The
upload/paste-a-link surfaces in the Details drawer land with the
context-menu slice once the drawer PR merges.
13 new tests (tests/test_art_layer.py) + demo-mode routes; full-suite
failure set byte-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: harden cover-art layer — SSRF guard, demo/size caps, override-delete state reset
Follow-up hardening on the R3 cover-art layer:
- remove_song_art_override: reset the enrichment row (set_enrichment_art(fn,
None, None)) when an override is deleted, so a row previously settled as
'user' re-queues and the CAA fallback resumes. Previously a removed override
stranded the row (enrichment_art_pending only re-queues art_state IS NULL),
leaving the song with no art at all.
- Base64 art upload: block it in demo mode (was open — a write/disk-fill vector,
worse now that GIFs are stored verbatim), validate the filename resolves to a
real song (mirrors the url route), and cap the decoded payload at 10 MB.
- Art-by-URL: reject hosts that resolve to loopback/private/link-local/reserved/
multicast/unspecified addresses (SSRF, e.g. cloud metadata) and stop following
redirects (allow_redirects=False) so a redirect can't smuggle the request to an
internal target. Fails closed on unresolvable/unparseable hosts.
- _caa_http_get: stream with a per-file 10 MB cap (bounds any one response
independently of the aggregate LRU); guard release_id against a conservative
token before interpolating it into a cache-file path (no separators/dots).
Tests: delete-override→CAA-fallback, upload unknown-song/oversize rejection,
SSRF internal-host guard, and a demo-mode block assertion for art/upload.
Note: art_state='error' rows are intentionally not auto-retried — there is no
per-row attempt counter on the art side, so an unbounded retry could storm CAA
for permanently-bad rows; a bounded retry would need extra state, left out here.
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
|
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
|
55060c4f67
|
v3 library: artist sort orders titles within an artist (tree-view feel) (#720)
* v3 library: artist sort orders titles within an artist (tree-view feel) Tester report: "the list is set up by artist, but the cards are alphabetical(-ish random)". Real: the tree orders artist -> album -> title, while the grid's artist sort ordered within an artist by RAW FILENAME — community-pack filename noise, so an artist's cards looked shuffled. - artist / artist-desc gain a title secondary (direction baked per entry so the legacy `dir=desc` append can't land on the title term; titles stay A->Z under Z->A artists). - The two-term (value, filename) keyset cursor can't seek a three-term order, so artist sorts leave _KEYSET_SORTS and page by OFFSET — measured trivial at real library sizes; title/recent keep their keyset. Restore via a composite sort-key column if 50k-song libraries ever hurt. - The tree view says "List view groups by artist — the selected sort applies to the card grid" when a non-artist sort is active, instead of silently ignoring the picker. - Keyset proof-tests repinned to the title sort (same property, a sort that still keysets); 2 new tests pin the title-within-artist order and the OFFSET pagination's no-skip/no-dupe across pages. Full-suite failure set identical to the same-main baseline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN * v3 library: honor legacy sort=artist&dir=desc (fold dir into effective sort) Codex/review follow-up to the title-within-artist change: the new artist ORDER BY bakes in `ASC` (for the title secondary), so the global `dir=desc` append is suppressed and `sort=artist&dir=desc` silently returned A->Z instead of Z->A — a regression on the legacy /api/library dir contract. Fold `dir=desc` into the canonical sort key BEFORE the sort_map lookup via the existing _effective_keyset_sort helper (same fold the cursor side already does), so the ORDER BY is built from the effective sort. Only artist/title fold (they have `-desc` twins); title/recent/tuning/year/mastery are unaffected — verified by the keyset/filter suites. New test pins that legacy `sort=artist&dir=desc` matches the explicit `artist-desc` ordering (Z->A artists, A->Z titles within each). 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
|
28b0319e27
|
play-queue: peekNext() — expose the following track for queue-aware UIs (#719)
A results screen that offers "Up next: <song> — starting in 10s" needs to
know WHAT follows without reaching into queue internals. peekNext() returns
{filename, index, total} for the next track (null when nothing follows),
pure — peeking never plays or mutates.
First consumer: the note_detect results card's queue-advance strip (the
"Playlist Play All has no way to progress" tester issue).
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
K. O. A.
|
4b6cbe8b11
|
Fix 3D drum/keys highways not resizing on fullscreen under splitscreen (#723)
The guitar/bass highway_3d renderer self-detects panel-canvas size changes in its draw() loop and re-runs applySize() every frame, because the splitscreen host overrides hw.resize and never calls renderer.resize(). The drum and keys highways lacked that fallback — they only re-framed when the host explicitly called resize(w, h) — so their panels stayed framed for the pre-fullscreen size while the guitar/bass panels adapted. Symptom: a too-small, off-center highway in the drum/keys panels after maximizing a split-screen session. Port highway_3d's per-frame drift check into both draw() loops: re-apply on backing-store change (canvas.width/height) AND on CSS-box drift (clientWidth/clientHeight vs the last applied logical size, throttled to every 10th frame). Track _lastHwW/_lastHwH + _appliedW/_appliedH per instance and reset them in destroy() so a reused instance re-frames on the next song. plugins/drum_highway_3d -> 0.3.1, plugins/keys_highway_3d -> 0.1.1. Tests: tests/js/drum_keys_highway_3d_resize_reframe.test.js. Signed-off-by: Kris Anderson <topkoa@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |