feat(editor): undoable part rename — kind-guarded display labels - #124
Conversation
DAW-workspace 2.2b (first half — rename; reorder is a separable follow-up), unblocked by #101: the merge-not-rebuild save keeps type/unknown keys across a rename, and sloppak sessions carry a stable manifest id. - RenameArrangementCmd: captured-index targeting (undo after a switch lands on the right part), exec/rollback refresh the selector, the stable id never changes (view prefs + manifest merge survive). - The hard limit, enforced honestly: the NAME still drives kind inference (KEYS_PATTERN -> piano roll + notation sidecar, /bass/i -> 4-lane layout, /^drums/i -> drum routing), so a rename that would change the inferred instrument is REFUSED with an explanation — silently re-laning a 6-string chart as a bass would strand notes on invisible strings. Cross-kind moves stay "add a new part". - Duplicate names refused case-insensitively (pack name discipline); empty/overlong refused; exact no-op fails silently. - Toolbar pencil button next to remove-arr + registry renamePart. Tests: tests/rename_part.test.js (6) — kind table (incl. the anchored KEYS_PATTERN nuance: "Electric Piano" is NOT a keys name by the layout rules), guard truth table, and the real command round-tripped through EditHistory (selector refresh, id stability, captured-index targeting). Full suite green except pre-existing CRLF section_coverage (#116). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu
|
Warning Review limit reached
Next review available in: 20 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 (3)
📝 WalkthroughWalkthroughThis PR adds undoable arrangement-part renaming. It wires a rename button and command into the editor, adds validation and kind-inference checks, implements undo/redo support, and includes tests plus changelog documentation. ChangesPart Renaming Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Guard the rename against BOTH name interpreters, not one: the live lane/roll router keys off prefix-anchored KEYS_PATTERN while the save side (routes.py _KEYS_NAME_RE / _TYPE_BASS_RE) keys off word-boundary matches. They disagree on names like "Electric Piano" (save-keys, runtime-guitar) and "Synthwave Lead" (runtime-keys, save-guitar), so a one-facet guard let a rename silently re-lane a chart on save/reload or on the next draw. _renameGuardPure now refuses when either _arrKindPure or _arrSaveKindPure moves; regression tests cover both directions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
# Conflicts: # CHANGELOG.md # screen.js
DAW-workspace 2.2b (second half; rename shipped as #124). Completes the 2.2b remainders that were unblocked by #101. - New < / > buttons next to the arrangement selector + registry commands movePartEarlier/movePartLater: one-slot moves, per-end disabling so the affordance always tells the truth. - Order persists: sloppak saves ship the CLIENT S.arrangements array as the full snapshot and the manifest merge keys entries by id — verified against _buildSaveBody before building. - A move renumbers arrangement indices, so the undo history RESETS (the remove-arrangement rationale) — which is also why the move itself is not undoable: move it back. Blocked mid-recording (a take pins its arrangement index). Selection cleared; selector rebuilt; currentArr follows the moved part. Tests: tests/reorder_part.test.js (4) — pure target math (ends, degenerate inputs), the real handler over an injected env (object identity through the swap, currentArr follow, history reset, selection clear), the clean-no-op-at-ends case (no gratuitous reset), and the recording block. Full suite green except pre-existing CRLF section_coverage (#116). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu
) * feat(editor): part reorder — move earlier/later, persisted on save DAW-workspace 2.2b (second half; rename shipped as #124). Completes the 2.2b remainders that were unblocked by #101. - New < / > buttons next to the arrangement selector + registry commands movePartEarlier/movePartLater: one-slot moves, per-end disabling so the affordance always tells the truth. - Order persists: sloppak saves ship the CLIENT S.arrangements array as the full snapshot and the manifest merge keys entries by id — verified against _buildSaveBody before building. - A move renumbers arrangement indices, so the undo history RESETS (the remove-arrangement rationale) — which is also why the move itself is not undoable: move it back. Blocked mid-recording (a take pins its arrangement index). Selection cleared; selector rebuilt; currentArr follows the moved part. Tests: tests/reorder_part.test.js (4) — pure target math (ends, degenerate inputs), the real handler over an injected env (object identity through the swap, currentArr follow, history reset, selection clear), the clean-no-op-at-ends case (no gratuitous reset), and the recording block. Full suite green except pre-existing CRLF section_coverage (#116). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu * feat(editor): review fixes for #125 (part reorder) Gate part-reorder to sloppak sessions. The new order persists only on the full-arrangement snapshot that _buildSaveBody ships for sloppak saves; an archive save writes just the active arrangement keyed by arrangement_index, so a client-side reorder was silently lost on reload and, worse, the stale index re-targeted the moved part into the wrong original slot. Hide the buttons for non-sloppak (matching +Keys/Record) and refuse in the handler so the command-palette/keyboard paths can't bypass the hidden buttons. Regression test: archive sessions refuse the move (fails on pre-fix code). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: ChrisBeWithYou <chris@rifflarr.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com>
DAW-workspace 2.2b, first half (rename; drag-reorder is a separable follow-up). This was flagged "safe once §7 lands" — #101''s merge-not-rebuild save landed it: a rename no longer strips the manifest entry''s
type/unknown keys, and sloppak sessions carry a stableid.What
renamePart) renames the current part through a newRenameArrangementCmd: full undo/redo, captured-index targeting (undo after an arrangement switch lands on the right part via the history''s per-arrangement tagging), selector text follows on both exec and rollback, and the stable manifestidnever changes — so per-part view prefs and the manifest merge survive.KEYS_PATTERN→ piano roll + notation sidecar,/bass/i→ 4-lane layout,/^drums/i→ drum routing — so a rename that would change the inferred instrument is refused with an explanation (guitar→bass would silently re-lane a 6-string chart and strand notes on invisible strings). Cross-kind moves stay "add a new part". This constraint dissolves whenarr.typefully replaces name inference (F9''s endgame).Tests
tests/rename_part.test.js(6): the kind table — including the anchored-KEYS_PATTERNnuance ("Electric Piano" is NOT a keys name by the layout rules, and the guard must mirror what routing would actually do) — the guard truth table (cross-kind reasons, dup/empty/overlong/no-op), and the real command round-tripped throughEditHistory(exec/undo/redo, selector refresh, id stability, captured-index targeting). Full suite green except the pre-existing CRLFsection_coveragefailure (#116 fixes it).Fresh region vs the open queue — the arrangement-selector toolbar cluster + one command class.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu
Summary by CodeRabbit
New Features
Bug Fixes
Tests