Skip to content

feat(editor): undoable part rename — kind-guarded display labels - #124

Merged
byrongamatos merged 3 commits into
mainfrom
feat/editor-rename-part
Jul 7, 2026
Merged

feat(editor): undoable part rename — kind-guarded display labels#124
byrongamatos merged 3 commits into
mainfrom
feat/editor-rename-part

Conversation

@ChrisBeWithYou

@ChrisBeWithYou ChrisBeWithYou commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

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 stable id.

What

  • ✏ button next to the arrangement selector (+ registry command renamePart) renames the current part through a new RenameArrangementCmd: 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 manifest id never changes — so per-part view prefs and the 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 (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 when arr.type fully replaces name inference (F9''s endgame).
  • Duplicate names refused case-insensitively (the pack name discipline); empty/overlong refused; an exact no-op fails silently.

Tests

tests/rename_part.test.js (6): the kind table — including the anchored-KEYS_PATTERN nuance ("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 through EditHistory (exec/undo/redo, selector refresh, id stability, captured-index targeting). Full suite green except the pre-existing CRLF section_coverage failure (#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

    • Added an undoable arrangement-part rename control to the editor top bar.
    • The rename option is available via the editor UI and is only shown when editing an arrangement.
  • Bug Fixes

    • Renames preserve stable part identity and related view/merge behavior.
    • Invalid renames are prevented, including empty names, overly long names, case-insensitive duplicates, and renames that would change the inferred instrument type.
  • Tests

    • Added automated coverage for rename validation, undo/redo behavior, and correct application to the targeted arrangement.

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
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@byrongamatos, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 20 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fd1f2759-6fe3-4841-885e-640b2ac9f2cb

📥 Commits

Reviewing files that changed from the base of the PR and between b8eea3b and 0b4c561.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • screen.html
  • screen.js
📝 Walkthrough

Walkthrough

This 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.

Changes

Part Renaming Feature

Layer / File(s) Summary
Rename command, validation logic, and command routing
screen.js
Adds the renamePart command entry, routes it to window.editorRenameArrangement(), and implements rename validation, kind inference, and the undoable RenameArrangementCmd.
Rename button UI wiring
screen.html, screen.js
Adds the hidden rename button and conditionally shows it when arrangements exist and a session is active.
Rename tests and changelog
tests/rename_part.test.js, CHANGELOG.md
Adds tests for rename guards and undo/redo behavior, and documents the feature in the changelog.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: an undoable editor part rename with kind-guarded behavior.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/editor-rename-part

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

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>
@byrongamatos

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@byrongamatos
byrongamatos merged commit 8eae01b into main Jul 7, 2026
ChrisBeWithYou pushed a commit that referenced this pull request Jul 8, 2026
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
byrongamatos added a commit that referenced this pull request Jul 8, 2026
)

* 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>
@byrongamatos
byrongamatos deleted the feat/editor-rename-part branch July 8, 2026 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants