fix(song): make bass detection instrument-type-aware, not name-only (#1019)

Editor now authors an arrangement's instrument as first-class data (a manifest
'type' field). Core dropped it: the sloppak loader never read 'type', and
'is this a bass?' was defined three different ways across call sites (name-only
in note_pitch_midi and the highway scale-degree path; path_bass+name in bass
selection; name-only in arrangement_string_count). So an authored type=bass
chart not named 'bass' got 6-string lane counts and guitar open-string MIDI.

- Add optional Arrangement.type; sloppak load_song lifts the manifest type onto it
- Add arrangement_is_bass(arr) = type=='bass' OR path_bass OR 'bass' in name
  (None/whitespace safe), and route string count, note_pitch_midi, the highway
  scale-degree base, and bass-player selection through it
- Back-compat: no bass signal -> unchanged 6-string / guitar behavior

Companion to editor #335 (first-class instrument type). Scale degrees are
display-only and never feed a grader.

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

lib/routers/ws_highway.py

@@ -26,6 +26,7 @@ from fastapi import APIRouter, WebSocket, WebSocketDisconnect
 
 from song import (
     anchor_to_wire,
+    arrangement_is_bass,
     arrangement_string_count,
     base_open_string_midis,
     chord_template_to_wire,
@@ -273,9 +274,8 @@ async def highway_ws(websocket: WebSocket, filename: str, arrangement: int = -1,
                 bass_idxs = [
                     i
                     for i, a in enumerate(song.arrangements)
-                    if getattr(a, "path_bass", False)
+                    if arrangement_is_bass(a)
                     or (smart_names[i] or "").lower().startswith("bass")
-                    or "bass" in (getattr(a, "name", "") or "").lower()
                 ]
                 if bass_idxs:
                     # Among the bass parts: (1) honor the saved default-arrangement
@@ -1014,7 +1014,7 @@ async def highway_ws(websocket: WebSocket, filename: str, arrangement: int = -1,
         # base[string] + offset + capo + fret (matches the tuner / open-string
         # labels). arrangement_string_count is O(notes), so compute once here.
         _base = base_open_string_midis(
-            arrangement_string_count(arr), "bass" in (arr.name or "").lower())
+            arrangement_string_count(arr), arrangement_is_bass(arr))
         _capo = int(getattr(arr, "capo", 0) or 0)
 
         def _fill_scale_degree(wire: dict, n, t: float) -> None:

lib/sloppak.py

@@ -935,6 +935,11 @@ def load_song(
         # the arrangement JSON (name, tuning, capo, centOffset).
         if entry.get("name"):
             arr.name = str(entry["name"])
+        # Editor-authored instrument type (feedpak-spec §5.2 / editor PR #335).
+        # Drives arrangement_string_count's bass fallback so a bass authored on
+        # an arrangement whose NAME doesn't say "bass" still reports 4 strings.
+        if entry.get("type"):
+            arr.type = str(entry["type"]).strip().lower()
         if "tuning" in entry:
             arr.tuning = list(entry["tuning"])
         if "capo" in entry:

lib/song.py

@@ -182,6 +182,12 @@ class Arrangement:
     # `base`/`changes` drive the highway tone-change markers; `definitions`
     # feed the Tones plugin gear panel.
     tones: dict | None = None
+    # Editor-authored instrument type (feedpak-spec §5.2 / editor PR #335):
+    # "bass" | "guitar" | "piano" | "keys" | "drums" | "". First-class DATA that
+    # lets a user author an instrument on an arrangement whose NAME doesn't say
+    # so. Lifted from the sloppak manifest entry by sloppak.load_song(); "" for
+    # archive/loose sources, which instead carry the path_* flags below.
+    type: str = ""
     # arrangement XML <arrangementProperties> flags for smart naming (feedBack feat/arrangement).
     # Populated from the XML; default False/0 for sloppak / GP-imported sources.
     path_lead: bool = False
@@ -503,8 +509,8 @@ def note_pitch_midi(arr: "Arrangement", note: "Note") -> int | None:
     O(notes) via ``arrangement_string_count`` — for a whole arrangement, hoist
     the base with :func:`base_open_string_midis` and call :func:`pitch_from_base`
     per note instead."""
-    is_bass = "bass" in (arr.name or "").lower()
+    base = base_open_string_midis(arrangement_string_count(arr),
-    base = base_open_string_midis(arrangement_string_count(arr), is_bass)
+                                  arrangement_is_bass(arr))
     return pitch_from_base(base, int(getattr(arr, "capo", 0) or 0),
                            arr.tuning or [], note.string, note.fret)
 
@@ -633,6 +639,23 @@ def phrase_from_wire(d: dict) -> Phrase:
     )
 
 
+def arrangement_is_bass(arr: Arrangement) -> bool:
+    """Whether ``arr`` is a bass, most-authoritative signal first: an
+    editor-authored ``type == "bass"`` (feedpak-spec §5.2 / editor PR #335,
+    lifted onto sloppaks by :func:`sloppak.load_song`), then the archive
+    ``path_bass`` <arrangementProperties> flag, then the legacy "bass"
+    case-insensitive substring in the name. Single source of the bass decision
+    so string-count derivation and the open-string pitch base (via
+    :func:`base_open_string_midis`) agree — a bass authored on an arrangement
+    whose NAME doesn't say "bass" must get both 4 lanes AND the bass MIDI base,
+    not 4 lanes on a guitar octave."""
+    return (
+        (arr.type or "").strip().lower() == "bass"
+        or bool(arr.path_bass)
+        or "bass" in (arr.name or "").lower()
+    )
+
+
 def arrangement_string_count(arr: Arrangement) -> int:
     """Derive the active arrangement's string count.
 
@@ -650,10 +673,17 @@ def arrangement_string_count(arr: Arrangement) -> int:
        But this is a LOWER BOUND only — a 6-string lead chart that
        never plays string 5 reports 5, undercounting by 1.
 
-    2. **Name-based fallback.** Arrangements named "Bass" (case-
+    2. **Instrument-type fallback.** An arrangement whose authoritative
-       insensitive substring match) default to 4; everything else
+       instrument signal says bass defaults to 4; everything else
-       defaults to 6. This catches the partial-string-usage case
+       defaults to 6. This catches the partial-string-usage case where
-       where notes don't span all the instrument's strings.
+       notes don't span all the instrument's strings. The bass signal is
+       any of: an editor-authored ``type == "bass"`` (feedpak-spec §5.2 /
+       editor PR #335 — lifted onto sloppaks by ``sloppak.load_song``),
+       the ``path_bass`` <arrangementProperties> flag (archive/DLC
+       sources), or the legacy "bass" case-insensitive substring in the
+       name. Trusting ``type``/``path_bass`` closes the gap where a user
+       authors a bass instrument on an arrangement whose NAME doesn't say
+       "bass" (the editor lays out 4 lanes; core must agree).
 
     A third signal — ``len(arr.tuning)`` when it isn't the arrangement XML
     padded value of 6 — folds in for sloppak / GP-imported sources
@@ -684,6 +714,10 @@ def arrangement_string_count(arr: Arrangement) -> int:
       max(0, 4, 0) = 4
     * Empty arrangement named "Lead" (tuning len 6) →
       max(0, 6, 0) = 6
+    * Editor-authored bass named "Low End" (type "bass", tuning len 6,
+      notes 0..3) → name_based=4 → max(4, 4, 0) = 4
+    * DLC bass named "Low End" (pathBass flag set, tuning len 6, notes
+      0..3) → name_based=4 → max(4, 4, 0) = 4
 
     Topkoa's issue argues plugins shouldn't do arrangement-name
     matching; server-side fallback IS the right place for it
@@ -699,7 +733,9 @@ def arrangement_string_count(arr: Arrangement) -> int:
             if cn.string > max_s:
                 max_s = cn.string
     notes_count = max_s + 1 if max_s >= 0 else 0
-    name_based = 4 if "bass" in arr.name.lower() else 6
+    # Bass signal (type / pathBass / name substring) — see arrangement_is_bass.
+    # Any one being bass pulls the fallback to 4.
+    name_based = 4 if arrangement_is_bass(arr) else 6
     # Tuning-length signal — only trustworthy when NOT the arrangement XML
     # padded value of 6. Length 4/5 indicates explicit bass / 5-string
     # bass; length 7/8 indicates an extended-range guitar from GP.

tests/test_song.py

@@ -329,6 +329,31 @@ def test_note_pitch_midi_bass_uses_bass_base():
     assert note_pitch_midi(bass, Note(time=0, string=0, fret=0)) == 28
 
 
+def test_note_pitch_midi_authored_bass_type_uses_bass_base():
+    """Editor PR #335: a bass authored via `type` on an arrangement whose NAME
+    doesn't say "bass". arrangement_string_count now returns 4 for it, so the
+    open-string base MUST also be the bass base (low E1 = 28), not the guitar
+    octave (40). Pre-fix note_pitch_midi keyed is_bass off the name only, so
+    this returned 40 (4 lanes on a guitar octave — the exact inconsistency)."""
+    bass = Arrangement(
+        name="Low End", type="bass",
+        tuning=[0, 0, 0, 0],
+        notes=[Note(time=float(i), string=i, fret=0) for i in range(4)],
+    )
+    assert note_pitch_midi(bass, Note(time=0, string=0, fret=0)) == 28
+
+
+def test_note_pitch_midi_path_bass_flag_uses_bass_base():
+    """Same guarantee via the archive <arrangementProperties> pathBass flag on a
+    non-"bass"-named arrangement: bass base (28), not guitar (40)."""
+    bass = Arrangement(
+        name="Low End", path_bass=True,
+        tuning=[0, 0, 0, 0],
+        notes=[Note(time=float(i), string=i, fret=0) for i in range(4)],
+    )
+    assert note_pitch_midi(bass, Note(time=0, string=0, fret=0)) == 28
+
+
 def test_note_pitch_midi_out_of_range_string_is_none():
     arr = Arrangement(name="Lead", tuning=[0, 0, 0, 0, 0, 0])
     assert note_pitch_midi(arr, Note(time=0, string=9, fret=0)) is None
@@ -1153,6 +1178,60 @@ def test_string_count_ignores_rs_padded_tuning_for_bass():
     assert arrangement_string_count(arr) == 4
 
 
+def test_string_count_4_for_path_bass_flag_without_bass_in_name():
+    # Archive/DLC bass whose manifest ArrangementName isn't "Bass" but whose
+    # <arrangementProperties> pathBass flag is set. Notes on 0..3, tuning
+    # padded to the arrangement-XML length of 6. Pre-fix, name_based forced
+    # 6 (name has no "bass"), so this returned 6 despite the authoritative
+    # instrument flag saying bass.
+    arr = Arrangement(
+        name="Low End",
+        path_bass=True,
+        tuning=[0, 0, 0, 0, 0, 0],
+        notes=[Note(time=float(i), string=i, fret=0) for i in range(4)],
+    )
+    assert arrangement_string_count(arr) == 4
+
+
+def test_string_count_4_for_authored_bass_type_without_bass_in_name():
+    # Editor PR #335: an instrument `type` authored as bass on an arrangement
+    # whose NAME does not contain "bass" (the sloppak loader lifts the manifest
+    # `type` onto Arrangement.type). Notes on 0..3, tuning padded to 6.
+    # The editor lays out 4 lanes off the type; core must agree.
+    arr = Arrangement(
+        name="Low End",
+        type="bass",
+        tuning=[0, 0, 0, 0, 0, 0],
+        notes=[Note(time=float(i), string=i, fret=0) for i in range(4)],
+    )
+    assert arrangement_string_count(arr) == 4
+
+
+def test_string_count_6_for_authored_guitar_type_no_regression():
+    # A non-bass authored type on a generic name still resolves to the
+    # canonical 6 — the type signal only pulls DOWN to 4 for bass.
+    arr = Arrangement(
+        name="Track 1",
+        type="guitar",
+        notes=[Note(time=float(i), string=i, fret=0) for i in range(5)],
+    )
+    assert arrangement_string_count(arr) == 6
+
+
+def test_arrangement_is_bass_signal_safety():
+    # The manifest `type` is lifted onto arr.type verbatim; the helper must be
+    # safe against the messy shapes a hand-edited/loose source can produce.
+    from song import arrangement_is_bass
+    assert arrangement_is_bass(Arrangement(name="Low End", type="bass"))
+    assert arrangement_is_bass(Arrangement(name="Low End", type=" BASS "))  # ws/case
+    assert arrangement_is_bass(Arrangement(name="Low End", path_bass=True))
+    assert arrangement_is_bass(Arrangement(name="Slap Bass"))               # legacy name
+    # Non-bass / absent signals stay False (back-compat: no bass signal → 6).
+    assert not arrangement_is_bass(Arrangement(name="Lead", type=""))
+    assert not arrangement_is_bass(Arrangement(name="Rhythm", type="guitar"))
+    assert not arrangement_is_bass(Arrangement(name="", type=""))
+
+
 # ── compute_smart_names ───────────────────────────────────────────────────────
 
 def _sarr(path_lead=False, path_rhythm=False, path_bass=False,