diff --git a/server.py b/server.py index 7994650..3a17593 100644 --- a/server.py +++ b/server.py @@ -45,7 +45,8 @@ from audio import find_wem_files, convert_wem from tunings import ( DEFAULT_REFERENCE_PITCH, DEFAULT_TUNINGS, PROFILE_IDS, PROFILE_PATHWAYS, apply_flat_instrument_patch_to_profiles, apply_reference_pitch, - normalize_instrument_profiles, settings_with_instrument_profiles, tuning_name, + normalize_instrument_profile, normalize_instrument_profiles, + settings_with_instrument_profiles, tuning_name, ) import sloppak as sloppak_mod import drums as drums_mod @@ -9331,13 +9332,25 @@ def save_settings(data: dict): return {"error": "pathway must be one of songs, practice, learn, studio"} updates["pathway"] = raw + _profile_patch = None if "instrument_profiles" in data: raw = data["instrument_profiles"] if raw is not None: - profiles, error = normalize_instrument_profiles(raw) - if error: - return {"error": error} - updates["instrument_profiles"] = profiles + if not isinstance(raw, dict): + return {"error": "instrument_profiles must be an object"} + # Validate each PROVIDED profile individually and keep the patch + # PARTIAL — /api/settings is a partial-merge endpoint, so updating one + # profile must NOT reset the others to defaults. Merged over the + # persisted profiles inside the lock below (not via the wholesale + # `updates` merge, which would clobber the unspecified ones). + _profile_patch = {} + for _pid, _praw in raw.items(): + if _pid not in PROFILE_IDS: + return {"error": f"unknown instrument profile: {_pid}"} + _prof, _perr = normalize_instrument_profile(_pid, _praw) + if _perr: + return {"error": _perr} + _profile_patch[_pid] = _prof if "active_instrument_profile" in data: raw = data["active_instrument_profile"] if raw is not None: @@ -9360,6 +9373,15 @@ def save_settings(data: dict): if cfg is None: cfg = _default_settings() cfg.update(updates) + if _profile_patch is not None: + # Merge the validated partial over the persisted profiles so a + # single-profile update leaves the others intact (a fresh config + # falls back to the built-in defaults for the unspecified ones). + _existing, _ = normalize_instrument_profiles(cfg.get("instrument_profiles")) + if _existing is None: + _existing = {} + _existing.update(_profile_patch) + cfg["instrument_profiles"] = _existing # Only canonicalize/persist the instrument profiles when this save # actually touches them (or the config already carries them). GET always # virtualizes profiles via settings_with_instrument_profiles, so a save diff --git a/static/v3/badges.js b/static/v3/badges.js index 20e94a1..c67b3e1 100644 --- a/static/v3/badges.js +++ b/static/v3/badges.js @@ -500,8 +500,17 @@ renderInstrument(); keepOpen(); })); menu.querySelectorAll('[data-pill="strings"]').forEach((b) => b.addEventListener('click', async () => { - await saveSettings({ string_count: Number(b.getAttribute('data-val')) }); - setWorkingInstrument(settings.instrument, settings.string_count); + const newSc = Number(b.getAttribute('data-val')); + // Clamp the tuning to one valid for the new string count and post it + // alongside string_count — otherwise the backend silently resets a + // now-invalid tuning to Standard while this UI keeps showing the old + // one (settings/tuner desync). Mirrors the instrument-switch clamp. + const tunings = _tuningsForInstrument(settings.instrument, newSc); + await saveSettings({ + string_count: newSc, + tuning: tunings.includes(settings.tuning) ? settings.tuning : (tunings[0] || settings.tuning), + }); + setWorkingInstrument(settings.instrument, newSc); renderInstrument(); keepOpen(); })); menu.querySelector('[data-inst-tuning]').addEventListener('change', (e) => saveSettings({ tuning: e.target.value })); diff --git a/tests/test_settings_api.py b/tests/test_settings_api.py index 2428929..0574ade 100644 --- a/tests/test_settings_api.py +++ b/tests/test_settings_api.py @@ -834,6 +834,23 @@ def test_reset_clears_requested_keys(client, tmp_path): assert cfg["demucs_server_url"] == "http://demucs.example:9000" +def test_partial_instrument_profiles_update_preserves_others(client, tmp_path): + # /api/settings is a partial-merge endpoint, so a POST that carries only ONE + # instrument profile must not reset the others to defaults. + gl = client.get("/api/settings").json()["instrument_profiles"]["guitar-lead"] + gl = dict(gl); gl["tuning"] = "Drop D" + client.post("/api/settings", json={"instrument_profiles": {"guitar-lead": gl}}) + assert (client.get("/api/settings").json()["instrument_profiles"] + ["guitar-lead"]["tuning"] == "Drop D") + # Now update ONLY bass (Drop D is valid for a 4-string bass). + bass = client.get("/api/settings").json()["instrument_profiles"]["bass"] + bass = dict(bass); bass["tuning"] = "Drop D" + client.post("/api/settings", json={"instrument_profiles": {"bass": bass}}) + out = client.get("/api/settings").json()["instrument_profiles"] + assert out["guitar-lead"]["tuning"] == "Drop D", "the untouched profile survived" + assert out["bass"]["tuning"] == "Drop D" + + def test_active_profile_switch_on_fresh_config(client, tmp_path): # A fresh config has no instrument_profiles; an explicit active-profile # switch must be honored, not overwritten by the profile inferred from the