feat(editor): create-mode songs can hold several drum parts too (re-land of #340) - #343
Conversation
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 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 |
PR4 of the drums-as-arrangement arc — closes the last authoring gap. #339 gave N drums to EDIT-mode (re-open + Save via /save_song); a CREATE-mode session (New Song from a GP/MIDI import) still capped at one drum chart, because its build persists through /build → _write_sloppak_pak, which only knew a single song-level drum_tab. Now the create-mode build path reaches parity: multiple drum parts persist per feedpak-spec 1.17.0 (primary as the song-level drum_tab alias, extras as type:"drums" arrangement entries with per-arrangement drum_tab pointers — old readers see one drum, new readers all). Backend (routes.py): - New module-level `_create_build_drum_entries(staging, drum_tab, drum_parts)`: writes each extra part's drum_tab_<id>.json into the staging dir and returns the type:"drums" manifest entries (primary alias + extras; ids sanitized to a safe filename charset and de-collided). Twin of /save_song's inline drum-parts block, sharing `_sanitize_extra_drum_tab` for one on-disk shape. Extracted to module scope (out of _write_sloppak_pak's closure) and kept PURE of the core `lib` — so pytest drives the real file write directly. - `_write_sloppak_pak` gains a `drum_parts` param; appends the entries after the pitched arrangements when the client opted in (a list, possibly empty). Absent → an old frontend → the pack stays byte-identical with just the song-level key. - `build_song_endpoint` reads + boundary-validates `drum_parts` (each part a dict with a schema-valid drum_tab, via lib.drums like /save_song — fail-fast 400) and threads it into the build. The other _write_sloppak_pak callers (save-as-sloppak) pass nothing → unaffected. Frontend: - editorBuild ships the drum payload via new pure `_drumBuildPayloadPure` (create.js): the PRIMARY drum_tab is the FIRST drums arrangement's payload, NOT the active S.drumTab (which tracks whichever part is open in the grid — the user may be editing a secondary at build time); `drum_parts` (the extras) ships only when drums are materialized as arrangements, so a legacy single-tab compose build stays byte-identical. - `_canAddAnotherDrums` drops the `!S.createMode` guard — create sessions add a 2nd part beside a pitched track now that the build persists it. The only remaining one-part case is a drums-only session (no melodic track to sit beside — drums are never index 0); the New-Track note + refusal message say so. Tests: JS 297 files green — `_drumBuildPayloadPure` (primary-not-active, empty-extras-still-ships, byte-identical fallback) + a REAL editorBuild→/build body integration test through saveCDLC (asserts the primary + extras on the wire); new_track create-mode add-a-2nd-part + the drums-only one-part guard. pytest 369 green (+5): `_create_build_drum_entries` real file write — side files + alias, empty extras, hit sanitation + name propagation, id de-collision + filename sanitization, non-list-hits sanitized-not-crashing. Lint 0 err / 3 baseline. Runtime posture: the create-mode /build write is now directly unit-tested against real file I/O (the extracted helper), the frontend build wire is integration-tested against the real editorBuild, and the on-disk shape is identical to /save_song's drum-parts persistence that #339 runtime-verified end-to-end (create parts → save → reload → both parts return). Stacked on #339. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix Signed-off-by: ChrisBeWithYou <chris@rifflarr.local>
…and of #340) Includes review fix: round-trip the primary drum part's id through the create/ build path (drum_tab_id → _primary_drum_alias_id), mirroring the #339 save-path fix, so a promoted primary keeps its stem links + tree placement on reload. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
b723891 to
4e8fbfe
Compare
Keep-both CHANGELOG; comment reconciliation with #343 (create-mode N-drums): create mode is no longer replace-semantics, so the Place-at gating comments now state the real rationale — placement there is deferred until the create build path proves it round-trips regions[]; the import still lands selected at source timing. Behavior unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix
2 participants
Re-land of #340, which GitHub auto-closed when its base branch (
mt-drums-p3/ #339) was squash-merged tomainthis morning — so create-mode N-drums never landed. Verified:_create_build_drum_entries/_drumBuildPayloadPurewere absent frommain, even though #339's/save_songN-drums path is in.This is #340's content (commit
76d37ef) cherry-picked onto currentmain.What
The several-drum-parts feature previously covered only songs you re-open and Save; a create-mode session (New Song from a Guitar Pro / MIDI import) was capped at one drum chart. Now, once a melodic track exists, ＋ Track ▸ Drums adds another part in a create session, and Build persists every part (primary → song-level
drum_tab; extras →type: drumsarrangement entries per feedpak-spec 1.17.0). Drums-only drafts still hold one part until a melodic track is added. Packs stay backward-compatible.Re-land conflict resolution (both trivial, keep-both)
CHANGELOG.md— kept main's region entries + this feature's entry.tests/test_drum_parts.py— union of imports (feat(editor): a song can hold several drum parts #339's_primary_drum_alias_idtest + feat(editor): create-mode songs can hold several drum parts too #340's_create_build_drum_entriestests both retained).Gates
_create_build_drum_entries(backend, real-file-write pytest) +_drumBuildPayloadPure(frontend) intact; #340's JS tests pass (create_save_routing,new_track= 2/2); lint 0 errors. Original #340 was CI-green (JS 297/0, pytest 369/0, lint 0/3). (Two local JS failures —mixer_meter_teardown,song_fit— are byte-identical tomainand environmental, unrelated to this change.)Part of the drums-as-arrangements arc now on main: #335–#339, core #1020 (+ picker re-land got-feedBack/feedBack#1028), spec got-feedBack/feedpak-spec#63.
🤖 Generated with Claude Code
https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix