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>

tests/js/working_tuning.test.js

@@ -16,8 +16,8 @@ const { createWindow, ROOT } = require('./capabilities_test_harness');
 const CAPABILITIES_JS = path.join(ROOT, 'static', 'capabilities.js');
 const WORKING_TUNING_JS = path.join(ROOT, 'static', 'capabilities', 'working-tuning.js');
 
-// A /api/tunings-shaped fixture (frequencies at 440), enough to resolve names to offsets.
-const TUNINGS = {
+// Tuning frequency fixture at 440 Hz, enough to resolve names to offsets.
+const TUNING_TABLE = {
     'guitar-6': {
         Standard: [82.41, 110.00, 146.83, 196.00, 246.94, 329.63],
         'Drop D': [73.42, 110.00, 146.83, 196.00, 246.94, 329.63],
@@ -26,6 +26,7 @@ const TUNINGS = {
         Standard: [30.87, 41.20, 55.00, 73.42, 98.00],
     },
 };
+const API_TUNINGS = { referencePitch: 440, tunings: TUNING_TABLE };
 
 function deferred() {
     let resolve;
@@ -159,7 +160,7 @@ test('bare-instrument writes target the current selection, not a hard-coded defa
 test('seed resolves a NAMED tuning to offsets via /api/tunings', async () => {
     const { wt, changes } = loadWorkingTuning({
         '/api/settings': { instrument: 'guitar', string_count: 6, tuning: 'Drop D', reference_pitch: 440 },
-        '/api/tunings': TUNINGS,
+        '/api/tunings': API_TUNINGS,
     });
     await flush();
     const s = wt.get('guitar-6');
@@ -183,7 +184,7 @@ test('boot race: an explicit set() before settings resolve is not clobbered by t
     const settings = deferred();
     const { wt } = loadWorkingTuning({
         '/api/settings': settings.promise,           // held open
-        '/api/tunings': TUNINGS,
+        '/api/tunings': API_TUNINGS,
     });
     // A consumer writes before the seed lands.
     wt.set({ offsets: [-5, -5, -5, -5, -5, -5] }, { instrument: 'guitar-6' });

tests/test_settings_api.py

@@ -753,29 +753,142 @@ def test_defaults_include_gameplay_keys(client, tmp_path):
     assert data["fail_behavior"] == "continue"
 
 
+
+def test_get_settings_exposes_default_instrument_profiles(client, tmp_path):
+    data = client.get("/api/settings").json()
+    assert data["active_instrument_profile"] == "guitar-lead"
+    assert set(data["instrument_profiles"]) == {"guitar-lead", "guitar-rhythm", "bass"}
+    assert data["instrument"] == "guitar"
+    assert data["string_count"] == 6
+    assert data["tuning"] == "Standard"
+    assert data["pathway"] == "songs"
+
+
+def test_post_flat_instrument_updates_active_profile(client, tmp_path):
+    r = client.post("/api/settings", json={"instrument": "bass", "pathway": "practice"})
+    assert r.status_code == 200
+    cfg = _read_cfg(tmp_path)
+    assert cfg["active_instrument_profile"] == "bass"
+    assert cfg["instrument"] == "bass"
+    assert cfg["string_count"] == 4
+    assert cfg["tuning"] == "Standard"
+    assert cfg["pathway"] == "practice"
+    assert cfg["instrument_profiles"]["bass"]["string_count"] == 4
+    assert cfg["instrument_profiles"]["bass"]["pathway"] == "practice"
+
+
+def test_post_instrument_profiles_mirrors_active_profile(client, tmp_path):
+    r = client.post("/api/settings", json={
+        "active_instrument_profile": "guitar-rhythm",
+        "instrument_profiles": {
+            "guitar-rhythm": {
+                "string_count": 7,
+                "tuning": "Drop A",
+                "reference_pitch": 432,
+                "pathway": "studio",
+            },
+            "bass": {
+                "string_count": 6,
+                "tuning": "C Standard",
+            },
+        },
+    })
+    assert r.status_code == 200
+    cfg = _read_cfg(tmp_path)
+    assert cfg["active_instrument_profile"] == "guitar-rhythm"
+    assert cfg["instrument"] == "guitar"
+    assert cfg["string_count"] == 7
+    assert cfg["tuning"] == "Drop A"
+    assert cfg["reference_pitch"] == 432
+    assert cfg["pathway"] == "studio"
+
+
+def test_post_pathway_rejects_bad_value(client, tmp_path):
+    (tmp_path / "config.json").write_text(json.dumps({"pathway": "songs"}))
+    r = client.post("/api/settings", json={"pathway": "invalid"})
+    assert "error" in r.json()
+    assert _read_cfg(tmp_path)["pathway"] == "songs"
+
+
+def test_post_instrument_profiles_rejects_bad_custom_string_count(client, tmp_path):
+    r = client.post("/api/settings", json={
+        "instrument_profiles": {
+            "bass": {"string_count": 6, "tuning": [0, 0, 0, 0]},
+        },
+    })
+    assert "error" in r.json()
+
 # ── /api/settings/reset ─────────────────────────────────────────────────────
 
 def test_reset_clears_requested_keys(client, tmp_path):
     (tmp_path / "config.json").write_text(json.dumps({
         "master_difficulty": 40,
         "countdown_before_song": True,
+        "pathway": "studio",
         "default_arrangement": "Lead",
         "demucs_server_url": "http://demucs.example:9000",
     }))
     r = client.post("/api/settings/reset",
-                    json={"keys": ["master_difficulty", "countdown_before_song"]})
+                    json={"keys": ["master_difficulty", "countdown_before_song", "pathway"]})
     assert r.status_code == 200
     body = r.json()
-    assert set(body["reset"]) == {"master_difficulty", "countdown_before_song"}
+    assert set(body["reset"]) == {"master_difficulty", "countdown_before_song", "pathway"}
     cfg = _read_cfg(tmp_path)
     # Reset removes the key so GET falls back to the default.
     assert "master_difficulty" not in cfg
     assert "countdown_before_song" not in cfg
+    assert "pathway" not in cfg
     # Unlisted keys are untouched.
     assert cfg["default_arrangement"] == "Lead"
     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
+    # legacy flat defaults (guitar-lead).
+    r = client.post("/api/settings", json={"active_instrument_profile": "bass"})
+    assert r.status_code == 200 and "error" not in r.json()
+    got = client.get("/api/settings").json()
+    assert got["active_instrument_profile"] == "bass"
+    assert got["instrument"] == "bass"
+
+
+def test_reset_pathway_reaches_into_instrument_profiles(client, tmp_path):
+    # pathway is mirrored into every instrument profile, so a Gameplay reset
+    # that only deleted the flat key would leave GET re-deriving the old value
+    # from the profile. The reset must reach into the persisted profiles too.
+    client.post("/api/settings", json={"pathway": "studio"})
+    assert client.get("/api/settings").json()["pathway"] == "studio"
+    profiles = _read_cfg(tmp_path)["instrument_profiles"]
+    assert any(p["pathway"] == "studio" for p in profiles.values())
+
+    r = client.post("/api/settings/reset", json={"keys": ["pathway"]})
+    assert r.status_code == 200
+    assert "pathway" in r.json()["reset"]
+    # GET re-derives from the profile — which must now be back to the default.
+    assert client.get("/api/settings").json()["pathway"] == "songs"
+    for prof in _read_cfg(tmp_path)["instrument_profiles"].values():
+        assert prof["pathway"] == "songs"
+
+
 def test_reset_ignores_unknown_keys(client, tmp_path):
     (tmp_path / "config.json").write_text(json.dumps({"master_difficulty": 40}))
     # Unknown / non-resettable keys are silently ignored, not an error, and

tests/test_settings_instrument.py

@@ -31,12 +31,14 @@ def _cfg(tmp_path):
 def test_instrument_fields_persist(env):
     srv, tmp = env
     c = TestClient(srv.app)
+    # "Drop A" is the 5-string bass drop tuning (its low string is B, not E, so
+    # "Drop D" is a 4-string tuning — now correctly rejected per-profile).
     r = c.post("/api/settings", json={"instrument": "bass", "string_count": 5,
-                                      "tuning": "Drop D", "reference_pitch": 442})
+                                      "tuning": "Drop A", "reference_pitch": 442})
     assert r.status_code == 200
     cfg = _cfg(tmp)
     assert cfg["instrument"] == "bass" and cfg["string_count"] == 5
-    assert cfg["tuning"] == "Drop D" and cfg["reference_pitch"] == 442.0
+    assert cfg["tuning"] == "Drop A" and cfg["reference_pitch"] == 442.0
     # Reflected back through GET.
     got = c.get("/api/settings").json()
     assert got["instrument"] == "bass" and got["reference_pitch"] == 442.0

tests/test_tunings.py

@@ -2,7 +2,31 @@
 
 import pytest
 
-from tunings import tuning_name
+from tunings import (
+    DEFAULT_TUNINGS,
+    TUNING_PRESET_MIDIS,
+    _valid_tuning_for_key,
+    apply_flat_instrument_patch_to_profiles,
+    open_midis_to_freqs,
+    settings_with_instrument_profiles,
+    tuning_midis_from_offsets,
+    tuning_name,
+    tuning_offsets_from_midis,
+    tuning_preset_offsets,
+)
+
+
+def test_valid_tuning_for_key_builtin_and_provider_names():
+    # A built-in valid for the key is accepted; a built-in valid only for a
+    # DIFFERENT key (misapplied, e.g. "Drop D" on a 5-string bass) is rejected.
+    assert _valid_tuning_for_key("bass-5", "Drop A") == "Drop A"
+    assert _valid_tuning_for_key("bass-5", "Drop D") is None
+    assert _valid_tuning_for_key("guitar-6", "Standard") == "Standard"
+    # A name unknown to every built-in table is a provider/custom tuning (tuner
+    # plugin, /api/tunings) the pure layer can't resolve — accept it so settings
+    # round-trip rather than normalizing it away to Standard.
+    assert _valid_tuning_for_key("bass-5", "My Custom DADGAD") == "My Custom DADGAD"
+    assert _valid_tuning_for_key("guitar-6", "x" * 65) is None   # length cap kept
 
 
 # ── Standard tunings (all six strings share the same offset) ─────────────────
@@ -132,3 +156,96 @@ def test_drop_pattern_takes_precedence_over_named_dict():
     # auto-generator fires first and produces the same string. The named dict entry
     # is effectively dead code for this case — this test documents the behavior.
     assert tuning_name([-2, 0, 0, 0, 0, 0]) == "Drop D"
+
+
+# ── Host tuning profile catalogue -------------------------------------------
+
+def test_default_tunings_include_extended_host_profiles():
+    assert "bass-6" in DEFAULT_TUNINGS
+    assert "C Standard" in DEFAULT_TUNINGS["guitar-6"]
+    assert "C# Standard" in DEFAULT_TUNINGS["guitar-6"]
+    assert "Drop Ab" in DEFAULT_TUNINGS["guitar-6"]
+    assert "BEAD" in DEFAULT_TUNINGS["bass-4"]
+    assert "High C" in DEFAULT_TUNINGS["bass-5"]
+    assert "Drop A + Drop E" in DEFAULT_TUNINGS["guitar-8"]
+
+
+def test_default_tuning_frequencies_are_derived_from_midis():
+    assert DEFAULT_TUNINGS["guitar-6"]["Standard"] == open_midis_to_freqs([40, 45, 50, 55, 59, 64])
+    assert DEFAULT_TUNINGS["bass-6"]["Standard"] == open_midis_to_freqs([23, 28, 33, 38, 43, 48])
+
+
+def test_tuning_offsets_from_named_presets():
+    assert tuning_preset_offsets("guitar-6", "Drop D") == [-2, 0, 0, 0, 0, 0]
+    assert tuning_preset_offsets("guitar-6", "C Standard") == [-4, -4, -4, -4, -4, -4]
+    assert tuning_preset_offsets("bass-4", "BEAD") == [-5, -5, -5, -5]
+    assert tuning_preset_offsets("bass-5", "High C") == [5, 5, 5, 5, 5]
+
+
+def test_tuning_midis_round_trip_offsets():
+    offsets = [-2, 0, 0, 0, 0, 0]
+    midis = tuning_midis_from_offsets("guitar-6", offsets)
+    assert midis == TUNING_PRESET_MIDIS["guitar-6"]["Drop D"]
+    assert tuning_offsets_from_midis("guitar-6", midis) == offsets
+
+
+def test_tuning_conversion_rejects_wrong_string_count():
+    assert tuning_offsets_from_midis("guitar-6", [40, 45, 50, 55]) is None
+    assert tuning_midis_from_offsets("bass-4", [0, 0, 0, 0, 0]) is None
+
+def test_settings_profiles_default_to_lead_rhythm_and_bass():
+    settings = settings_with_instrument_profiles({})
+    assert settings["active_instrument_profile"] == "guitar-lead"
+    assert set(settings["instrument_profiles"]) == {"guitar-lead", "guitar-rhythm", "bass"}
+    assert settings["instrument"] == "guitar"
+    assert settings["string_count"] == 6
+    assert settings["tuning"] == "Standard"
+    assert settings["pathway"] == "songs"
+    assert settings["instrument_profiles"]["guitar-lead"]["pathway"] == "songs"
+
+
+def test_settings_profiles_migrate_legacy_flat_bass_selection():
+    settings = settings_with_instrument_profiles({
+        "instrument": "bass",
+        "string_count": 6,
+        "tuning": "C Standard",
+        "reference_pitch": 432,
+        "pathway": "practice",
+    })
+    assert settings["active_instrument_profile"] == "bass"
+    assert settings["instrument_profiles"]["bass"]["string_count"] == 6
+    assert settings["instrument_profiles"]["bass"]["tuning"] == "C Standard"
+    assert settings["reference_pitch"] == 432
+    assert settings["pathway"] == "practice"
+    assert settings["instrument_profiles"]["bass"]["pathway"] == "practice"
+
+
+def test_flat_patch_updates_active_profile_and_mirrors_legacy_keys():
+    settings = settings_with_instrument_profiles({})
+    patched = apply_flat_instrument_patch_to_profiles(settings, {"tuning": "Drop D"})
+    assert patched["tuning"] == "Drop D"
+    assert patched["instrument_profiles"]["guitar-lead"]["tuning"] == "Drop D"
+
+
+def test_flat_pathway_patch_updates_active_profile_and_mirrors_legacy_key():
+    settings = settings_with_instrument_profiles({})
+    patched = apply_flat_instrument_patch_to_profiles(settings, {"pathway": "studio"})
+    assert patched["pathway"] == "studio"
+    assert patched["instrument_profiles"]["guitar-lead"]["pathway"] == "studio"
+
+
+def test_flat_instrument_patch_defaults_to_target_string_count():
+    settings = settings_with_instrument_profiles({"instrument": "guitar", "string_count": 6, "tuning": "Drop D"})
+    patched = apply_flat_instrument_patch_to_profiles(settings, {"instrument": "bass"})
+    assert patched["instrument"] == "bass"
+    assert patched["string_count"] == 4
+    assert patched["tuning"] == "Standard"
+    assert patched["active_instrument_profile"] == "bass"
+    assert patched["instrument_profiles"]["bass"]["string_count"] == 4
+
+
+def test_flat_string_count_patch_resets_incompatible_named_tuning():
+    settings = settings_with_instrument_profiles({"instrument": "guitar", "string_count": 6, "tuning": "DADGAD"})
+    patched = apply_flat_instrument_patch_to_profiles(settings, {"string_count": 7})
+    assert patched["string_count"] == 7
+    assert patched["tuning"] == "Standard"