feat(editor): handshape / arpeggio region authoring (E2, PR-B) - #10
Merged
Merged
Conversation
PR-B of E2: the span-lane authoring UI, built on PR-A's load/serialize/ chord_id-remap foundation. Lets the user draw, move, resize, retarget and delete handshape (chord-shape / arpeggio framing) regions directly in the Arrangement Editor. Modelled on the existing anchor lane, adapted from point markers to time spans: - Layout: HS_LANE_H strip below the anchor lane (_handshapeLaneTopY); the LANE_H math reserves it alongside the anchor lane (keeps the max(30,…) floor so note lanes aren't starved). - Draw: drawHandshapeLane / _drawHandshapeBar — horizontal bars from start_time..end_time, coloured by arp (arpeggio vs held shape), labelled with the covered template's name; selection highlight. - Interactions: drag empty lane = create span (arp defaults true); click = select; drag body = move; drag edges = resize; Del = remove; right-click = toggle arp / pick the covered template. Commands AddHandshapeCmd, RemoveHandshapeCmd, MoveHandshapeCmd, ResizeHandshapeCmd, ToggleHandshapeArpCmd, SetHandshapeChordCmd run through S.history. - chord_id for a new span resolves from the voicing under it (_handshapeSpanFrets): a same-time chord's frets, else the span's single notes combined (arpeggio). AddHandshapeCmd find-or-creates the matching template, tail-appended + LIFO-removed on rollback so other handshapes' live chord_id indices stay stable. reconstructChords() remaps everything to the rebuilt indices on save (PR-A). - State/routing: S.handshapeSel + S.drag.type='handshape', mouse/contextmenu routing + Delete wiring mirroring the anchor lane; S.handshapeSel cleared wherever tone/anchor selection activates (mutual exclusion). Robustness (Codex preflight, 3 rounds): - remapHandshapeChordIds mutates chord_id in place (preserves object identity) so undo/redo refs survive a save's reconstruct. - SetHandshapeChordCmd resolves templates by fret-pattern key (survives the save-time template rebuild) with the index as fallback. Tests: tests/handshape_authoring.test.js (new) extracts the real _handshapeSpanFrets + the pure remap helpers and drives a full authoring → reconstruct round-trip (held + arpeggio spans stay backend-valid; orphan template appended for the arpeggio). chord_relink.test.js updated for the in-place remap. Verified end-to-end in a real browser (native core + Playwright on /v2): loaded a real pack, drew an arp span on the lane, saved, and confirmed the authored arp handshape + the loaded set serialized validly with 0 JS errors. Closes #5 (with PR-A). Part of got-feedBack/feedBack#334. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
byrongamatos
added a commit
that referenced
this pull request
Jun 20, 2026
Post-merge Codex review of the E2 span-lane authoring (PR #10) surfaced a P1 plus four P2/P3 edge cases. Fix all: - P1: drawing a span over empty bars created a handshape with chord_id=0 (an unrelated/invalid template). onHandshapeLaneMouseUp now requires _handshapeSpanFrets() to resolve a voicing before creating. - P2: a tempo/drum mode toggle mid-drag dropped an in-flight handshape edit with no history command. _finalizeActiveDrag now commits it via onHandshapeLaneMouseUp. - P2: deleting a selected handshape mid-drag left S.drag dangling -> clear it. - P2: AddHandshapeCmd tracked its template by object identity, so undo/redo across a save's reconstruct could orphan/duplicate templates. exec() now resolves/appends by fret-key (no redo duplicate); rollback() removes by object identity (so a save, which replaces template objects, isn't wrongly undone -> won't splice a template now backing a real chord). - P3: zero-/negative-length handshapes dropped on load (end_time > start_time). - P3: reconstructChords clears S.handshapeSel if the remap drops the selected handshape from the current arrangement. Codex re-reviewed twice (a first fix introduced a rollback regression, now resolved); final pass clean. node --check + both JS suites pass (10/10, 4/4). Part of got-feedBack/feedBack#334. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR-B of E2 — the span-lane authoring UI. Rebased onto
mainafter PR-A (#8) merged (supersedes the auto-closed #9, which lost its base branch when #8 was merged with--delete-branch).Lets the user draw, move, resize, retarget and delete handshape (chord-shape / arpeggio framing) regions in the Arrangement Editor. Modelled on the existing anchor lane, adapted from point markers to time spans.
What this PR adds
HS_LANE_Hstrip below the anchor lane (_handshapeLaneTopY);LANE_Hmath reserves it (keeps themax(30,…)floor so note lanes aren't starved).drawHandshapeLane/_drawHandshapeBar: barsstart_time..end_time, coloured byarp, labelled with the covered template; selection highlight + live create-preview.arpdefaults true); click = select; drag body = move; drag edges = resize;Del= remove; right-click = toggle arp / pick covered template. CommandsAdd/Remove/Move/Resize/ToggleArp/SetChord HandshapeCmdviaS.history._handshapeSpanFrets: same-time chord, else combined single notes = arpeggio).AddHandshapeCmdfind-or-creates the template, tail-appended + LIFO-removed on rollback so live indices stay stable;reconstructChords()remaps on save.S.handshapeSel+S.drag.type='handshape', routing + Delete mirroring the anchor lane;S.handshapeSelcleared wherever tone/anchor selection activates.Robustness (Codex preflight, 3 rounds, converged — no P1s)
remapHandshapeChordIdsmutateschord_idin place (identity preserved → undo/redo survives a save's reconstruct).SetHandshapeChordCmdresolves templates by fret-pattern key (survives the save-time rebuild), index fallback.Tests / verification
tests/handshape_authoring.test.js(new) drives a full authoring → reconstruct round-trip on the real extracted_handshapeSpanFrets+ pure remap helpers. 14/14 pass;node --checkclean./v2): loaded a real pack (289 existing handshapes round-tripped), drew an arpeggio span, saved, confirmed the authoredarp=truehandshape serialized validly — 0 JS errors.Closes #5. Part of got-feedBack/feedBack#334.
🤖 Generated with Claude Code