feat(editor): loop edges as beat coordinates (Phase A4)

[ChrisBeWithYou]

Phase A4 — loop edges as beat coordinates

Fourth step of the workspace-shell time-model foundation (§1.6). Stacked on A2 (#135) — base is feat/editor-beat-primary; it'll retarget to main when A2 merges.

What changes

A bar/grid loop's edges are now anchored to beat coordinates — like note.beat — with their seconds as a derived cache. The loop follows the grid the same way a note does (A2):

Grid change	Notes (A2)	Loop (A4)
Tempo flex (TempoMapCmd, times move, indexing fixed)	β fixed, seconds re-derive	_loopReprojectFromBeats — β fixed, seconds = timeOf(newGrid, β) → stays on its bars
Re-index (TempoGridCmd, insert/delete sync-point, time-sig)	seconds fixed, β re-lifts	_loopReliftBeats — seconds fixed, β = beatOf(newGrid, seconds)

Free loops are unchanged — absolute seconds (D17); a grid edit never moves them.

Why this is better

• A tempo edit keeps a bar loop on the same bars instead of lossily re-snapping its seconds to the nearest new downbeats.
• Undoing a tempo edit restores the loop's position exactly. Previously the forward re-snap wasn't self-inverse (a bar loop [2,4] could resurface as [1,6] after edit+undo), which is the only reason TempoMapCmd/TempoGridCmd snapshotted beforeBarSel.

Because both operations are exact inverses, this deletes the lossy _loopRelockAfterGridChange re-snap and both beforeBarSel undo-snapshots. Net screen.js is +71/−57 despite adding the whole beat-anchored model.

How

• _setBarSel(region) — the single writer for a non-null S.barSel; it assigns and syncs β via _loopSyncBeats (bar/grid → beatOf of the edges; free → no β). Every loop-edit site (drag create/move, edge-adjust, arrow-key nudge, beat-bar span, Loop-in-3D pin, 3D-handoff restore) routes through it.
• Loop-in-3D still bakes to seconds — core setLoop takes seconds and the seconds cache is always current, so that boundary is unchanged.
• No wire/spec change — S.barSel is runtime-only editor state; β is never persisted.

Tests

• tests/loop_beats.test.js (new, 10) — _loopSyncBeats (integer/fractional β, free clears, degenerate grid = identity); _loopReprojectFromBeats (bar loop stays on its bars, free untouched, β-less safe-degrade); _loopReliftBeats (seconds fixed, β re-lifts); TempoGridCmd round-trips the loop with no snapshot.
• tests/loop_undo_mode.test.js — rewritten for the reproject mechanism: TempoMapCmd flex keeps β and undo restores exact seconds with no beforeBarSel field; free-loop-untouched; null round-trip; the 3D-handoff mode round-trip stays.
• Harness dep updates: beat_primary / loop_nudge_live / midi_tempo_import (the extracted commands/nudge now call the new helpers).
• All new A4 symbols are absent on the base, so the loop suites fail on main.

Verification

• node --check screen.js clean
• Full JS suite: 72 pass / 0 fail
• No routes.py change (no pytest); no wire/spec change

Ordering

Requires A2 (#135). A5 (beat-lock) also needs A2. Phase B waits on the feature-PR backlog draining. Design source: SONG-EDITOR-WORKSPACE-SHELL-CHARRETTE.md §1.6.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Loop edges are now anchored to beat coordinates during tempo and grid edits, keeping bar/grid loops on the same bars while seconds are updated consistently.
  • Undo/redo restores loop placement exactly for bar/grid loops; free loops remain absolute-time and unchanged.
  • Loop bar-selection behavior now stays in sync to preserve correct loop timing across interactive selection and mode transitions.
• Tests
  • Added and expanded unit tests to validate beat syncing, reprojection, relifting, and exec/rollback symmetry for both bar/grid and free loops.
  • Updated MIDI tempo import and loop nudge tests to reflect the new timing/undo behavior.
• Documentation
  • Updated the Unreleased changelog entry to describe the revised loop edge handling and undo/redo behavior.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0dca0fec-5e04-43c6-8061-cd2382c189ca

📥 Commits

Reviewing files that changed from the base of the PR and between 4eb557f and d3a2e6e.

📒 Files selected for processing (7)

• CHANGELOG.md
• screen.js
• tests/beat_primary.test.js
• tests/loop_beats.test.js
• tests/loop_nudge_live.test.js
• tests/loop_undo_mode.test.js
• tests/midi_tempo_import.test.js

---

📝 Walkthrough

Walkthrough

Loop edges for bar/grid selections now track beat coordinates, with seconds derived from tempo/grid changes. _setBarSel centralizes bar selection updates, the old relock path is removed, command exec/rollback now reproject or relift beats, and tests plus changelog were updated.

Changes

Beat-anchored loop edges

Layer / File(s)	Summary
Beat-cache helpers and bar selection setter screen.js	Adds _setBarSel and loop beat-cache helpers, removing _loopRelockAfterGridChange.
Wiring _setBarSel into gesture and pending-view code paths screen.js	Routes loop drag, bar-span selection, and pending-view barSel updates through _setBarSel.
TempoMapCmd/TempoGridCmd exec and rollback updates screen.js	Replaces beforeBarSel snapshot handling with beat reprojection/relifting in grid and tempo command flows.
Test harness updates for helper injection tests/beat_primary.test.js, tests/loop_nudge_live.test.js, tests/midi_tempo_import.test.js	Updates injected helper lists and stubs to use the new loop helper functions and _setBarSel.
New loop_beats test suite tests/loop_beats.test.js	Adds helper extraction and assertions for beat syncing, reprojection, relifting, and TempoGridCmd round-tripping.
Rewritten loop_undo_mode test suite tests/loop_undo_mode.test.js	Reworks undo/redo tests around real loop logic, exact rollback, free-loop behavior, and null selection handling.
Changelog documentation CHANGELOG.md	Documents the new beat-anchored loop behavior and removed relock workaround.

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

Possibly related PRs

• got-feedBack/feedBack-plugin-editor#92: Both PRs modify loop relocking behavior in TempoMapCmd/TempoGridCmd for bar/grid loops.
• got-feedBack/feedBack-plugin-editor#127: Both PRs modify TempoGridCmd behavior in screen.js.
• got-feedBack/feedBack-plugin-editor#133: The new loop beat syncing helpers depend on the beat-converter introduced there.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: loop edges now use beat coordinates.

✨ 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 feat/editor-loop-beats

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.44.1)

screen.js

ast-grep timed out on this file

---

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

[byrongamatos]

@coderabbitai review

[coderabbitai]

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