Fix Nord Lead MIDI import and remaining PR #559 review findings - #570
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Nord Lead MIDI parser now rejects embedded status bytes in escaped SysEx messages. Tests cover malformed dumps and rename the SMF helper parameter. Project documentation clarifies live-request bank availability. ChangesNord Lead updates
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Nord Lead MIDI imports now fail cleanly for malformed escaped SysEx data rather than returning partial patches, while compatibility notes clarify supported live-request banks. No current merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Nord Lead MIDI imports could silently return only earlier valid patches when an F7 escape contained an explicitly framed SysEx message with an embedded status byte. Validate those messages before collecting them and raise ValueError for the malformed file.
Addresses the three outstanding CodeRabbit findings from #559:
The earlier request-ID finding was already settled in #559; the documented 0x0B-0x0E range remains correct.
Validation: Python 3.12, Nord Lead generic and dedicated tests: 125 passed, 10 expected skips for unsupported optional adaptation capabilities. All ten new cases failed before the parser fix and pass afterward. Existing valid escaped, fragmented, and official MIDI fixture imports pass. git diff --check is clean. Hardware behavior was not tested.
Summary by CodeRabbit
Bug Fixes
Documentation