fix(editor): handshape authoring edge-case fixes (E2 follow-up, Codex) - #11
Merged
Merged
Conversation
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.
Follow-up to #10 (E2 span-lane handshape authoring). An independent post-merge Codex review surfaced a P1 + four P2/P3 edge cases; this fixes all six.
chord_id=0(unrelated/invalid template)onHandshapeLaneMouseUpcreate path now also requires_handshapeSpanFrets(...)to resolve a voicing; else a status hint, no handshape_finalizeActiveDragcommits ahandshapedrag viaonHandshapeLaneMouseUpS.dragdanglingS.dragwhen it points at the deleted handshapeAddHandshapeCmdtracked its template by identity → undo/redo across a save could orphan/dup templatesexec()resolves/appends by fret-key (no redo dup);rollback()removes by object identity (a save replaces template objects, so post-save it correctly leaves reconstruct-owned templates — incl. ones now backing a real chord — alone)end_time > start_time)S.handshapeSelwhen it dropped the selected handshapereconstructChordsReview
Codex re-reviewed twice: the first pass confirmed 5/6 resolved but caught a regression in the P2c fix (key-based rollback could splice a template referenced by
arr.chordsafter a save) — fixed by the exec-key / rollback-identity split above; the final pass is clean (no findings).Test
Part of got-feedBack/feedBack#334.
🤖 Generated with Claude Code