Skip to content

refactor(editor): move arrangement management to src/arrangement.js (R2, step 30) - #182

Merged
byrongamatos merged 1 commit into
mainfrom
refactor/r2-step30-arrangement
Jul 10, 2026
Merged

byrongamatos merged 1 commit into
mainfrom
refactor/r2-step30-arrangement

Conversation

@byrongamatos

@byrongamatos byrongamatos commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

What

Step 30 of the R2 module-extraction refactor: move arrangement-level operations out of src/main.js into a new native ES module src/arrangement.js.

Covers:

  • Part rename — the undoable RenameArrangementCmd + the @pure:rename-arr cross-kind guard (a rename can't silently change the name-inferred instrument kind and strand notes).
  • Remove arrangement.
  • Add Drumsdrum_tab.json import from a GP or MIDI file. This is the drum sibling of the keys/guitar imports moved in step 29, and chains into import.js's two shared post-import dialogs (_maybeOfferMidiTempoMap / _showDrumImportUnmappedModal).

How the seam is drawn

  • 7 window.editor* entry points become named exports; main.js re-attaches them.
  • 0 back-exports — nothing in main.js references a symbol defined here.
  • Every dependency that stays in main.js (draw, updateStatus, updateArrangementSelector, effectiveAudioOffset) routes through host; all four are pre-existing hooks, so no new host wiring this step.
  • arrangement.js imports the two dialog helpers from import.js — a clean module→module edge.
  • tests/rename_part.test.mjs retargeted to slice @pure:rename-arr + RenameArrangementCmd from src/arrangement.js; it now injects a host stub because the command refreshes the selector via host.updateArrangementSelector.

main.js drops ~377 lines. 31 modules.

Verification

  • 90/90 JS suites pass; ESLint gate clean (0 errors); strict no-undef clean on arrangement.js.
  • Codex preflight: 0 correctness issues.
  • New headless harness (verify_arrangement.py) drives a real Add-Drums import of a 76-byte channel-9 drums SMF (kick+snare, 120 BPM, 4/4) through the actual #editor-add-drums-gp input and editorDoAddDrums: asserts the 7 handlers bind, the single-arrangement remove is a guarded no-op, host.effectiveAudioOffset is wired (POSTed audio_offset == a seeded +5s), and the arrangement.js → import.js edge works (the MIDI tempo-map modal appears). Negative-checked by dropping a re-attach → harness throws.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added arrangement renaming with validation for compatible instrument layouts, valid names, and uniqueness.
    • Added arrangement removal with confirmation and automatic editor state updates.
    • Added “Add Drums” import support for Guitar Pro and MIDI files, including track selection and import guidance.
  • Bug Fixes
    • Improved arrangement management reliability while preserving undo/redo behavior.

…R2, step 30)

Extract the arrangement-level operations out of the src/main.js monolith into
a new native ES module: the undoable part rename (with the cross-kind guard
@pure:rename-arr + RenameArrangementCmd), remove-arrangement, and the Add-Drums
import (drum_tab.json from a GP/MIDI file). Add-Drums is the drum sibling of the
keys/guitar imports in import.js and chains into its two shared post-import
dialogs (_maybeOfferMidiTempoMap / _showDrumImportUnmappedModal).

- src/arrangement.js: 7 window.editor* entry points (re-attached by main.js),
  the @pure:rename-arr block, RenameArrangementCmd. No back-exports: nothing in
  main.js references a symbol defined here.
- Every dep that stays in main.js (draw, updateStatus, updateArrangementSelector,
  effectiveAudioOffset) routes through host — all four are pre-existing hooks, so
  no new host wiring this step.
- tests/rename_part.test.mjs retargeted to slice @pure:rename-arr +
  RenameArrangementCmd from src/arrangement.js; injects a host stub because the
  command now refreshes the selector via host.updateArrangementSelector.

main.js drops ~377 lines (5,804 → after wiring). 31 modules.

Verified: 90/90 JS suites pass, ESLint gate clean (0 errors), strict
no-undef clean on arrangement.js, Codex preflight 0 issues. New headless harness
drives a real 76-byte channel-9 drums SMF through the actual Add-Drums import:
proves the 7 handlers bind, host.effectiveAudioOffset is wired (POSTed
audio_offset == seeded +5s), and the arrangement.js->import.js edge works (the
tempo-map modal appears). Negative-checked by dropping a re-attach.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 10, 2026 06:22
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f701bf0-e144-467e-819d-50cf4ae1c84e

📥 Commits

Reviewing files that changed from the base of the PR and between fba9977 and c05db6d.

📒 Files selected for processing (3)
  • src/arrangement.js
  • src/main.js
  • tests/rename_part.test.mjs

📝 Walkthrough

Walkthrough

Changes

Arrangement editor operations

Layer / File(s) Summary
Guarded arrangement rename
src/arrangement.js, tests/rename_part.test.mjs
Arrangement renaming now validates names and instrument compatibility, supports undo/redo, refreshes the selector, and uses the relocated pure-function test harness.
Arrangement removal state updates
src/arrangement.js
Removal confirms eligibility, optionally calls the backend, resets history, updates arrangement state, clears selection, and redraws the editor.
Drum-track import and editor wiring
src/arrangement.js, src/main.js
The Add Drums flow parses GP and MIDI files, imports selected tracks, updates drum state, triggers follow-up dialogs, and exposes handlers through module wiring.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Editor as editorDrumsFileSelected
  participant ImportAPI as import-list endpoint
  participant Commit as editorDoAddDrums
  participant DrumAPI as drum import endpoint
  participant State as S.drumTab and UI
  Editor->>ImportAPI: submit GP or MIDI file
  ImportAPI-->>Editor: return candidate tracks
  Editor->>Commit: select drum track
  Commit->>DrumAPI: submit selected track and offset
  DrumAPI-->>Commit: return drum_tab
  Commit->>State: store and sort hits, clear selection, refresh UI
Loading

Possibly related PRs

🚥 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 accurately reflects the main change: moving editor arrangement management into src/arrangement.js.
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 refactor/r2-step30-arrangement

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR continues the R2 module-extraction refactor by moving arrangement-level editor operations out of src/main.js into a dedicated ES module (src/arrangement.js), with main.js re-attaching the public window.editor* entry points and routing retained main.js dependencies through the existing host hook object.

Changes:

  • Extracts arrangement rename (including the cross-kind @pure:rename-arr guard), arrangement removal, and Add-Drums import flow into src/arrangement.js.
  • Updates src/main.js to import these new module exports and re-attach the corresponding window.editor* handlers.
  • Retargets tests/rename_part.test.mjs to slice @pure:rename-arr and RenameArrangementCmd from src/arrangement.js, injecting a host stub for selector refresh.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/rename_part.test.mjs Updates the test harness to read/extract the pure rename guard and command from src/arrangement.js, and stubs host.updateArrangementSelector.
src/main.js Removes arrangement-management implementations and re-attaches arrangement-related window.editor* entry points to src/arrangement.js exports; keeps dependencies routed via host.
src/arrangement.js New module containing rename/remove/add-drums logic, calling back into main.js through host and reusing shared import dialogs from import.js.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@byrongamatos
byrongamatos merged commit 59bf3f5 into main Jul 10, 2026
5 checks passed
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