From 7c0142c62485b442fabf3bdb8d0300fd6f00e280 Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Tue, 14 Jul 2026 07:55:39 -0500 Subject: [PATCH 1/4] Make resnap an explicit quantize: works with Snap off, snaps both edges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit snapTime gains a force arg (explicit verbs bypass the live-placement toggle; the snap VALUE/mode still apply — default unchanged for every interactive caller). New snapGuidelineAfter supplies the end-edge minimum. _resnapEdgesPure snaps starts AND sustained end edges to the current-subdivision guidelines (piano-roll model), never collapsing a sustained note or inflating a chip; one undoable composite step and an honest status line either way. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q --- CHANGELOG.md | 11 ++++ src/input.js | 59 +++++++++++++++++++-- src/loop.js | 25 +++++++-- tests/resnap_explicit.test.mjs | 96 ++++++++++++++++++++++++++++++++++ 4 files changed, 184 insertions(+), 7 deletions(-) create mode 100644 tests/resnap_explicit.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 16628916..d6eaf94c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 shift back to zero removes it from the pack again, so unshifted songs stay byte-identical to before. - **Inspector technique edits are undoable now.** Toggling a technique flag +- **Resnap selection now works with Snap toggled off — and snaps both edges.** + "Resnap selection to grid" (Edit menu / its shortcut) honoured the live Snap + toggle, so with snapping off it silently moved nothing — which read as the + feature not existing at all ("I miss a way to snap the selected notes to the + grid"). An explicit quantize now always snaps, using whatever **subdivision + you have selected** — the same guidelines the grid draws, like the piano-roll + grid in a DAW. And it snaps **both edges**: note starts quantise to the + nearest guideline, and a sustained note's end edge follows — never collapsing + onto its start (it keeps at least one subdivision), while zero-length chips + are never inflated. One undoable step, and the status line now always tells + you what happened, including "already on the grid." (Palm Mute, Hammer-On, Tap, …) or setting a bend/slide value from the inspector panel used to mutate the note in place with no undo — so Ctrl+Z couldn't take it back, even though the same toggle from the keyboard could. diff --git a/src/input.js b/src/input.js index 5eb3c3a7..baff5f56 100644 --- a/src/input.js +++ b/src/input.js @@ -18,7 +18,7 @@ import { hitNote } from './hit-test.js'; import { _renderInspector, _selectedChordContext } from './inspector.js'; import { PIANO_LANE_H, _rollLockNotice, _rollReadOnly, isKeysArr, isKeysMode, midiToFret, midiToString, pianoLaneCount, yToMidi } from './keys.js'; import { lanes, _stringCountFor } from './lanes.js'; -import { _editorClampScrollX, _loopNudgeEdge, snapTime } from './loop.js'; +import { _editorClampScrollX, _loopNudgeEdge, snapGuidelineAfter, snapTime } from './loop.js'; import { _recState } from './midi-record.js'; import { getMousePos } from './mouse.js'; import { _resizeSustainsForDeltaPure, notes } from './notes.js'; @@ -502,22 +502,73 @@ function _editorNudgeSelectionTime(dir) { return true; } +/* @pure:resnap-edges:start */ +// Both EDGES of every note snap to the current-subdivision guidelines — the +// Logic piano-roll model: the start edge quantises to its nearest guideline, +// and a SUSTAINED note's end edge does too, except it never collapses onto +// the start (it keeps at least one subdivision — `afterFn` supplies the first +// guideline strictly after the new start). A zero-sustain chip stays a chip: +// length is authored intent, never inflated by a quantize. +export function _resnapEdgesPure(oldTimes, oldSustains, snapFn, afterFn) { + const newTimes = oldTimes.map(t => snapFn(t)); + const newSustains = oldSustains.map((sus, i) => { + if (!(sus > 0)) return sus || 0; + const end = snapFn(oldTimes[i] + sus); + const bounded = end > newTimes[i] + 1e-9 ? end : afterFn(newTimes[i]); + return bounded - newTimes[i]; + }); + return { newTimes, newSustains }; +} +/* @pure:resnap-edges:end */ + +// One undoable step for the two halves of an edge resnap — starts move, +// sustained ends re-length. Exec in order, rollback in reverse; gating +// follows the MoveNoteCmd half (same as the verb always had). +class ResnapEdgesCmd { + constructor(move, resize) { this.move = move; this.resize = resize; } + exec() { this.move.exec(); if (this.resize) this.resize.exec(); } + rollback() { if (this.resize) this.resize.rollback(); this.move.rollback(); } +} + function _editorResnapSelection() { const idxs = _editorCurrentNoteIndices(); if (!idxs.length) { setStatus('Select notes first'); return false; } const nn = notes(); const oldTimes = idxs.map(i => nn[i].time); - const newTimes = oldTimes.map(t => snapTime(t)); - for (let i = 0; i < idxs.length; i++) nn[idxs[i]].time = oldTimes[i]; + const oldSustains = idxs.map(i => Number(nn[i].sustain) || 0); + // FORCED snap: this is the explicit quantize verb, so it snaps to the + // guidelines (or onsets, in Onset mode) even while the live Snap toggle + // is OFF — with the toggle honoured it silently moved nothing, which read + // as "there's no way to snap notes to the grid" (a real tester report). + const { newTimes, newSustains } = _resnapEdgesPure( + oldTimes, oldSustains, (t) => snapTime(t, true), (t) => snapGuidelineAfter(t, true)); const dtimes = newTimes.map((t, i) => t - oldTimes[i]); + const touched = idxs.filter((_, i) => Math.abs(dtimes[i]) > 1e-9 + || Math.abs(newSustains[i] - oldSustains[i]) > 1e-9).length; + if (!touched) { + // Say so — a silent no-op is indistinguishable from a missing feature. + setStatus(`Selection already on the grid (${idxs.length} note${idxs.length === 1 ? '' : 's'} checked).`); + return true; + } const dstrings = idxs.map(() => 0); - S.history.exec(new MoveNoteCmd(idxs, dtimes, dstrings, null)); + const susIdx = [], susVals = []; + for (let i = 0; i < idxs.length; i++) { + if (Math.abs(newSustains[i] - oldSustains[i]) > 1e-9) { + susIdx.push(idxs[i]); + susVals.push(newSustains[i]); + } + } + S.history.exec(new ResnapEdgesCmd( + new MoveNoteCmd(idxs, dtimes, dstrings, null), + susIdx.length ? new ResizeSustainGroupCmd(susIdx, susVals) : null)); // Repeated resnapping is the canonical "fighting the grid" signal (charrette // §3.2): the notes keep landing off the beat, so the GRID may be the thing // that's wrong — nudge toward the Tempo tools. _signpostNote('gridFight'); host.draw(); host.updateStatus(); + setStatus(`Snapped ${touched} of ${idxs.length} note${idxs.length === 1 ? '' : 's'} to the ` + + `${S.snapMode === 'onset' ? 'detected onsets' : 'grid'} (both edges).`); return true; } diff --git a/src/loop.js b/src/loop.js index 484f832b..20ea9f72 100644 --- a/src/loop.js +++ b/src/loop.js @@ -382,18 +382,24 @@ export function editorToggleLoopRegion() { return _setLoopRegionEnabled(!S.loopE // placements; beyond it, snapTime falls back to grid snap. export const ONSET_SNAP_TOL = 0.07; -export function snapTime(t) { +// `force` bypasses the Snap ON/OFF toggle (the snap VALUE and mode still +// apply): the toggle governs live interactive placement, but an EXPLICIT +// quantize verb ("Resnap selection to grid") must snap regardless — with the +// toggle off it silently returned every time unchanged, which read as the +// command not existing at all. Default off: every interactive caller keeps +// honouring the toggle exactly as before. +export function snapTime(t, force = false) { // Onset-snap mode (charrette §1.6): when snapping is on and the target is // 'onset', prefer the nearest detected audio transient within ONSET_SNAP_TOL // — the bridge between musical time and audio-attack time for transcription // (no warp; just snap placement to the onset time). Falls back to grid snap // when no onset is near, or none is computed, so placement stays sensible. - if (S.snapEnabled && S.snapMode === 'onset') { + if ((S.snapEnabled || force) && S.snapMode === 'onset') { const onsets = (typeof _ensureOnsetsShifted === 'function') ? _ensureOnsetsShifted() : null; const near = _nearestOnsetTimePure(onsets, t, ONSET_SNAP_TOL); if (near !== null) return near; } - const sv = _editorEffectiveSnapValuePure(S.snapEnabled, SNAP_VALUES[S.snapIdx]); + const sv = _editorEffectiveSnapValuePure(S.snapEnabled || force, SNAP_VALUES[S.snapIdx]); if (!sv || S.beats.length < 2) return t; // Snap in the beat domain, then convert back (charrette §1.1): // snap = timeOf(round(beatOf(t)·subs)/subs). Sharing the converter makes the @@ -406,6 +412,19 @@ export function snapTime(t) { return timeOf(S.beats, _swingQuantizeBeatPure(beatOf(S.beats, t), subs, S.swingPct)); } +// The first grid guideline strictly AFTER `t` at the current subdivision — +// the minimum length an END-edge snap may leave a sustained note (the Logic +// piano-roll rule: edges snap to the guidelines, but a sustained note never +// collapses to zero; it keeps at least one subdivision). Same `force` +// semantics as snapTime. Identity when snapping is unavailable. +export function snapGuidelineAfter(t, force = false) { + const sv = _editorEffectiveSnapValuePure(S.snapEnabled || force, SNAP_VALUES[S.snapIdx]); + if (!sv || S.beats.length < 2) return t; + const subs = _editorSnapSubdivisionsPure(sv); + const q = Math.floor(beatOf(S.beats, t) * subs + 1e-6) + 1; + return timeOf(S.beats, q / subs); +} + /* @pure:group-time-delta:start */ // The time delta to apply to a WHOLE dragged selection. Snapping each note's // absolute time independently (`snapFn(origTime + dtRaw)` per note) quantises diff --git a/tests/resnap_explicit.test.mjs b/tests/resnap_explicit.test.mjs new file mode 100644 index 00000000..aceeab70 --- /dev/null +++ b/tests/resnap_explicit.test.mjs @@ -0,0 +1,96 @@ +/* + * Explicit resnap works with the Snap toggle OFF (tester report: "I miss a + * way to snap the selected notes to the grid" — the command existed, but + * snapTime honoured the live-placement toggle, so with Snap off the explicit + * quantize verb silently moved nothing and read as a missing feature). + * + * Pinned: snapTime's new `force` arg bypasses the ON/OFF toggle while the + * snap VALUE still applies; the default (no force) keeps honouring the + * toggle bit-exactly, so every interactive caller is unchanged. + * + * The force cases fail on main (the arg is ignored there → identity). + * Run: node tests/resnap_explicit.test.mjs + */ +import assert from 'node:assert'; + +globalThis.document = globalThis.document || { + getElementById: () => null, addEventListener: () => {}, activeElement: null, +}; +globalThis.localStorage = globalThis.localStorage || { getItem: () => null, setItem: () => {} }; +globalThis.window = globalThis.window || globalThis; + +const { snapGuidelineAfter, snapTime } = await import('../src/loop.js'); +const { _resnapEdgesPure } = await import('../src/input.js'); +const { S } = await import('../src/state.js'); + +let pass = 0, fail = 0; +function t(name, fn) { + try { fn(); pass++; console.log(' ok ' + name); } + catch (e) { fail++; console.error(' FAIL ' + name + ': ' + e.message); } +} + +// A steady 120 BPM 4/4 grid: beats every 0.5s, downbeats every 2s. +const beats = []; +for (let m = 0; m < 4; m++) for (let b = 0; b < 4; b++) { + beats.push({ time: (m * 4 + b) * 0.5, measure: b === 0 ? m + 1 : 0 }); +} +Object.assign(S, { beats, snapIdx: 0, snapMode: 'grid', swingPct: 50 }); + +t('snap OFF: interactive snapTime stays an identity (toggle honoured, unchanged)', () => { + S.snapEnabled = false; + assert.strictEqual(snapTime(1.13), 1.13); +}); + +t('snap OFF + force: the explicit quantize still lands on the grid', () => { + S.snapEnabled = false; + assert.strictEqual(snapTime(1.13, true), 1.0, 'quantised to the nearest guideline'); + assert.strictEqual(snapTime(1.38, true), 1.5); +}); + +t('snap ON: force and no-force agree (force only widens, never changes)', () => { + S.snapEnabled = true; + assert.strictEqual(snapTime(1.13), snapTime(1.13, true)); + assert.strictEqual(snapTime(1.13), 1.0); +}); + +t('force in onset mode with no audio falls back to the grid, not identity', () => { + S.snapEnabled = false; + S.snapMode = 'onset'; + assert.strictEqual(snapTime(1.13, true), 1.0, 'no onsets under node → grid fallback'); + S.snapMode = 'grid'; +}); + +// ── the guideline-after helper (the end-edge minimum) ──────────────── + +t('snapGuidelineAfter returns the first guideline strictly after t', () => { + S.snapEnabled = false; + assert.strictEqual(snapGuidelineAfter(1.0, true), 1.5, 'ON a line → the next one'); + assert.strictEqual(snapGuidelineAfter(1.13, true), 1.5, 'between lines → the next one'); + assert.strictEqual(snapGuidelineAfter(1.0), 1.0, 'unforced with snap off → identity, like snapTime'); +}); + +// ── both edges (the Logic piano-roll model) ────────────────────────── + +t('both edges land on guidelines: start quantises, the end edge follows', () => { + // start 1.13 → 1.0; end 1.63 → 1.5 → sustain 0.5 (both edges on lines). + const r = _resnapEdgesPure([1.13], [0.5], (x) => snapTime(x, true), (x) => snapGuidelineAfter(x, true)); + assert.strictEqual(r.newTimes[0], 1.0); + assert.strictEqual(r.newSustains[0], 0.5); +}); + +t('a short sustained note never collapses — it keeps one subdivision', () => { + // start 1.13 → 1.0; end 1.23 quantises to 1.0 = the start line → the + // guard takes the NEXT guideline instead: end 1.5, sustain 0.5. + const r = _resnapEdgesPure([1.13], [0.1], (x) => snapTime(x, true), (x) => snapGuidelineAfter(x, true)); + assert.strictEqual(r.newTimes[0], 1.0); + assert.strictEqual(r.newSustains[0], 0.5, 'one subdivision, never zero'); +}); + +t('a zero-sustain chip stays a chip — length is authored intent', () => { + const r = _resnapEdgesPure([1.13], [0], (x) => snapTime(x, true), (x) => snapGuidelineAfter(x, true)); + assert.strictEqual(r.newTimes[0], 1.0); + assert.strictEqual(r.newSustains[0], 0, 'never inflated by a quantize'); +}); + +console.log(`\n${pass} passed, ${fail} failed`); +process.exit(fail ? 1 : 0); From 748ec0910dbd82e0988ac10884a7a8d5524c6623 Mon Sep 17 00:00:00 2001 From: byrongamatos Date: Tue, 14 Jul 2026 19:42:17 +0200 Subject: [PATCH 2/4] Guard the end-edge snap against eating a sustain; restore the orphaned Inspector changelog bullet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The end-edge bound falls back to afterFn (the first guideline after the new start) when the snapped end lands on or before it. But afterFn is the IDENTITY when there is no usable grid (fewer than two beats, or the snap value off) — and in onset mode snapFn still snaps, so both edges of a short note can land on the same onset with no guideline to push the end past it. That returned a sustain of 0: a quantize silently turning a sustained note into a chip. With no positive bound to offer, keep the authored length. The changelog entry had also swallowed the first line of the Inspector technique-undo bullet, orphaning its body onto the resnap entry. --- CHANGELOG.md | 1 + node_modules | 1 + src/input.js | 7 ++++++- tests/resnap_explicit.test.mjs | 12 ++++++++++++ 4 files changed, 20 insertions(+), 1 deletion(-) create mode 120000 node_modules diff --git a/CHANGELOG.md b/CHANGELOG.md index d6eaf94c..edc1a2cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 onto its start (it keeps at least one subdivision), while zero-length chips are never inflated. One undoable step, and the status line now always tells you what happened, including "already on the grid." +- **Inspector technique edits are undoable now.** Toggling a technique flag (Palm Mute, Hammer-On, Tap, …) or setting a bend/slide value from the inspector panel used to mutate the note in place with no undo — so Ctrl+Z couldn't take it back, even though the same toggle from the keyboard could. diff --git a/node_modules b/node_modules new file mode 120000 index 00000000..c5cf027d --- /dev/null +++ b/node_modules @@ -0,0 +1 @@ +/home/byron/Repositories/feedback-plugin-editor/node_modules \ No newline at end of file diff --git a/src/input.js b/src/input.js index baff5f56..64c6b6e7 100644 --- a/src/input.js +++ b/src/input.js @@ -515,7 +515,12 @@ export function _resnapEdgesPure(oldTimes, oldSustains, snapFn, afterFn) { if (!(sus > 0)) return sus || 0; const end = snapFn(oldTimes[i] + sus); const bounded = end > newTimes[i] + 1e-9 ? end : afterFn(newTimes[i]); - return bounded - newTimes[i]; + // afterFn is the identity when there is no usable grid (fewer than two + // beats, or the snap value is off) — and in ONSET mode snapFn still + // snaps, so a short note's two edges can land on the same onset with no + // guideline to push the end past it. A quantize must never eat a + // sustained note: with no positive bound to offer, keep the length. + return bounded > newTimes[i] + 1e-9 ? bounded - newTimes[i] : sus; }); return { newTimes, newSustains }; } diff --git a/tests/resnap_explicit.test.mjs b/tests/resnap_explicit.test.mjs index aceeab70..3677aad2 100644 --- a/tests/resnap_explicit.test.mjs +++ b/tests/resnap_explicit.test.mjs @@ -92,5 +92,17 @@ t('a zero-sustain chip stays a chip — length is authored intent', () => { assert.strictEqual(r.newSustains[0], 0, 'never inflated by a quantize'); }); +t('no usable grid: a sustained note keeps its length, never collapses to a chip', () => { + // The hole the end-edge guard has to plug: in ONSET mode snapFn still snaps + // even with no beat grid, so both edges of a short note can land on the same + // onset — and afterFn (grid-based) has no guideline to offer, so it returns + // the identity. A quantize must not silently eat the sustain. + const bothToSameOnset = () => 1.0; // snapFn: every edge → the one onset + const noGuideline = (x) => x; // afterFn: identity (no grid) + const r = _resnapEdgesPure([1.13], [0.1], bothToSameOnset, noGuideline); + assert.strictEqual(r.newTimes[0], 1.0); + assert.strictEqual(r.newSustains[0], 0.1, 'original length kept, not zeroed'); +}); + console.log(`\n${pass} passed, ${fail} failed`); process.exit(fail ? 1 : 0); From ede086faa6429c394b1673fd405385e3ce9f6c65 Mon Sep 17 00:00:00 2001 From: byrongamatos Date: Tue, 14 Jul 2026 19:43:26 +0200 Subject: [PATCH 3/4] Refuse resnap on a read-only roll out loud, instead of claiming success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The history gate already refused the verb there (its MoveNoteCmd half is not pitchPreserving), but it refuses SILENTLY — and the new status line ran unconditionally after exec, overwriting the roll's lock notice with 'Snapped N of M notes' when nothing had moved. Guard up front, the way the other roll-locked verbs in this file do. --- src/input.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/input.js b/src/input.js index 64c6b6e7..c1ff5dda 100644 --- a/src/input.js +++ b/src/input.js @@ -538,6 +538,11 @@ class ResnapEdgesCmd { function _editorResnapSelection() { const idxs = _editorCurrentNoteIndices(); if (!idxs.length) { setStatus('Select notes first'); return false; } + // The verb has always been refused on a read-only roll (its MoveNoteCmd half + // isn't pitchPreserving, so the history gate rejects it). Say so HERE: the + // gate's refusal is silent to us, and the success line below would otherwise + // overwrite the lock notice and claim notes moved when none did. + if (_rollReadOnly()) { _rollLockNotice(); return true; } const nn = notes(); const oldTimes = idxs.map(i => nn[i].time); const oldSustains = idxs.map(i => Number(nn[i].sustain) || 0); From 2f3b2dbb8ca3ca78f0854748b1bb3b28c8ba5063 Mon Sep 17 00:00:00 2001 From: byrongamatos Date: Tue, 14 Jul 2026 20:21:57 +0200 Subject: [PATCH 4/4] chore: untrack the node_modules symlink An agent worktree symlinked node_modules; .gitignore only lists node_modules/ (trailing slash), which matches a directory but not a symlink, so git add -A tracked it. The symlink pointed at a local absolute path and would break any other checkout. --- node_modules | 1 - 1 file changed, 1 deletion(-) delete mode 120000 node_modules diff --git a/node_modules b/node_modules deleted file mode 120000 index c5cf027d..00000000 --- a/node_modules +++ /dev/null @@ -1 +0,0 @@ -/home/byron/Repositories/feedback-plugin-editor/node_modules \ No newline at end of file