feat(editor): author chord harmony fn + template voicing (§6.3.1, §6.6) - #21
Merged
Conversation
Add E1 chord-inspector authoring for the two feedpak 1.7.0 harmony annotations, mirroring the teaching-marks editor work (#16/#17): - voicing (TEMPLATE): a select in the chord inspector, undoable via the existing EditChordTemplateCmd. Added to relinkChordTemplate's carry-forward set so the save rebuild doesn't BLANK it (same E0 gotcha as name/displayName/fingers/arp). - fn (INSTANCE): rn/q/deg inputs, one undo unit (EditChordFnCmd). fn rides the chord instance, so it's keyed by chord time in a transient arr._chordFn map (one chord per time) rather than stamped on movable notes — a note dragged out of a chord can't carry a stale fn into another group. reconstructChords reads the map back and prunes orphans for times that no longer hold a chord. routes.py round-trips both: _song_to_dict hydrates chord fn + template voicing on load; _arr_dict_to_wire emits them default-omitted and range-guarded (new _chord_fn_wire mirrors core's _validate_fn — never a partial/out-of-range fn; voicing only when a non-empty string). Display only — no scoring path touched. Client-side deg auto-fill is deferred: the editor has no active-key/tonic accessor (per-note sd auto-derivation is server-side), so deg is manual 0..11. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
byrongamatos
added a commit
that referenced
this pull request
Jun 21, 2026
…#21 follow-up) (#22) Post-merge Codex review of #21 found fn desynced when a chord moved: fn was stored in a time-keyed map `arr._chordFn`, so dragging a chord (which changes its time) orphaned its fn, and a stale old-time entry could leak to a new chord there. The time key is fragile against EVERY time-mutating edit — drag, global shift, time-scale, offset, tempo remap — not just drag. Make fn ride the chord INSTANCE the robust way: carry it on the note objects (which all those edits mutate in place), and adopt a group's fn by majority on rebuild. - flattenChords: seed each chord note's `_fn` from the chord's fn (every note of the chord gets the same value) instead of the time-keyed store. - new _groupFn (pure, node-tested): a group's fn = the `_fn` carried by > half its notes, else null. Authoring writes `_fn` to all of a chord's notes (unanimous), so a real chord always keeps its fn; a lone note dragged in from another chord is outvoted and can't impose a stale fn — the exact property the time-keyed store was protecting, now without losing fn on a move. - reconstructChords: adopt fn via _groupFn; a length-1 (solo) group drops its `_fn` so the internal field can't ride into the saved wire. - _selectedChordContext: read fn via _groupFn over the time-group. - EditChordFnCmd: write `_fn` to every note at the chord's time, snapshotting prior per-note `_fn` by object ref for undo. Removed the `arr._chordFn` store. +3 _groupFn tests (move-survival, single-note-leak prevention, no-majority/empty). Codex-reviewed (2 rounds — caught the solo-note wire leak): clean. node 16/16, editor pytest 90 passed. 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.
Part of got-feedBack/feedBack#334
PR 3 of 3. Editor authoring for the per-chord harmony annotations carried by core wire (got-feedBack/feedBack#540) and drawn by the highway (got-feedBack/feedBack#541). Based on
main(the core PRs are in a separate repo, so this is not git-stacked).What
Adds E1 chord-inspector authoring for both annotations, mirroring the teaching-marks editor work (#16 / #17):
voicing(chord template, §6.6) — a<select>in the chord inspector (open/triad/shell/drop2/drop3/barre), undoable via the existingEditChordTemplateCmd. Added torelinkChordTemplate's carry-forward set so the save rebuild doesn't BLANK it (the same E0 gotcha asname/displayName/fingers/arp).fn(chord instance, §6.3.1) —rn/q/deginputs, one undo unit (EditChordFnCmd).fn instance round-trip (the tricky part)
fnrides the chord instance, but the editor flattens chords to notes during editing and rebuilds them on save. Stampingfnon the notes is unsafe — a note dragged out of a chord would carry a stalefninto another group (caught in review). Insteadfnis keyed by chord time in a transientarr._chordFnmap (there is exactly one chord per time in the reconstruct model), independent of the movable notes:flattenChordsseeds the map from loaded chords.EditChordFnCmdwrites/rolls-back the map entry (one undo unit).reconstructChordsreads the map back onto the rebuilt chord and prunes orphan entries for times that no longer hold a chord.Serialization (
routes.py)_song_to_dicthydrates chordfn+ templatevoicing._arr_dict_to_wireemits both default-omitted and range-guarded — new_chord_fn_wiremirrors core's_validate_fn(never a partial / out-of-rangefn);voicingonly when a non-empty string.Honesty rule
Display/teaching authoring only. No scoring / NoteVerifier path is touched.
Deferral: client-side
degauto-fillThe editor has no active-key/tonic accessor (per-note
sdauto-derivation is server-side), sodegis a manual0–11input, mirroring the existing per-notesdfield. Noted for a follow-up if a key accessor lands.Tested locally
python -m pytest→ 90 passed, 2 skipped (incl. newtests/test_chord_harmony.py: fn emit/omit/range-guard, voicing emit/omit/trim/non-string).node tests/chord_relink.test.js→ 13 passed (voicing carry-forward +_normChordFn/_mergeChordFn).node tests/teaching_marks.test.js+node tests/edit_history_reset.test.jsstill pass.node --check screen.jsclean._chordFncould go stale on a move) — fixed by the time-keyed map + orphan prune above; re-review clean.CI may be red on infra; the above was verified locally.
🤖 Generated with Claude Code