Repository navigation
Per-section sync 1/5: guard the uniform model's do-no-harm behavior on meter-changing songs #22
Description
Part of #8. Independent; protects the shipped model before anything is built on top of it.
#8 requires that the uniform sync keep its do-no-harm guarantee: on an already-synced song, including meter-changing ones like Pink Floyd's "Money" (7/8 verses, 4/4 solo), it must return factor ~1 and offset ~0 instead of inventing a damaging global correction. Per-section sync is a bigger feature layered on this, so the guarantee should be pinned by a test first.
Scope
- Add (or confirm the existence of) a regression test where the uniform sync is run on a chart that is already aligned but changes tempo/meter mid-song, and asserts factor ~1 / offset ~0 within a stated tolerance.
- If a real audio fixture cannot be committed, build a synthetic one (e.g. a click or tone track with a mid-song tempo/meter change) and say why.
Acceptance criteria
- The test fails if the uniform model applies a non-trivial global correction to such a song.
- The tolerance and fixture are documented in the test so later sub-issues can reuse them.
Out of scope
Any per-section behavior. Changing the uniform model.
Activity
pullfrog commentedon Oct 2, 2026
cchacs commentedon Oct 2, 2026
@pullfrog please work on this issue. It is the first sub-issue of #8 and has no dependencies. Keep the PR scoped to this sub-issue only, follow the repo
CLAUDE.md/AGENTS.md, and do not start the dependent sub-issues. Open a PR againstmain; do not merge it.Generated by Claude Code
pullfrog commentedon Oct 2, 2026
Done — PR opened, unmerged: #45
What landed (tests + CHANGELOG only, no model change): a synthetic click-track fixture (128 BPM grid, 4-beat count-in so the pivot isn't at
t=0, bar length switching 7 beats → 4 mid-song, with its own matching clicks) driving the shipped Sync Tempo dialog end to end on both real onset paths — the synchronous RMS stripeditorSyncTemporeads on load, and the banded spectral flux it settles on, fed through the dialog's own manual-BPM override.What it asserts: factor within 1% of 1 (measured 1.0000 RMS / 1.0055 flux), no authored time moving more than 0.6 s (measured 0.5 ms / 296 ms), the pivot not moving at all, no invented offset, and a clean undo → exact-restore → redo round-trip. Times are read off
_eachTimed, the same walk the shipped command uses, so sustains and phrase/handshape spans are covered and the guard can't fall behind the model. A deliberate half-tempo correction through the same shipped path is asserted to blow the budget, so the tolerances aren't vacuous — and a temporaryt0 = 0mutation ineditorApplySyncwas confirmed to fail the suite (restored).Baseline pinned for #8: an already-synced song that changes tempo as well as meter, where
getTabBPM()'s mean beat interval (112.94) and the onset vote's per-beat majority (128) disagree and the factor lands 13.3% off — pinned to both numbers so the per-section sub-issues have a stable baseline to beat.Fixture rationale: no binary fixtures are committable here, and exact known tempo/meter/seconds make a failure point at the model rather than the recording. 7/8 is deliberately not the numerator — the editor's one fixed beat grid makes a 7/8 bar 3.5 beats, putting the grid on the eighth note, which the octave-folding detector can't agree with; answering that would mean changing the model, so it's left to a separate question.
Verification:
npm test→ 316/316 passed;npm run lint→ 0 errors (3 pre-existing warnings); working tree clean. Dependent sub-issues #23–#26 not started.Task list (8/8 completed)
big-pickle(free) ｜ 𝕏test: pin the uniform sync's do-no-harm guarantee on a meter change (#22