feat(editor): tempo ramps + fit-from-drift + Tempo List (P2-7) - #279
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughAdds authored tempo ramps with validation, curve compilation, drift fitting, marker rendering, and undo/redo persistence. Adds a Tempo List overlay for viewing and navigating authored tempo marks, with menu, startup, redraw, and markup-test integration. ChangesTempo authoring and inspection
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
ChrisBeWithYou
pushed a commit
that referenced
this pull request
Jul 15, 2026
…asureEnd validation (review #279 items 4-7) - item 4: _marksRemapPure remaps BOTH ramp endpoints atomically through a renumber; a ramp whose end bar was deleted or whose span collapses drops whole (never measureEnd <= measure). - item 5: editorRampRange now REPLACES-from-baseline — overlapping ramp marks are stripped and their spans neutralized (_rampNeutralizePure: endpoints held, interior uniform) before the new mark compiles, making same-range reapply bit-identical and a BPM edit equivalent to compiling against the original grid. Endpoint-touching ramps still coexist. - item 6: the ramp prompt parses strictly (_rampPromptParsePure — complete finite numbers, 0 < bpm <= 1000) and the mark is _markNormPure-validated BEFORE any compile or history mutation; invalid input leaves beats, marks, and history reference-identical. - item 7: routes.py parses measureEnd with _exact_int instead of int() — fractions/bools/strings drop instead of truncating, inf no longer crashes the load (OverflowError), at both wire boundaries. Regression suites: tests/tempo_ramp_review.test.mjs (10 of 15 cases fail pre-fix) + two pytest cases (both fail pre-fix). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
byrongamatos
added a commit
that referenced
this pull request
Jul 15, 2026
Review #279: the ramp compile is monotonic by construction inside the span (positive BPM), but nothing pinned the trailing block-shift at the span END — the exact seam where a regression would write a backwards or duplicate beat time and silently corrupt beatOf/timeOf's binary search. Add a whole-grid strict-monotonicity assertion over every curve preset, both accel and rit, plus the lock re-space path. No production change: the current compile already holds the invariant; this is the guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The expressive centerpiece: a rit/accel is ONE authored object, never N
shrinking bars painted as noisy chips.
- ramp marker kind: {measure, measureEnd, bpmStart, bpmEnd, curve} with
hard-edged validation (curve presets linear/ease-in/ease-out — a rit
defaults to ease-out; never hand-tuned beziers).
- _rampCompilePure: the span's beat times re-space along the tempo
curve (midpoint-sampled integral, beat granularity), span start
anchored, tail shifts as a block — the _tempoSetMeasureBpmPure rule.
Locks WIN via the existing _respaceWithLocksPure post-pass.
- editorRampRange (context menu on a multi-selection): ONE TempoMapCmd
carries mark + compiled grid — notes ride the reproject, undo
restores marks and beats together (TempoMapCmd gains the same marks
snapshot contract TempoGridCmd got in P2-5).
- editorRampFitFromDrift: least-squares linear ramp over the observed
per-bar onset BPMs; offered only when its residual beats a flat
reading; lands through the same verb with provenance 'detected'.
- Chips: ONE ramp chip; derived tempo chips suppressed INSIDE the span
(honest step chips at the boundaries stay).
- src/tempo-list.js + panel: the numeric ledger (UX G3) — one row per
authored mark (Bar/Type/Value/Source), click jumps to the bar;
identity-keyed render riding drawNow.
- routes.py: ramp joins the persistence vocabulary.
DEFERRED (documented): the graphical Tempo-lane strip with vertical
drag + curve-node gestures (UX D1) — it needs live drag verification
and its own PR; the engine, verbs, list, and persistence here are its
foundation.
tests/tempo_ramps.test.mjs (8 cases, fails on main): validation,
monotone accel compile + anchored start + block-shifted tail, ease-out
vs linear, ONE-chip contract, locks-win, exec/rollback/redo on marks +
beats, fit-from-drift on a synthesized rit, Tempo List rows. Backend
ramp round-trip in pytest.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
…asureEnd validation (review #279 items 4-7) - item 4: _marksRemapPure remaps BOTH ramp endpoints atomically through a renumber; a ramp whose end bar was deleted or whose span collapses drops whole (never measureEnd <= measure). - item 5: editorRampRange now REPLACES-from-baseline — overlapping ramp marks are stripped and their spans neutralized (_rampNeutralizePure: endpoints held, interior uniform) before the new mark compiles, making same-range reapply bit-identical and a BPM edit equivalent to compiling against the original grid. Endpoint-touching ramps still coexist. - item 6: the ramp prompt parses strictly (_rampPromptParsePure — complete finite numbers, 0 < bpm <= 1000) and the mark is _markNormPure-validated BEFORE any compile or history mutation; invalid input leaves beats, marks, and history reference-identical. - item 7: routes.py parses measureEnd with _exact_int instead of int() — fractions/bools/strings drop instead of truncating, inf no longer crashes the load (OverflowError), at both wire boundaries. Regression suites: tests/tempo_ramp_review.test.mjs (10 of 15 cases fail pre-fix) + two pytest cases (both fail pre-fix). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
Review #279: the ramp compile is monotonic by construction inside the span (positive BPM), but nothing pinned the trailing block-shift at the span END — the exact seam where a regression would write a backwards or duplicate beat time and silently corrupt beatOf/timeOf's binary search. Add a whole-grid strict-monotonicity assertion over every curve preset, both accel and rit, plus the lock re-space path. No production change: the current compile already holds the invariant; this is the guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
byrongamatos
force-pushed
the
feat/editor-tempo-ramps-lane
branch
from
July 15, 2026 20:35
77dd43e to
dfa4a3b
Compare
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
2 participants
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.
What
P2-7 — a ritardando is ONE thing. Select a run of barlines, right-click ▸ Ramp the range (accel/rit)…, give it
start → endBPM: the whole gesture becomes a single authoredrampobject whose span re-spaces smoothly along a curve, with notes riding and one undo restoring everything._respaceWithLocksPurepost-pass — a locked (human-verified) barline inside the span holds its exact time and the ramp compiles around it. Pinned by test showing the raw compile would have moved it.rit. 140→120; derived tempo chips are suppressed inside the span (the smooth drift is the point) while honest step-chips at the boundaries stay.detected).rampjoins the backend persistence vocabulary (same contract as feat(editor): authored marks I — hold/fermata bars, meter groupings, provenance (P2-5) #276).The one command rule
A ramp edit is ONE
TempoMapCmdcarrying{markerDelta, compiledGridDelta}—TempoMapCmdgains the same optional marks-snapshot contractTempoGridCmdgot in P2-5, so undo restores marks and beats together (pinned).Deferred (documented in the commit)
The graphical Tempo-lane strip (UX D1: derived-height points, vertical drag as sugar over the horizontal re-space, corner curve-node gesture, Snap pop-up) needs live mouse-drag verification and its own PR — this PR lands its entire foundation (engine, verbs, list, persistence). DAW divergences held: tempo points remain DERIVED, no tempo sets.
Testing
tests/tempo_ramps.test.mjs(8 cases, fails on main): validation hard-edges, monotone accel compile + anchored start + block-shifted tail, ease-out ≠ linear, the one-chip contract (inside-span silence + honest boundary steps), locks-win, exec → rollback → redo deep equality on marks + beats, fit-from-drift on a synthesized 120→90 rit (falling ramp, residual beats flat), Tempo List rows for all four kinds. Backend ramp round-trip in pytest. Full suites green: 162 files / 0 fail, pytest 268, lint at baseline.🤖 Generated with Claude Code
https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
Summary by CodeRabbit
New Features
Bug Fixes
Tests