feat(editor): heal uneven beat spacing — repair degenerate interior beats - #261
Merged
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 (5)
📝 WalkthroughWalkthroughAdds a Tempo/Grid action that detects uneven interior beat spacing, heals affected measures without moving barlines or note timing, records the change as one undoable step, and exposes the action through the editor menu and window API. ChangesGrid healing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Menu as Tempo/Grid menu
participant Editor as editorHealGrid
participant Grid as _gridHealScanPure and _gridHealPure
participant History as TempoGridCmd
Menu->>Editor: invoke editorHealGrid
Editor->>Grid: scan and heal current beat grid
Grid-->>Editor: healed grid or null
Editor->>History: commit one undoable grid update
History-->>Editor: redraw and update status
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/tempo.js (1)
1000-1006: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRedundant re-scan just to get a count.
_gridHealScanPurealready ran once inside_gridHealPure; calling it again on Line 1005 just for.lengthduplicates the O(beats) scan. Harmless at typical grid sizes, but could be avoided by having the heal helper return the sick-measure list alongside the healed grid.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tempo.js` around lines 1000 - 1006, Update _gridHealPure and its caller to return or expose the sick-measure list alongside the healed grid, then use that result to obtain the count instead of calling _gridHealScanPure again. Preserve the existing no-healing behavior and TempoGridCmd execution in the surrounding heal flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tempo.js`:
- Around line 992-999: Reorder the guard clauses in the healing flow so the
!S.sessionId || !S.history check runs before the !S.beats || S.beats.length < 2
check. Preserve both existing status messages and return behavior, ensuring no
open song reports the song-open message first.
---
Nitpick comments:
In `@src/tempo.js`:
- Around line 1000-1006: Update _gridHealPure and its caller to return or expose
the sick-measure list alongside the healed grid, then use that result to obtain
the count instead of calling _gridHealScanPure again. Preserve the existing
no-healing behavior and TempoGridCmd execution in the surrounding heal flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 06bbc48c-c976-4c88-a37e-efdd0985b4f3
📒 Files selected for processing (5)
CHANGELOG.mdsrc/main.jssrc/menu-bar.jssrc/tempo.jstests/grid_heal.test.mjs
Collaborator
|
@coderabbitai review |
✅ Action performedReview finished.
|
Collaborator
|
@coderabbitai full review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 9 minutes. |
Field projects carry measures whose sub-beats pile milliseconds apart beside a seconds-wide hole (hand re-syncs / old imports). New pures scan (gap <30% or >300% of even spacing) and re-space sick measures' interiors evenly; downbeats never move; one TempoGridCmd so notes keep seconds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
Two holes in the heal, both on the data-loss side: - A measure whose even spacing lands under the grid's millisecond resolution was re-spaced with _r3, collapsing two beats onto the SAME time — a duplicate-time grid is worse than the sickness it replaced (beatOf/timeOf stop being inverses across a zero-width gap). Reachable in exactly the pathological grids this action targets. The scan now skips those measures: corruption past what an even re-space can fix. - The scan cannot tell a corrupt pile-up from a deliberate grand pause held ~9x its neighbours — both are a wildly uneven interior gap, and the heal flattens either. Undo restores, but only if the charter can SEE which bars moved, so the status line now names them instead of only counting them. Also: check the session before the grid (the row is ungated, so "no song open" must say so, not blame a grid that cannot exist yet) and scan once instead of twice. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An agent worktree symlinked node_modules; .gitignore lists only node_modules/ (trailing slash), which matches a directory but not a symlink, so git add -A tracked it. The symlink points at a local absolute path and would break any other checkout.
byrongamatos
force-pushed
the
feat/editor-grid-heal
branch
from
July 14, 2026 19:04
b868e00 to
49e9be6
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
Motivated by a tester's real project (the same one behind #258): its grid carried measures whose interior sub-beats piled 5 ms apart right before the next barline, leaving a 2 s hole after it — the residue of hand re-syncs chasing tempo drift. That shape garbles the metronome, snapping, and every per-beat view even though the barlines themselves are where the charter put them.
New Tempo/Grid ▸ Heal uneven beat spacing:
_gridHealScanPureflags a measure when any interior gap is under 30% or over 300% of the measure's even spacing._gridHealPurere-spaces ONLY sick measures' interior beats evenly between their downbeats. Downbeats never move — barlines are the charter's authored truth; the beats between them are bookkeeping.editorHealGridcommits it as oneTempoGridCmd— notes keep their exact seconds (they were synced to the recording; the grid was the sick part) and re-lift their beat positions from the healed grid. Healthy grid → "nothing to heal", no command.Tests
tests/grid_heal.test.mjs(5, fail on main): the field pile-up shape (real numbers from the reporting project) flags and heals to exact even quarters with downbeats pinned and neighbours untouched; a healthy grid scans clean and heals to null; an oversized hole flags without a tiny gap beside it; non-time fields and input immutability. Full suite green, lint 0 errors,routes.pyuntouched.Live-verified on the reporting project
Insomnium — Where the Last Wave Broke: scan finds measures 48, 50, 76 (matching offline analysis of the shipped pak exactly); heal takes the grid's minimum beat gap from 5 ms → 231 ms and maximum from 2.078 s → 0.528 s, moves 0 of 1114 notes, lands as one undo step, and Ctrl+Z restores bit-exact. Zero page errors.
🤖 Generated with Claude Code
https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
Summary by CodeRabbit