Skip to content

feat(editor): track regions — move a region by dragging its block - #334

Merged
byrongamatos merged 1 commit into
mainfrom
track-regions-p3
Jul 21, 2026
Merged

feat(editor): track regions — move a region by dragging its block#334
byrongamatos merged 1 commit into
mainfrom
track-regions-p3

Conversation

@ChrisBeWithYou

Copy link
Copy Markdown
Contributor

The move step of the track-region layer: grab a track's region block in the Tracks (Parts) view and slide it along the timeline to reposition the track's content, as one undoable, bar-snapped step. Third in the region series — stacked on #333 (strip + selection), which is stacked on #332 (data model). Merge #332#333 → this.

This is PR 3a of a split: the charrette's PR 3 was commands + drag and the "Add Track from File" import driver; per the operability seat they're separated. The import driver (3b) is a follow-up and is blocked on a product decision (see below).

What it does

  • src/region-commands.js (new): MoveRegionCmd. A notation region shifts the arrangement's contained notes; a drum region shifts S.drumTab's contained hits. Routes through S.history.exec (not the track-session commit() path, which never bumps editGen, so coverage/chord/lint memos would go stale). pitchPreserving so it passes the read-only-roll edit lock; the drum variant is songScope like every drum command.
  • Time model (_regionRemapPure). A move preserves musical position, not wall-clock seconds — each note's whole interval remaps through the one tempo map (newTime = timeOf(beatOf(oldTime) + dBeat), sustain via its end-beat), mirroring TempoMapCmd. On a steady-tempo song this is just a clean shift; across a tempo change the part re-locks to the destination grid and a sustained note keeps its beat-length. Fast path: a constant/absent tempo collapses to one bit-exact seconds shift (no beatOf∘timeOf drift). dBeat === 0 is a no-op. Rollback restores a verbatim snapshot (the beat round trip is not bit-reversible), never inverts. (Move semantics reconciled with the rhythm-meter design lane against the seconds-primary reality; audio regions stay constant-dtime for the audio-region PR.)
  • Bounded vs default. A bounded region rides its window startBeat with the move (and restores, incl. a move that re-sorts it past a neighbour in regions[]); the implicit default full-span region leaves regions[] untouched, carrying the move purely in its content — so untouched packs stay byte-identical.
  • Drag wiring in parts-view.js: mousedown on a hit block arms the drag (transcription rows only — audio blocks still just select, pending the audio-region PR), a bar-snapped dashed ghost previews the drop (Alt = free), and mouseUp commits. Routed through two host hooks so the mouse.js move/up branches stay cycle-free.

Tests & gates

  • tests/region_move.test.mjs (12): the pure time model (constant/varying tempo, fast path, snap), MoveRegionCmd exec→rollback deep-equality→redo for notation and drums, the bounded-region startBeat ride + restore, zero-delta no-op, and the real S.history.exec/undo/redo path.
  • JS 298/0, lint 0 errors / 3 baseline warnings, routes.py untouched (no pytest).

Runtime-verified

Served through the real host and driven with Playwright (AC/DC — Back In Black, 9 tracks): dragging the Lead block commits the move (Moved "region:1" later), only the Lead track moves (Rhythm/Bass untouched), it snaps to bar lines (a sub-bar drag on this tempo-mapped song correctly no-ops), and Undo/Redo revert/re-apply exactly.

Follow-up (not this PR)

The "Add Track from File" import driver (drums-only GP into an existing project) is blocked on a product call: drums are a single session-wide S.drumTab — a project can hold only one drum part, so a new drum track isn't representable without extending the model to multiple drum tabs. Non-drum parts already add as N coexisting tracks. Flagged separately for the maintainer.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in: 43 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: 3e01ffac-485c-49ff-ac40-5d1aad8a7018

📥 Commits

Reviewing files that changed from the base of the PR and between 25a3403 and dc4fa1b.

📒 Files selected for processing (8)
  • CHANGELOG.md
  • src/host.js
  • src/main.js
  • src/mouse.js
  • src/parts-view.js
  • src/region-commands.js
  • src/region.js
  • tests/region_move.test.mjs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch track-regions-p3

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.

❤️ Share

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

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@byrongamatos
byrongamatos merged commit 04ef545 into main Jul 21, 2026
3 of 4 checks passed
@byrongamatos
byrongamatos deleted the track-regions-p3 branch July 21, 2026 11:34
ChrisBeWithYou pushed a commit that referenced this pull request Jul 21, 2026
…t the active tab

A song can hold several drum parts now (#339): each type:"drums"
arrangement owns its .drumTab and S.drumTab is only the ACTIVE grid
target. The region commands and the parts-view drag arming predate that
(#334) — kind:'drums' always operated on S.drumTab, and a materialized
extra part ('drums-2'…) armed as 'notation' over its empty shell
notes[], so dragging its block moved nothing (or, for the primary while
another part was active, moved the WRONG part's hits).

Resolution now mirrors the lane silhouette: arrIdx >= 0 names the part
whose track carries the region → its own .drumTab; arrIdx < 0 stays the
legacy unmaterialized S.drumTab. MoveRegionCmd delegates to the shared
_contentList, and parts-view arms kind:'drums' for any drum-arrangement
row. Tests drive a NON-active part and pin that the active tab never
moves, plus the legacy no-arrIdx fallback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix
ChrisBeWithYou added a commit that referenced this pull request Jul 22, 2026
…as a placed, movable region (#344)

* feat(editor): track regions — place + delete commands (the import-region foundation)

PlaceRegionCmd drops freshly-added track content onto the timeline as a
BOUNDED, selected, draggable block: it slides the content so its first
onset lands on the snapped startBeat (a musical move — beats preserved on
a varying grid, mirroring MoveRegionCmd's remap with the constant-tempo
fast path) and writes an explicit-length region covering it. Even a
bar-1 placement stays bounded — never the implicit default — so the
block is a distinct addressable object from birth. DeleteRegionCmd
removes exactly the notes/hits a region's end-exclusive window owns plus
the region entry, leaving neighbours untouched.

Both round-trip: rollback restores a verbatim content snapshot (the
beatOf∘timeOf round trip is not bit-reversible), the track's raw
`regions` key (including deleting one that was never there), and the
prior selection. Node-runnable with no DOM, like the move command.

_nextRegionIdPure allocates the next free region:N, counting the
implicit default region:1 even when unpersisted so a placed region can
never collide with it.

This is the command layer for "Add Track from File → placed region"
(TRACK-REGIONS-DESIGN.md PR 3); the import front door wires in next.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix

* fix(editor): region commands act on the drum part the track names, not the active tab

A song can hold several drum parts now (#339): each type:"drums"
arrangement owns its .drumTab and S.drumTab is only the ACTIVE grid
target. The region commands and the parts-view drag arming predate that
(#334) — kind:'drums' always operated on S.drumTab, and a materialized
extra part ('drums-2'…) armed as 'notation' over its empty shell
notes[], so dragging its block moved nothing (or, for the primary while
another part was active, moved the WRONG part's hits).

Resolution now mirrors the lane silhouette: arrIdx >= 0 names the part
whose track carries the region → its own .drumTab; arrIdx < 0 stays the
legacy unmaterialized S.drumTab. MoveRegionCmd delegates to the shared
_contentList, and parts-view arms kind:'drums' for any drum-arrangement
row. Tests drive a NON-active part and pin that the active tab never
moves, plus the legacy no-arrIdx fallback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix

* feat(editor): the drum import lands as a placed, selected region (Add Track from File)

The R3b payoff: importing a drum part into an existing project now
surfaces it in the Tracks view as a SELECTED region, ready to drag. The
Add-Drums dialog gains an inline "Place at" choice — Keep source timing
(default: the file's own timing, no content motion, no persisted
window), Bar 1, or Playhead — the latter two sliding the whole part
there as one undoable PlaceRegionCmd (bar-snapped, beat-preserving).

placeImportedPartAsRegion (track-session.js) is the orchestrator: it
re-normalizes the session so the fresh part's row exists (the import
just created the part — its row is synthesized here), resolves the
track from the part's arr:<idx> target, then places or just selects.
The import flow (arrangement.js) reaches it through the host table —
track-session imports arrangement, so the call inverts through the
usual seam. _placeAtStartBeatPure (region.js) resolves the dialog
choice to a startBeat, converter-free like the other region pures.

Create mode keeps replace semantics: the Place-at row hides, the choice
is forced to 'keep', and with no materialized arrangement the hook
no-ops gracefully.

Tests drive the real orchestrator: row synthesis + placement + selection
+ undo restore, keep-writes-no-window, playhead snapping, and the pure's
resolution table.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix

* feat(editor): Delete removes the selected region block (content + window, undoable)

Del/Backspace in the Tracks view deletes the selected region as one
DeleteRegionCmd: the notes/hits its end-exclusive window owns plus the
window entry, neighbours untouched. parts-view owns the resolution
(same kind/arrIdx logic as the drag arming, so a drum part's region
always deletes ITS OWN hits, never the active grid's) and input.js's
Delete ladder asks it first through the host table — transcription rows
only; audio regions stay select-only until the audio-region PR. Falls
through cleanly outside the Tracks view / with nothing selected.

Tests drive the real handler: delete + undo round-trip, the gate table,
and the non-active-drum-part case.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix

* fix(editor): route region-delete inside the Tracks-overview key gate

Runtime verification caught the Del key doing nothing in the Tracks
view: onKeyDown's partsViewMode gate returns for every key except the
parts toggle, so the region-delete branch further down the Delete
ladder was unreachable on the only surface it serves. The gate itself
now offers Delete/Backspace to host.partsViewRegionDelete() before
ignoring the key — a region op on this surface, not a note-edit on the
hidden chart — and the dead ladder branch is folded into it.

Verified end-to-end via Playwright against the real host (import a
drums MIDI → Place at Bar 1 → region:2 lands selected → drag commits
"Moved region:2 later" → Delete commits "Deleted region region:2 and
its notes" → undo restores).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix

* Harden imported region placement commands

* Support legacy drum region placement

---------

Co-authored-by: ChrisBeWithYou <chris@rifflarr.local>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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