From 0d4f1129c86b1806eefebdb2d654411ba23a83cf Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Tue, 14 Jul 2026 11:44:28 -0500 Subject: [PATCH 1/2] =?UTF-8?q?Add=20Song=20Fit=20'Re-sync=20from=20this?= =?UTF-8?q?=20bar=20on'=20=E2=80=94=20the=20drift=20rescue=20front=20door?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourth Song Fit choice: anchors the assisted barline fit on the last downbeat at/before the playhead (_songFitResyncAnchorPure), enters Tempo Map, and runs the fit immediately so the ghost corrections show without another keypress. Pure chrome — mode entry and the fit both dispatch through registry commands (editorRunShortcutCommand). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q --- CHANGELOG.md | 10 ++++++++++ src/song-fit.js | 41 ++++++++++++++++++++++++++++++++++++++++- tests/song_fit.test.mjs | 25 ++++++++++++++++++++++--- 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67d88cd5..02be22d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 subdivision when snap is on (so the entry caret sits on a real note position), instead of seeking to the raw pixel time. Hold **Alt** while clicking for a free, un-snapped scrub. +- **Song Fit ▸ Re-sync from this bar on…** The drift rescue, right where you'd + look for it. The classic trap: you set a constant tempo from a tab, but the + band actually plays a hair slower — the chart lines up perfectly at the + start and drifts further off the deeper you get. Now park the playhead + where things stop matching, open **Song Fit**, pick **Re-sync from this bar + on…**, and the editor jumps into the Tempo Map and immediately shows its + suggested barline corrections from that bar forward as ghost markers — you + click to accept as far as it looks right. Nothing commits until you accept, + Esc dismisses, and as always the audio never moves: barlines re-fit to the + recording and your notes ride along. ### Fixed diff --git a/src/song-fit.js b/src/song-fit.js index 8487e8b7..15e5961c 100644 --- a/src/song-fit.js +++ b/src/song-fit.js @@ -19,19 +19,37 @@ export function _consequenceBadgePure(kind) { case 'shift': return 'audio stays · grid, notes & sections all move together'; case 'fit': return 'audio stays · grid rescales to the tempo · notes ride along'; case 'constant': return 'audio stays · one steady tempo · you pick whether notes ride or hold'; + case 'resync': return 'audio stays · barlines re-fit to the recording from this bar on · notes ride'; default: return ''; } } -// The three Song Fit options, each carrying its consequence badge as the hint. +// The four Song Fit options, each carrying its consequence badge as the hint. export function _songFitChoicesPure() { return [ { key: 'shift', label: 'Shift everything…', hint: _consequenceBadgePure('shift') }, { key: 'fit', label: 'Fit tempo to recording…', hint: _consequenceBadgePure('fit') }, { key: 'constant', label: 'Set constant tempo…', hint: _consequenceBadgePure('constant') }, + { key: 'resync', label: 'Re-sync from this bar on…', hint: _consequenceBadgePure('resync') }, ]; } +// The re-sync anchor: the last DOWNBEAT at or before the playhead (the user +// parked the playhead where the chart stops matching the recording — "right +// up to here, wrong after"), or the first downbeat when the playhead sits +// before bar 1. Returns the beats index, or -1 with no downbeats at all. +export function _songFitResyncAnchorPure(beats, cursorTime) { + const t = Number.isFinite(cursorTime) ? cursorTime : 0; + let anchor = -1, first = -1; + for (let i = 0; i < (beats || []).length; i++) { + const b = beats[i]; + if (!b || !(b.measure > 0)) continue; + if (first < 0) first = i; + if (b.time <= t + 1e-6) anchor = i; + } + return anchor >= 0 ? anchor : first; +} + // Open the Song Fit popover and dispatch the chosen operation. Reachable from the // tempo-map inspector "Song Fit" button; window-exposed so any surface can open it. export async function _editorSongFit() { @@ -50,6 +68,27 @@ export async function _editorSongFit() { if (choice === 'shift') { _editorShiftEverything(sessionBefore); return; } if (choice === 'fit') { _callWindow('editorSyncTempo'); return; } if (choice === 'constant') { await _songFitSetConstant(sessionBefore); return; } + if (choice === 'resync') { _songFitResync(); return; } +} + +// "Re-sync from this bar on": the drift-rescue front door (a real tester +// workflow — constant tempo set from a tab, recording actually a hair slower, +// chart right up to bar N and increasingly wrong after). Enters Tempo Map, +// anchors the assisted fit on the playhead's bar, and RUNS it immediately — +// the suggested barline corrections appear as ghost markers to click-accept +// (nothing commits until the user accepts; Esc dismisses). Pure chrome: mode +// entry and the fit both dispatch through the same registry commands the +// keyboard uses, so this stays a front door, not a second engine. +function _songFitResync() { + if (!S.tempoMapMode) _callWindow('editorRunShortcutCommand', 'toggleTempoMap'); + if (!S.tempoMapMode) return; // no grid to map — the toggle already said so + // Anchor AFTER entering the mode (entry clears the barline selection), and + // drop any multi-selection — a live multi outranks the anchor in the fit. + const anchor = _songFitResyncAnchorPure(S.beats, S.cursorTime); + if (anchor < 0) { setStatus('No barlines to re-fit — mark a barline first.'); return; } + S.tempoSel = anchor; + if (S.tempoSelMulti) S.tempoSelMulti.clear(); + _callWindow('editorRunShortcutCommand', 'tempoSuggestFit'); } // "Set constant tempo": prompt for one BPM, then reuse editorSetBPM's flatten diff --git a/tests/song_fit.test.mjs b/tests/song_fit.test.mjs index 7444493f..8cdc190c 100644 --- a/tests/song_fit.test.mjs +++ b/tests/song_fit.test.mjs @@ -17,7 +17,7 @@ */ import assert from 'node:assert'; import fs from 'node:fs'; -import { _consequenceBadgePure, _songFitChoicesPure } from '../src/song-fit.js'; +import { _consequenceBadgePure, _songFitChoicesPure, _songFitResyncAnchorPure } from '../src/song-fit.js'; let pass = 0, fail = 0; function t(name, fn) { @@ -42,11 +42,30 @@ t('_consequenceBadgePure is empty for an unknown kind', () => { }); // ── 2. _songFitChoicesPure ─────────────────────────────────────────────────── -t('_songFitChoicesPure offers exactly the three fit operations', () => { +t('_songFitChoicesPure offers exactly the four fit operations', () => { const c = _songFitChoicesPure(); - assert.deepStrictEqual(c.map(x => x.key), ['shift', 'fit', 'constant']); + assert.deepStrictEqual(c.map(x => x.key), ['shift', 'fit', 'constant', 'resync']); for (const x of c) assert.ok(x.label && x.label.length, 'each choice is labelled'); }); + +// ── 2b. The re-sync anchor (the drift-rescue entry point) ──────────────────── +t('the re-sync anchor is the last downbeat at or before the playhead', () => { + const beats = [ + { time: 0, measure: 1 }, { time: 0.5, measure: -1 }, + { time: 2, measure: 2 }, { time: 2.5, measure: -1 }, + { time: 4, measure: 3 }, + ]; + assert.strictEqual(_songFitResyncAnchorPure(beats, 3.2), 2, 'inside bar 2 → its downbeat'); + assert.strictEqual(_songFitResyncAnchorPure(beats, 2), 2, 'exactly ON a downbeat → that one'); + assert.strictEqual(_songFitResyncAnchorPure(beats, 99), 4, 'past the end → the last downbeat'); +}); + +t('a playhead before bar 1 anchors on the FIRST downbeat, and no downbeats refuses', () => { + const beats = [{ time: 1, measure: 1 }, { time: 1.5, measure: -1 }, { time: 3, measure: 2 }]; + assert.strictEqual(_songFitResyncAnchorPure(beats, 0.2), 0, 'before bar 1 → bar 1'); + assert.strictEqual(_songFitResyncAnchorPure([{ time: 0, measure: -1 }], 1), -1, 'interiors only → -1'); + assert.strictEqual(_songFitResyncAnchorPure([], 1), -1); +}); t('_songFitChoicesPure hints ARE the shared badges (single source)', () => { for (const x of _songFitChoicesPure()) { assert.strictEqual(x.hint, _consequenceBadgePure(x.key), From 7a5525b5acf76f4ab4ff1ec0698cca4c737d9ad9 Mon Sep 17 00:00:00 2001 From: byrongamatos Date: Tue, 14 Jul 2026 20:32:09 +0200 Subject: [PATCH 2/2] test(song-fit): pin the re-sync boundary contract and guard the dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two properties of "Re-sync from this bar on" carried the whole feature and neither was pinned. 1. THE BOUNDARY. "From this bar ON" is a promise about the bars BEFORE it: the drift rescue is reached for precisely when the chart is already right up to bar N, so eating any of that authored grid is the one unforgivable failure, and an off-by-one at the boundary bar is the bug this class of feature ships. The invariant does hold — structurally, since _suggestFitPure marches only downbeats >= fromIdx and _suggestApplyPure re-spaces only spans with a moved edge — but structure regresses silently. Composed over the same engine pures the accept path runs (_suggestFitPure → _suggestApplyPure) on a grid whose recording drifts 2% from bar 5. The fixture puts the recording a uniform 40ms behind the authored grid so the early bars sit where the USER authored them, not where a naive onset fit would drag them — that lag is what gives the test teeth: without it the pre-anchor region is already grid-true and the assertion passes VACUOUSLY even with a one-bar-early anchor. Mutation-checked both ways. Pins both sides: beats[0..anchor] byte-identical, and the re-fit begins at the very next beat (no off-by-one dead bar). 2. THE DISPATCH (CodeRabbit's nitpick, via this module's own convention). It asked for a mocked-state test; _songFitResync is a 6-line private dispatcher and this suite's section 3 already pins every sibling verb with source guards, so the guard follows that pattern. It covers the four things CodeRabbit named (registry mode entry, the anchor, S.tempoSel, the tempoSuggestFit dispatch) plus two it missed: - ORDER: entering Tempo Map CLEARS tempoSel, so the anchor must be taken AFTER entry and the fit must read it after it is set. Hoisting the anchor above the toggle would silently fit from a cleared selection. The source comment warned about this; nothing enforced it. - COMMITS NOTHING: re-sync is proposal-only (it shows ghosts; tempo.js owns the undoable TempoMapCmd on accept). Asserting the body never touches history.exec / TempoMapCmd / S.beats pins the undo-safety property that makes this a safe front door to the highest-blast-radius op in the editor. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/song_fit.test.mjs | 75 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 73 insertions(+), 2 deletions(-) diff --git a/tests/song_fit.test.mjs b/tests/song_fit.test.mjs index 8cdc190c..42f0f03d 100644 --- a/tests/song_fit.test.mjs +++ b/tests/song_fit.test.mjs @@ -6,8 +6,10 @@ * (the conform/rebuild flatten). The inline Offset/Sync/BPM controls are left * alone. This suite proves: * 1. _consequenceBadgePure — the shared audio/grid/notes contract copy. - * 2. _songFitChoicesPure — the three options, each hinted by its badge (the - * badge is the single source, so menu + any future surface stay in step). + * 2. _songFitChoicesPure — the four options, each hinted by its badge (the + * badge is the single source, so menu + any future surface stay in step), + * plus "Re-sync from this bar on" — its anchor, and the boundary contract + * that gives the option its name (nothing at or before the anchor moves). * 3. Source guards (fail-on-main): editorSetBPM's flatten was EXTRACTED into a * shared _editorFlattenSongToBpm so Song Fit can reach it inside Tempo Map * mode, and Song Fit's "set constant" passes an overriding message; the @@ -18,6 +20,7 @@ import assert from 'node:assert'; import fs from 'node:fs'; import { _consequenceBadgePure, _songFitChoicesPure, _songFitResyncAnchorPure } from '../src/song-fit.js'; +import { _suggestApplyPure, _suggestFitPure } from '../src/tempo-suggest.js'; let pass = 0, fail = 0; function t(name, fn) { @@ -66,6 +69,45 @@ t('a playhead before bar 1 anchors on the FIRST downbeat, and no downbeats refus assert.strictEqual(_songFitResyncAnchorPure([{ time: 0, measure: -1 }], 1), -1, 'interiors only → -1'); assert.strictEqual(_songFitResyncAnchorPure([], 1), -1); }); +// ── 2c. The re-sync BOUNDARY contract — the invariant the option is named for ─ +// "From this bar ON" is a promise about the bars BEFORE it: the drift rescue is +// reached for when the chart is already right up to bar N, so eating any of that +// authored grid is the one unforgivable failure. Composed over the same engine +// pures the accept path runs (_suggestFitPure → _suggestApplyPure), on a grid +// whose recording runs 2% slow from bar 5 — the exact drift this feature exists +// to rescue. Pins BOTH sides of the boundary: nothing at/before the anchor +// moves, and the re-fit starts at the very next beat (no off-by-one dead bar). +t('re-sync leaves every beat AT or BEFORE the anchor byte-identical', () => { + const beats = []; + for (let bar = 1; bar <= 12; bar++) { + for (let b = 0; b < 4; b++) { + beats.push({ time: +((bar - 1) * 2 + b * 0.5).toFixed(6), measure: b === 0 ? bar : -1 }); + } + } + // The recording sits a uniform 40ms behind the authored grid EVERYWHERE — so + // the early bars are where the user authored them, NOT where a naive onset + // fit would drag them — and drifts a further 2% from bar 5 (t=8s) on. That + // uniform lag is what gives this test teeth: anchoring even one bar early + // WOULD move the early bars, so the assertion below fails on an off-by-one + // instead of passing vacuously on an already-grid-true region. + const onsets = beats.map(b => ({ + t: +(b.time + 0.04 + (b.time < 8 ? 0 : (b.time - 8) * 0.02)).toFixed(6), + s: b.measure > 0 ? 1 : 0.7, + })); + + const anchor = _songFitResyncAnchorPure(beats, 9.1); // playhead parked inside bar 5 + assert.strictEqual(anchor, 16, 'a playhead inside bar 5 anchors on bar 5’s own downbeat'); + + const { proposals } = _suggestFitPure(beats, onsets, anchor); + assert.ok(proposals.length, 'the drift must produce forward corrections to rescue'); + const applied = _suggestApplyPure(beats, proposals, proposals[proposals.length - 1].i); + + assert.strictEqual(applied.length, beats.length, 'equal length — the TempoMapCmd invariant'); + assert.deepStrictEqual(applied.slice(0, anchor + 1), beats.slice(0, anchor + 1), + 'every beat at or before the anchor is untouched — "from this bar on" means exactly that'); + assert.notStrictEqual(applied[anchor + 1].time, beats[anchor + 1].time, + 'and the re-fit begins at the very next beat — the boundary is the anchor, not the next bar'); +}); t('_songFitChoicesPure hints ARE the shared badges (single source)', () => { for (const x of _songFitChoicesPure()) { assert.strictEqual(x.hint, _consequenceBadgePure(x.key), @@ -98,6 +140,35 @@ t('Song Fit + its set-constant message are wired', () => { assert.match(songFitSrc, /editorSyncTempo/, 'Fit tempo dispatches to the existing sync verb'); assert.match(songFitSrc, /editorNudgeOffset/, 'Shift keeps the ±10ms nudge arrows'); }); +t('Re-sync dispatches through the registry — mode entry, anchor, fit — and commits nothing', () => { + const at = songFitSrc.indexOf('function _songFitResync()'); + assert.ok(at >= 0, '_songFitResync must exist'); + const rest = songFitSrc.slice(at); + const body = rest.slice(0, rest.indexOf('\n}\n') + 2); + + // Chrome charter: no second engine — mode entry and the fit both go through + // the same registry commands the keyboard uses. + assert.match(body, /editorRunShortcutCommand', 'toggleTempoMap'/, 'enters Tempo Map through the registry command'); + assert.match(body, /editorRunShortcutCommand', 'tempoSuggestFit'/, 'runs the fit through the registry command'); + assert.match(body, /_songFitResyncAnchorPure\(S\.beats, S\.cursorTime\)/, 'anchors on the playhead’s own bar'); + assert.match(body, /S\.tempoSel = anchor/, 'the anchor becomes the selection the fit reads'); + // A live multi-selection outranks the anchor in _editorTempoSuggestFit's range + // branch — leaving one set would silently fit a DIFFERENT span than the bar the + // user parked on. + assert.match(body, /S\.tempoSelMulti\.clear\(\)/, 'a live multi-selection is dropped'); + + // Order trap: entering the mode CLEARS tempoSel, so the anchor must be taken + // after entry, and the fit must read it after it is set. + assert.ok(body.indexOf('toggleTempoMap') < body.indexOf('S.tempoSel = anchor'), + 'mode entry precedes the anchor — entry clears the barline selection'); + assert.ok(body.indexOf('S.tempoSel = anchor') < body.indexOf('tempoSuggestFit'), + 'the anchor is set before the fit reads it'); + + // Proposal-only: re-sync shows ghosts. The undoable command belongs to the + // accept path (tempo.js owns TempoMapCmd) — re-sync must never commit. + assert.doesNotMatch(body, /history\.exec|TempoMapCmd|S\.beats\s*=/, + 're-sync itself commits nothing — nothing is undoable-able until the user accepts a ghost'); +}); t('Song Fit revalidates the session after awaited prompts and before shift actions', () => { assert.match(songFitSrc, /const sessionBefore = S\.sessionId[\s\S]*await _editorPromptChoice[\s\S]*_sameSession\(sessionBefore\)/, 'choice prompt revalidates the session before dispatch');