feat(notation_lift): authored per-note hands steer the split — heuristic only guesses the rest

[ChrisBeWithYou]

What

The keys LH/RH hand arc, 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" path. Now that authored hands reach the wire (editor #299 emits per-note hand, core #990 round-trips it), the lift honors them:

• decode_wire_notes carries hand through ('lh'/'rh' strict enum; junk → None — a hand-edited pack can never 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 — the crossing-hands texture that makes the override load-bearing). All-explicit groups skip the heuristic entirely; charts with no hand data behave exactly as before (existing heuristic tests untouched and green).

Relationship to the other arc PRs

Independent and mergeable in any order with #990 / editor #299 / musicxml-import #8 — it reads the raw wire dicts, not the Note model. Together they complete: authored hands survive import → edit → save → relift.

Tests

Five new in tests/test_notation_lift.py: authored-wins (incl. crossing hands), group-removal-before-math (exact mean arithmetic that would flip without removal), all-explicit group, junk-enum fallback, decode carry-through. The decode shape pin updated for the new key. Suite: 1723 passed (+5 vs main; pre-existing env failures identical on pristine main).

🤖 Generated with Claude Code

https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q

Summary by CodeRabbit

• New Features

  • Decoded notes now preserve valid authored left- or right-hand assignments.
  • Explicit hand assignments take priority when splitting notes between hands.
  • Invalid hand values safely fall back to automatic assignment.

• Bug Fixes

  • Improved handling of mixed explicit and automatically assigned notes.
  • Sustain values remain safely constrained to non-negative values.

• Tests

  • Added coverage for authored hand assignments, invalid values, chord notes, and heuristic splitting.

[coderabbitai]

📝 Walkthrough

Walkthrough

Wire note decoding now preserves valid authored hand assignments for notes and chord notes. split_hands applies those assignments before using heuristic staff inference, while invalid or absent values continue through normalized fallback behavior.

Changes

Authored hand flow

Layer / File(s)	Summary
Decode authored hand assignments lib/notation_lift.py, tests/test_notation_lift.py	Decoded notes include normalized hand values, including chord notes, with only "lh" and "rh" preserved; tests cover absent and invalid values.
Prioritize authored hands lib/notation_lift.py, tests/test_notation_lift.py	split_hands assigns explicitly labeled notes before heuristic calculation and tests mixed, fully labeled, and invalid-hand groups.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

• got-feedBack/feedBack#990: Adds the underlying Note.hand model and wire serialization used by this decoding and splitting flow.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is detailed, but it omits the required feedpak surface section and the checklist items from the template.	Add the feedpak surface section (or remove it if irrelevant) and complete the checklist for changelog, tests, and DCO sign-off.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and clearly describes the main change: authored hands now influence hand splitting.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/lift-hand-aware

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

lib/notation_lift.py (1)

78-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract shared VALID_HANDS constant. The ("lh", "rh") enum literal is duplicated between decode_wire_notes and split_hands; both gatekeep the same authored-hand contract independently.

• lib/notation_lift.py#L78-L81: use a shared VALID_HANDS = ("lh", "rh") module constant instead of the inline tuple.
• lib/notation_lift.py#L134-L134: reference the same VALID_HANDS constant here to keep the two enum checks from drifting apart.

♻️ Proposed fix

+VALID_HANDS = ("lh", "rh")
+
 def decode_wire_notes(arr_data: dict) -> list[dict]:
     ...
-                "hand": hand if hand in ("lh", "rh") else None,
+                "hand": hand if hand in VALID_HANDS else None,

         for n in full_group:
-            if n.get("hand") in ("lh", "rh"):
+            if n.get("hand") in VALID_HANDS:
                 hands[n["hand"]].append(n)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/notation_lift.py` around lines 78 - 81, Define a module-level VALID_HANDS
= ("lh", "rh") constant in lib/notation_lift.py, then update decode_wire_notes
at lines 78-81 and split_hands at line 134 to reference it instead of inline
tuples; both sites require direct changes and behavior must remain unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@lib/notation_lift.py`:
- Around line 78-81: Define a module-level VALID_HANDS = ("lh", "rh") constant
in lib/notation_lift.py, then update decode_wire_notes at lines 78-81 and
split_hands at line 134 to reference it instead of inline tuples; both sites
require direct changes and behavior must remain unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 13bc7e6d-4062-4207-8330-4b40be8f1572

📥 Commits

Reviewing files that changed from the base of the PR and between 3717e43 and 2f414fc.

📒 Files selected for processing (2)

• lib/notation_lift.py
• tests/test_notation_lift.py