Fix keyboard-arrangement misclassification and highway_3d chord-diagram mirroring

.gitignore

@@ -65,3 +65,5 @@ Thumbs.db
 *.swp
 .idea/
 plugins/support_creators
+/library
+/static/sloppak_cache

CHANGELOG.md

@@ -355,6 +355,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   engine (`app.js`, `highway.js`, `playSong`, `showScreen`, the capability registry).
 
 ### Fixed
+- **`convert_wem` no longer blocks the event loop inside `highway_ws`.** Both
+  call sites in `lib/routers/ws_highway.py` (loose-folder and archive audio
+  conversion) invoked `convert_wem` directly inside the `async def
+  highway_ws` handler; `convert_wem` shells out to vgmstream-cli/ffmpeg via
+  `subprocess.run` with up to a 120s timeout, so a single slow/large
+  conversion held the whole event loop and stalled every other concurrent
+  WebSocket connection on that worker for as long as it ran. Both sites now
+  run through `loop.run_in_executor()`, reusing the `contextvars.copy_context()`
+  snapshot already taken earlier in the function so the bound `ws_conn_id`
+  correlation ID still applies to log lines raised inside the executor
+  thread — same pattern this file already uses for `load_song`/
+  `sloppak_mod.load_song`.
+- **highway_3d chord diagram no longer mirrors on Invert.** The top-left chord
+  diagram overlay (`drawChordDiagram()`) was flipping its column order
+  (high-e/low-E swapped) whenever the highway's Invert toggle was on, passed
+  through as `inverted: _invertedCached` at both call sites. The diagram's
+  orientation should be fixed regardless of that toggle, so both call sites
+  now pass `inverted: false`. Note: `plugins/highway_3d/CLAUDE.md` had
+  documented the mirroring as this overlay's contract, but that line traces
+  only to a single squashed "Clean release snapshot" commit with no
+  surviving design rationale — treated here as an inaccurate description of
+  a bug, not a protected feature, and updated accordingly.
 - **Count-in follows the song's meter and its pickup measure.** The count-in
   (loop wrap, section practice, and the "Countdown before song" setting) always
   clicked exactly four beats, so a 3/4 song was counted in 4/4, and a song
@@ -366,6 +388,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   bar shorter than that meter shortens the count by its length: a 1-beat pickup
   in 4/4 counts "1 2 3" and the music enters on 4. Songs without beats — pre-chart,
   minigames, synthetic highways — still get four.
+
 - **GP8 asset resolution honours the directory the registry named.**
   `<EmbeddedFilePath>` is matched on filename stem so a format variant of the
   same recording can win (an `.ogg` beside the declared `.mp3` is copied out

lib/gp2rs_gpx.py

@@ -2531,12 +2531,20 @@ def _auto_select_gpx(tracks: list[dict]) -> tuple[list[int], dict[int, str]]:
 
         is_bass = (t['string_pitches'] and max(t['string_pitches']) <= 48) \
             or t['midi_program'] in BASS_PROGS
-        is_guitar = bool(t['string_pitches']) and not is_bass
         is_keys = (not t['string_pitches'] and t['midi_program'] in KEYS_PROGS) \
             or any(kw in name_l for kw in ('piano', 'keys', 'organ'))
+        # GP6+ often notates piano/keys parts on a fretted string template, so
+        # string_pitches alone can't distinguish a keyboard part from a real
+        # guitar. Check is_keys (which also matches on name/program) before
+        # is_guitar, so a track explicitly named "Keys ..." isn't swept into
+        # the unhinted-guitar Lead/Rhythm/Combo bucket just because it has
+        # string tuning data (feedBack: "Combo" mislabeled piano arrangement).
+        is_guitar = bool(t['string_pitches']) and not is_bass and not is_keys
 
         if is_bass:
             selected.append((i, 'bass'))
+        elif is_keys:
+            selected.append((i, 'keys'))
         elif is_guitar:
             # Honor "lead"/"rhythm" in the GP track name so two guitars keep
             # the author's roles instead of being labelled by appearance order
@@ -2547,8 +2555,6 @@ def _auto_select_gpx(tracks: list[dict]) -> tuple[list[int], dict[int, str]]:
                 selected.append((i, 'guitar_rhythm'))
             else:
                 selected.append((i, 'guitar'))
-        elif is_keys:
-            selected.append((i, 'keys'))
 
     if not selected:
         for i, t in enumerate(tracks):

lib/routers/ws_highway.py

@@ -209,10 +209,23 @@ async def _send_keepalives():
             _ctx = contextvars.copy_context()
             if is_slop:
                 appstate.sloppak_cache_dir.mkdir(parents=True, exist_ok=True)
-                loaded_slop = await loop.run_in_executor(
-                    None,
-                    lambda: _ctx.run(sloppak_mod.load_song, filename, dlc, appstate.sloppak_cache_dir),
-                )
+                try:
+                    loaded_slop = await loop.run_in_executor(
+                        None,
+                        lambda: _ctx.run(sloppak_mod.load_song, filename, dlc, appstate.sloppak_cache_dir),
+                    )
+                except Exception:
+                    # load_song() never returns None on failure — it raises
+                    # (bad zip, missing/corrupt manifest, ...). Catch that
+                    # here so the client gets a clean, generic message
+                    # instead of the outer handler's raw str(e), which can
+                    # leak filesystem paths.
+                    log.exception("sloppak load failed for %s", filename)
+                    _keepalive_active = False
+                    keepalive_task.cancel()
+                    await websocket.send_json({"error": "Failed to load sloppak"})
+                    await websocket.close()
+                    return
                 song = loaded_slop.song
                 tmp = str(loaded_slop.source_dir)
                 owns_tmp = False
@@ -460,7 +473,15 @@ def _evict_audio_cache():
                     tmp_suffix = uuid.uuid4().hex[:8]
                     tmp_base = appstate.audio_cache_dir / f"audio_{audio_id}.{tmp_suffix}"
                     try:
-                        produced = convert_wem(str(wem_resolved), str(tmp_base))
+                        # convert_wem shells out to vgmstream-cli/ffmpeg via
+                        # subprocess.run (up to 120s timeout) — bare-calling it
+                        # here would block the whole event loop, stalling every
+                        # other concurrent connection's WebSocket traffic for
+                        # as long as the conversion takes.
+                        produced = await loop.run_in_executor(
+                            None,
+                            lambda: _ctx.run(convert_wem, str(wem_resolved), str(tmp_base)),
+                        )
                         ext = Path(produced).suffix
                         final_path = appstate.audio_cache_dir / f"audio_{audio_id}{ext}"
                         os.replace(produced, final_path)
@@ -482,7 +503,13 @@ def _evict_audio_cache():
                 audio_error = "No WEM audio files were found inside this archive."
             else:
                 try:
-                    audio_path = convert_wem(wem_files[0], os.path.join(tmp, "audio"))
+                    # Same reasoning as the loose-folder conversion above:
+                    # convert_wem is a blocking subprocess call and must not
+                    # run inline on the event loop.
+                    audio_path = await loop.run_in_executor(
+                        None,
+                        lambda: _ctx.run(convert_wem, wem_files[0], os.path.join(tmp, "audio")),
+                    )
                     ext = Path(audio_path).suffix
                     audio_dest = appstate.audio_cache_dir / f"audio_{audio_id}{ext}"
                     shutil.copy2(audio_path, audio_dest)
@@ -500,6 +527,11 @@ def _evict_audio_cache():
                 "name": a.name,
                 "smart_name": smart_names[i],
                 "notes": len(a.notes) + sum(len(c.notes) for c in a.chords),
+                # Manifest `type` (sloppak.py:942) — authoritative instrument
+                # classification, independent of the display name. Lets viz
+                # auto-selection (e.g. the piano viz's matchesArrangement)
+                # match on real type instead of name-sniffing.
+                "type": (a.type or "").strip().lower() if isinstance(a.type, str) else "",
             }
             for i, a in enumerate(song.arrangements)
         ]
@@ -512,6 +544,9 @@ def _evict_audio_cache():
             "arrangement": arr.name,
             "arrangement_smart_name": smart_names[best],
             "arrangement_index": best,
+            # Named distinctly from the top-level "type" (WS message
+            # discriminator, = "song_info") to avoid colliding with it.
+            "arrangement_type": (arr.type or "").strip().lower() if isinstance(arr.type, str) else "",
             # Echo the resolved naming mode so highway.js doesn't have to
             # re-read localStorage (which can be unavailable / disagree with
             # app.js's in-memory cache when storage writes fail).

lib/song.py

@@ -805,6 +805,14 @@ def _resolve(a: Arrangement) -> tuple[str | None, bool]:
             return "path_rhythm", bool(a.bonus_arr)
         if a.path_bass:
             return "path_bass", bool(a.bonus_arr)
+        # The manifest `type` field (sloppak.py:942) is authoritative when
+        # present — trust it over name-sniffing. A "keys"/"vocals"/"drums"
+        # arrangement is never part of the Lead/Rhythm/Bass grouping, even
+        # if its display name happens to collide with the name-fallback
+        # table below (e.g. a keys arrangement literally named "Combo").
+        arr_type = (a.type or "").strip().lower() if isinstance(a.type, str) else ""
+        if arr_type in ("keys", "vocals", "drums"):
+            return None, bool(a.bonus_arr)
         name = a.name if isinstance(a.name, str) else ""
         entry = _NAME_FALLBACK.get(name.strip().lower())
         if entry is None:

plugins/highway_3d/CLAUDE.md

@@ -124,7 +124,7 @@ Each entry names the function or banner you should grep for, plus key sub-blocks
 
 ### Lyrics & overlays
 - **Lyrics overlay** → `drawLyrics()`. 2D canvas, top centre, semi-transparent rounded background, syllable-level highlighting (current syllable in white, played in muted, upcoming in dim).
-- **Chord diagram overlay** → `drawChordDiagram()` (see "Chords" above). 2D canvas, top-left, fades over the 0.55 s linger window. Respects `inverted` (column 0 is high-e when inverted, low-E otherwise).
+- **Chord diagram overlay** → `drawChordDiagram()` (see "Chords" above). 2D canvas, top-left, fades over the 0.55 s linger window. `drawChordDiagram()` still accepts an `inverted` param (column 0 is high-e when inverted, low-E otherwise), but the two call sites always pass `inverted: false` — the diagram's orientation is fixed and does not mirror when the highway's own Invert toggle is on.
 - **The `lyricsCanvas`** is created in `initScene()` with `z-index:1`, appended to `wrap` **after** `ren.domElement` — this is the empirically-correct stacking order for all browsers/contexts (including splitscreen panels with `position:relative; overflow:hidden`). Don't reorder; see Pitfall #5.
 
 ### Splitscreen

plugins/highway_3d/plugin.json

@@ -1,7 +1,7 @@
 {
     "id": "highway_3d",
     "name": "3D Highway",
-    "version": "3.34.1",
+    "version": "3.34.2",
     "type": "visualization",
     "bundled": true,
     "script": "screen.js",

plugins/highway_3d/screen.js

@@ -1498,7 +1498,7 @@
      * Minimum staggered hits inside a hand-shape window for note-stream arpeggio
      * inference. A genuine arpeggio sweeps several strings of the held shape;
      * a 2-note melodic motif inside a multi-string ``<handShape>`` (e.g. Jackson 5
      * "I Want You Back" ~0:27 — Fm7 transition fingering with two plucks on
      * strings 4–5) earlier registered as arpeggio and produced a stray lavender
      * chord frame + purple lane outer dividers. Cap at ``min(shape.size, 3)``
      * so 2-string voicings still infer normally and 3+ string templates need
@@ -16544,7 +16544,9 @@
                                 ? Math.min(1.0, Math.max(0, (bundle.currentTime - _diagPrev.t) / DIAG_ENTRANCE_S))
                                 : 1.0,
                             canvasW: lyricsCanvas.width, canvasH: lyricsCanvas.height,
-                            inverted: _invertedCached,
+                            // Chord diagram orientation is fixed regardless of the
+                            // highway's own Invert toggle.
+                            inverted: false,
                             sizeSlider: chordDiagramSize, position: chordDiagramPosition,
                             nStr: _diagPrev.nStr ?? nStr,
                             lyricsBottom,
@@ -16558,7 +16560,9 @@
                             opacity: Math.max(0, 1 + (_diagChord.t - bundle.currentTime) / DIAG_LINGER_S),
                             entranceT: _diagEntranceT,
                             canvasW: lyricsCanvas.width, canvasH: lyricsCanvas.height,
-                            inverted: _invertedCached,
+                            // Chord diagram orientation is fixed regardless of the
+                            // highway's own Invert toggle.
+                            inverted: false,
                             sizeSlider: chordDiagramSize, position: chordDiagramPosition,
                             nStr: _diagChord.nStr ?? nStr,
                             lyricsBottom,
@@ -16662,7 +16666,13 @@
     // arrangements that merely contain these as substrings (e.g. a
     // "BasslineKeys" arrangement would otherwise match `bass`).
     window.feedBackViz_highway_3d.matchesArrangement = function (songInfo) {
-        const arr = (songInfo && songInfo.arrangement) || '';
+        // Manifest `type: keys` is authoritative and independent of the
+        // display name — a keys arrangement literally named "Combo" (GP
+        // import quirk) would otherwise match the /combo/ keyword below
+        // and steal the song from the piano/keys viz. Yield whenever the
+        // active arrangement's real type says keys.
+        if (songInfo && songInfo.arrangement_type === 'keys') return false;
+        const arr = songInfo?.arrangement || '';
         return /\b(?:lead|rhythm|bass|combo|guitar)\b/i.test(arr);
     };
 

static/app.js

@@ -1001,6 +1001,7 @@
     // migration step — playback genuinely continues, so don't emit song:play or
     // flip feedBack.isPlaying (the watcher keeps the canonical state itself).
     if (window._juceRerouteInProgress) return;
+    if (window._stemsRerouteInProgress) return;
     window.feedBack.isPlaying = true;
     const payload = _songEventPayload();
     window.feedBack.emit('song:play', payload);
@@ -1011,6 +1012,7 @@
     // Same as above: suppress the song:pause emitted by a reroute's deliberate
     // audio.pause() — the migration is transparent to plugin play-state.
     if (window._juceRerouteInProgress) return;
+    if (window._stemsRerouteInProgress) return;
     window.feedBack.isPlaying = false;
     window.feedBack.emit('song:pause', _songEventPayload());
 });
@@ -1496,7 +1498,7 @@
         'position:fixed', 'inset:0', 'z-index:200', 'display:flex',
         'align-items:center', 'justify-content:center',
         'background:rgba(0,0,0,0.6)',
         'font:14px/1.4 system-ui,-apple-system,"Segoe UI",Roboto,sans-serif',
     ].join(';');
 
     const card = document.createElement('div');

static/js/transport.js

@@ -358,6 +358,12 @@ export async function togglePlay() {
             // leave the button showing Play while the song keeps playing — the
             // "two clicks to pause on the first song after a fresh load" bug.
             if (window._juceRerouteInProgress) return;
+            // Same shape of race, HTML5 -> stems-plugin Web-Audio takeover
+            // (get-flashbacks/feedBack#39): the stems plugin deliberately
+            // pauses the core element while it builds its own multi-stem
+            // transport, then dispatches a synthetic 'play' once that
+            // transport actually starts. Don't stomp the button in between.
+            if (window._stemsRerouteInProgress) return;
             console.error('[app] audio.play() rejected:', err);
             S.isPlaying = false;
             setPlayButtonState(false);