From 2f414fc0bca32398ee38c59c84591a4bf703105e Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Thu, 16 Jul 2026 13:07:57 -0500 Subject: [PATCH] =?UTF-8?q?feat(notation=5Flift):=20authored=20per-note=20?= =?UTF-8?q?hands=20steer=20the=20split=20=E2=80=94=20heuristic=20only=20gu?= =?UTF-8?q?esses=20the=20rest?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The keys LH/RH hand arc, the lift slice. split_hands was purely heuristic (mean pitch vs middle C / largest-gap), so ANY edit to a keys arrangement re-derived hand splits that could contradict the score's authored grand-staff assignment — the documented "produces wrong hand splits" failure, now that authored hands actually reach the wire (editor #299 emits per-note `hand`; core #990 round-trips it). - decode_wire_notes carries `hand` through ('lh'/'rh' strict enum; junk → None so a hand-edited pack can't steer the split). - split_hands: an authored hand always wins, and explicit notes are REMOVED from their simultaneous group BEFORE the heuristic math runs — one authored assignment must never skew its chordmates' guesses (e.g. an authored LH melody note above middle C dragging the group mean down and flipping the rest). All-explicit groups skip the heuristic entirely; unassigned notes behave exactly as before. Design per the piano-pedagogy review of the arc: binary lh/rh + absent = unassigned; per-note explicit > heuristic precedence; crossing-hands textures are exactly why the override is load-bearing. Tests: five new in test_notation_lift.py (authored wins incl. a crossing-hands case, group-removal-before-math with exact mean arithmetic, all-explicit group, junk enum, decode carry-through); the decode shape pin updated for the new key. Suite: 1723 passed (+5 vs main; the pre-existing env failures reproduce identically on pristine main). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q Signed-off-by: ChrisBeWithYou --- lib/notation_lift.py | 50 +++++++++++++++++++++++++--------- tests/test_notation_lift.py | 53 ++++++++++++++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 13 deletions(-) diff --git a/lib/notation_lift.py b/lib/notation_lift.py index 7a40fb71..0a256828 100644 --- a/lib/notation_lift.py +++ b/lib/notation_lift.py @@ -54,15 +54,20 @@ def decode_wire_notes(arr_data: dict) -> list[dict]: """Decode an arrangement JSON's notes + chord notes to - ``[{"t": float, "midi": int, "sus": float}, ...]`` sorted by time. + ``[{"t": float, "midi": int, "sus": float, "hand": str|None}, ...]`` + sorted by time. Keys content packs absolute MIDI as ``midi = s*24 + f`` (sloppak-spec §5.3 legacy fallback). Sustain is the ``sus`` field (``l`` accepted as a - legacy alias). Entries with malformed fields are skipped. + legacy alias). ``hand`` is the authored per-note hand assignment + (``'lh'``/``'rh'`` — e.g. from a MusicXML grand-staff import via the + editor); a strict enum decode, anything else reads as ``None`` + (unassigned) so junk can never steer the hand split. Entries with + malformed fields are skipped. """ out: list[dict] = [] - def _push(t, s, f, sus): + def _push(t, s, f, sus, hand): try: t = float(t) midi = int(s) * 24 + int(f) @@ -70,11 +75,15 @@ def _push(t, s, f, sus): except (TypeError, ValueError): return if 0 <= midi <= 127: - out.append({"t": t, "midi": midi, "sus": max(0.0, sus)}) + out.append({ + "t": t, "midi": midi, "sus": max(0.0, sus), + "hand": hand if hand in ("lh", "rh") else None, + }) for n in arr_data.get("notes") or []: if isinstance(n, dict): - _push(n.get("t"), n.get("s"), n.get("f"), n.get("sus", n.get("l"))) + _push(n.get("t"), n.get("s"), n.get("f"), n.get("sus", n.get("l")), + n.get("hand")) for ch in arr_data.get("chords") or []: if not isinstance(ch, dict): continue @@ -83,7 +92,7 @@ def _push(t, s, f, sus): if isinstance(cn, dict): # Chord notes carry no own time — they sound at the chord's t. _push(cn.get("t", ch_t), cn.get("s"), cn.get("f"), - cn.get("sus", cn.get("l"))) + cn.get("sus", cn.get("l")), cn.get("hand")) out.sort(key=lambda n: (n["t"], n["midi"])) return out @@ -103,14 +112,31 @@ def group_simultaneous(notes: list[dict]) -> list[list[dict]]: def split_hands(notes: list[dict]) -> dict[str, list[dict]]: - """Assign every note to ``rh`` or ``lh`` per the heuristic. - - Per simultaneous group: a span > 12 semitones splits at the largest - internal interval gap (low side → lh); otherwise the whole group goes by - mean pitch vs middle C (≥ 60 → rh). + """Assign every note to ``rh`` or ``lh``. + + An AUTHORED per-note ``hand`` ('lh'/'rh' — a MusicXML grand-staff import + or a hand edit in the editor) always wins: those notes go straight to + their hand and are REMOVED from the group before any heuristic math runs, + so one explicit assignment can never skew its chordmates' guesses (e.g. + an authored LH melody note above middle C must not drag the group mean + down and flip the remaining notes). + + The remaining unassigned notes take the heuristic, per simultaneous + group: a span > 12 semitones splits at the largest internal interval gap + (low side → lh); otherwise the whole group goes by mean pitch vs middle C + (≥ 60 → rh). """ hands: dict[str, list[dict]] = {"rh": [], "lh": []} - for group in group_simultaneous(notes): + for full_group in group_simultaneous(notes): + # Authored hands first — explicit notes leave the group entirely. + group = [] + for n in full_group: + if n.get("hand") in ("lh", "rh"): + hands[n["hand"]].append(n) + else: + group.append(n) + if not group: + continue pitches = sorted(n["midi"] for n in group) span = pitches[-1] - pitches[0] if len(pitches) > 1 and span > HAND_SPLIT_SPAN_SEMITONES: diff --git a/tests/test_notation_lift.py b/tests/test_notation_lift.py index 7a8c8f8f..9d20e024 100644 --- a/tests/test_notation_lift.py +++ b/tests/test_notation_lift.py @@ -34,7 +34,7 @@ def test_decode_wire_notes_unpacks_midi_and_sorts(): arr = {"notes": [_wire(1.0, 67, 0.5), _wire(0.0, 60)]} out = nl.decode_wire_notes(arr) assert [n["midi"] for n in out] == [60, 67] - assert out[0] == {"t": 0.0, "midi": 60, "sus": 0.0} + assert out[0] == {"t": 0.0, "midi": 60, "sus": 0.0, "hand": None} assert out[1]["sus"] == 0.5 @@ -105,6 +105,57 @@ def test_split_hands_middle_c_split_falls_back_when_it_makes_unplayable_hand(): assert sorted(n["midi"] for n in hands["rh"]) == [59, 62, 67] +def test_split_hands_authored_hand_always_wins(): + # An authored 'lh' melody note ABOVE middle C (a crossing-hands texture): + # the heuristic alone would call midi 65 rh; the authored hand wins. + notes = [{"t": 0.0, "midi": 65, "sus": 0, "hand": "lh"}] + hands = nl.split_hands(notes) + assert [n["midi"] for n in hands["lh"]] == [65] + assert "rh" not in hands + + +def test_split_hands_explicit_notes_leave_the_group_before_heuristic_math(): + # Group [C3(authored rh!), C4, E4]: without removal, C3=48 drags the mean + # to (48+60+64)/3 ≈ 57.3 < 60 → the WHOLE group would flip lh. With the + # authored note removed first, the remaining [C4, E4] mean 62 ≥ 60 → rh. + notes = [ + {"t": 0.0, "midi": 48, "sus": 0, "hand": "rh"}, + {"t": 0.0, "midi": 60, "sus": 0}, + {"t": 0.0, "midi": 64, "sus": 0}, + ] + hands = nl.split_hands(notes) + assert sorted(n["midi"] for n in hands["rh"]) == [48, 60, 64] + assert "lh" not in hands + + +def test_split_hands_all_explicit_group_skips_heuristic_entirely(): + notes = [ + {"t": 0.0, "midi": 40, "sus": 0, "hand": "rh"}, # deliberately "wrong" + {"t": 0.0, "midi": 72, "sus": 0, "hand": "lh"}, # crossing hands + ] + hands = nl.split_hands(notes) + assert [n["midi"] for n in hands["rh"]] == [40] + assert [n["midi"] for n in hands["lh"]] == [72] + + +def test_split_hands_junk_hand_values_fall_to_the_heuristic(): + for junk in ("LH", "left", "", True, 3, None): + hands = nl.split_hands([{"t": 0.0, "midi": 72, "sus": 0, "hand": junk}]) + assert [n["midi"] for n in hands.get("rh", [])] == [72], repr(junk) + + +def test_decode_wire_notes_carries_hand_with_strict_enum(): + arr = {"notes": [ + {"t": 0.0, "s": 2, "f": 0, "sus": 0.5, "hand": "lh"}, + {"t": 0.5, "s": 2, "f": 12, "sus": 0.5, "hand": "LH"}, # junk case + {"t": 1.0, "s": 2, "f": 14, "sus": 0.5}, + ], "chords": [ + {"t": 1.5, "notes": [{"s": 3, "f": 0, "sus": 0.5, "hand": "rh"}]}, + ]} + decoded = nl.decode_wire_notes(arr) + assert [n["hand"] for n in decoded] == ["lh", None, None, "rh"] + + # ── Timing ─────────────────────────────────────────────────────────────────── def test_downbeat_times_filters_non_downbeats_and_sorts():