feat(editor): Map Health — per-bar grid-vs-onset drift review lens - #249
Merged
Conversation
The whole "review an automatic map" 2nd pass had no review surface until now
(P2-4). Map Health scores, per measure, how well the grid agrees with the
detected onsets and paints a thin three-state wash under the ruler, so drift is
visible wherever you chart.
New pure src/map-health.js: _mapHealthPure(beats, onsets, opts) → per-measure
{driftFrac, coverage, band} + overall. Per beat resid = |beat.time −
nearestOnset|, reported as a FRACTION of the local beat interval (25ms is
inaudible at 60bpm but half a subdivision on 200-bpm 16ths — colour by fraction,
never raw ms). Per-measure = the MEDIAN driftFrac over EVIDENCED beats (median so
one expressive off-beat note can't drag a bar red) + coverage.
THREE states, the third non-negotiable: green (agrees, <5%), amber (drifting,
5-12%), red (disagrees with PRESENT onsets, >12%), and GREY when there are no
onsets to judge (silence / sustained / held / pedaled) — NEUTRAL, never red.
Colouring an unmeasurable held bar red is crying wolf; the author learns to
ignore red, which is fatal.
Wiring (ruler.js): a ~5px wash under the beat ticks, memoized on editGen + the
onset-cache identity (no per-frame recompute), off by default, a view flag (no
history). Toggle in the Tempo/Grid menu (audio-only). Reads only S.beats +
_ensureOnsets() — so it rides #248's banded onsets once they land, and degrades
gracefully to the RMS detector meanwhile (worse onsets → more grey, never wrong).
Tests: tests/map_health.test.mjs (10) — aligned→green, offset 8%→amber (the
signature fail-on-main case), onsets-removed→grey NOT red, held-bar→grey, median
robustness (one expressive onset doesn't flag the timekeeper bar), >12%→red,
band thresholds, tempo/meter independence, degenerate input. 135 JS green, lint
0-err, routes.py untouched. Live-verified on AC/DC: 105 measures, overall green
@ 88.6% coverage, 68 green / 26 amber / 2 red / 9 grey (not crying wolf), toggle
+ persistence + stable memo, wash renders on the ruler.
FOLLOW-UP (noted): click a hot bar → jump to Tempo Map + offer the G suggest;
LCD pill; distinct-signature reporting (lay-back vs nudge vs ramp); octave
"reads 2×, halve?" one-click. Core review surface ships here.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds an optional Map Health feature that scores beat-grid alignment against recording onsets, renders per-measure status colors under the ruler, and exposes the feature through the Tempo/Grid menu. ChangesMap Health
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Editor
participant MenuBar
participant Main
participant Ruler
participant MapHealth
Editor->>MenuBar: select Map Health
MenuBar->>Main: call editorToggleMapHealth
Main->>Ruler: forward toggle
Ruler->>MapHealth: score beats against onsets
MapHealth-->>Ruler: return measure bands
Ruler-->>Editor: redraw ruler overlay
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/map_health.test.mjs (1)
103-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the exact threshold boundaries.
Add assertions for
0.05and0.12; the current representative values do not verify the documented inclusive/exclusive behavior.Proposed additions
assert.strictEqual(band(0.03), 'green'); + assert.strictEqual(band(0.05), 'amber'); assert.strictEqual(band(0.08), 'amber'); + assert.strictEqual(band(0.12), 'amber'); assert.strictEqual(band(0.15), 'red');🤖 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 `@tests/map_health.test.mjs` around lines 103 - 109, Extend the “band thresholds” test around _mapHealthPure to assert the exact boundary behavior at 0.05 and 0.12, verifying the documented inclusive/exclusive classification while preserving the existing green, amber, and red representative cases.
🤖 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/map-health.js`:
- Around line 54-58: Update _bandFor in src/map-health.js: classify driftFrac
equal to greenMax as amber while keeping red classification only for values
strictly above redMin. Add boundary assertions in tests/map_health.test.mjs for
the exact 0.05 and 0.12 thresholds, covering the documented classifications.
- Around line 79-115: The measure construction in src/map-health.js lines 79-115
must treat a terminal closing downbeat as a boundary, not a measure start:
exclude it from downs, use the next downbeat as the endpoint when present, and
extrapolate the final measure endpoint only when no boundary exists. Update
tests/map_health.test.mjs lines 26-35 to include a closing downbeat and assert
exactly bars measures with complete spans.
In `@src/ruler.js`:
- Around line 88-91: Update _drawMapHealthBand to apply the same
audio-availability gate as the menu before checking _mapHealthEnabled(), so
audio-less charts do not render inherited map-health state. Reuse the existing
audio/onset availability condition or helper used by the menu, while preserving
the current result and measures checks for eligible charts.
---
Nitpick comments:
In `@tests/map_health.test.mjs`:
- Around line 103-109: Extend the “band thresholds” test around _mapHealthPure
to assert the exact boundary behavior at 0.05 and 0.12, verifying the documented
inclusive/exclusive classification while preserving the existing green, amber,
and red representative cases.
🪄 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: 75cad3db-0b80-472d-a712-ccbdad468d61
📒 Files selected for processing (6)
CHANGELOG.mdsrc/main.jssrc/map-health.jssrc/menu-bar.jssrc/ruler.jstests/map_health.test.mjs
Comment on lines
+79
to
+115
| const downs = []; | ||
| for (let i = 0; i < beats.length; i++) if (beats[i] && beats[i].measure > 0) downs.push(i); | ||
| if (!downs.length) return empty; | ||
|
|
||
| const intervalAt = (j) => { | ||
| if (j + 1 < beats.length) return beats[j + 1].time - beats[j].time; | ||
| if (j - 1 >= 0) return beats[j].time - beats[j - 1].time; | ||
| return 0; | ||
| }; | ||
|
|
||
| const measures = []; | ||
| let cursor = 0; | ||
| for (let d = 0; d < downs.length; d++) { | ||
| const start = downs[d]; | ||
| const end = d + 1 < downs.length ? downs[d + 1] : beats.length; // exclusive | ||
| const drifts = []; | ||
| let total = 0; | ||
| for (let j = start; j < end; j++) { | ||
| if (!beats[j] || !Number.isFinite(beats[j].time)) continue; | ||
| total++; | ||
| const interval = intervalAt(j); | ||
| if (!(interval > 0)) continue; | ||
| const found = _nearestOnsetFrom(onsetTimes, beats[j].time, interval * evidenceWin, cursor); | ||
| cursor = found.cursor; | ||
| if (found.time !== null) drifts.push(Math.abs(found.time - beats[j].time) / interval); | ||
| } | ||
| const coverage = total > 0 ? drifts.length / total : 0; | ||
| const driftFrac = _median(drifts); | ||
| measures.push({ | ||
| i: measures.length, | ||
| measure: beats[start].measure, | ||
| startTime: beats[start].time, | ||
| endTime: (end < beats.length ? beats[end].time : beats[beats.length - 1].time), | ||
| driftFrac, | ||
| coverage, | ||
| band: _bandFor(driftFrac, coverage, greenMax, redMin, minCoverage), | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align measure construction with the closing-downbeat contract. The implementation treats the final boundary as a measure, while the test fixture omits that production shape entirely.
src/map-health.js#L79-L115: exclude a terminal closing downbeat from measure starts and extrapolate the last measure endpoint only when no boundary exists.tests/map_health.test.mjs#L26-L35: include a closing downbeat and assert the result still contains exactlybarsmeasures with complete spans.
📍 Affects 2 files
src/map-health.js#L79-L115(this comment)tests/map_health.test.mjs#L26-L35
🤖 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/map-health.js` around lines 79 - 115, The measure construction in
src/map-health.js lines 79-115 must treat a terminal closing downbeat as a
boundary, not a measure start: exclude it from downs, use the next downbeat as
the endpoint when present, and extrapolate the final measure endpoint only when
no boundary exists. Update tests/map_health.test.mjs lines 26-35 to include a
closing downbeat and assert exactly bars measures with complete spans.
…inal bar Review fixes on the Map Health lens: - HIGH: the lens compared chart-time beats against BUFFER-time onsets (_ensureOnsets). Any non-zero S.audioShift made the whole map read red. Consume _ensureOnsetsShifted() — the chart-time view, which is what every other beat-vs-onset consumer uses. - HIGH: _ensureOnsetsShifted() reallocates on every call when the shift is non-zero, so keying the memo on it would miss every frame and put the O(bars x beats) scan back on the draw path. Key on the RAW cache identity plus the shift scalar instead (and invalidate on a shift change, which the old key could not see at all). - MED: the unclosed final measure ended AT the last beat, clipping the last bar's wash a beat short and collapsing a grid that ends on a closing downbeat to zero width. Extrapolate one beat past the last beat, keeping the canonical _tempoMeasures rule that every downbeat starts a measure. - LOW: the persisted flag let an audio-less chart paint an all-grey strip with no menu row to switch it off. Gate the draw on S.audioBuffer, the same gate as the audioOnly menu row (CodeRabbit). - LOW: _bandFor made exactly greenMax green, contradicting its own header (green is strictly under greenMax) (CodeRabbit). - Toggling on now reports through setStatus like every sibling toggle, and says so explicitly when there are no transients to judge against — an all-grey strip is honest but looks broken when it is silent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/map_health.test.mjs (1)
181-192: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMemo test implicitly depends on
S.beatsleft over from the previous test.Unlike the preceding seam test (lines 173-174), this test never assigns
S.beatsbefore its first_mapHealthResults()call — it relies on the 8-element array set by the prior test still being in module state. If this test is run in isolation or reordered,S.beatswon't be that populated array, andS.beats.map(...)at Line 187 will throw ifS.beatsisnull/undefinedby default (or silently exercise a degenerate empty-grid path instead of the intended populated-grid identity check).♻️ Proposed fix: make the grid setup explicit
t('the memo survives a non-zero audio shift (no per-frame recompute on the draw path)', () => { seedPeaks(); S.audioShift = 0.2; + S.beats = []; + for (let i = 0; i < 8; i++) S.beats.push({ time: 0.4 + i * 0.5, measure: i % 4 === 0 ? (i / 4) + 1 : 0 }); const a = _mapHealthResults(); const b = _mapHealthResults();🤖 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 `@tests/map_health.test.mjs` around lines 181 - 192, Initialize S.beats explicitly with the populated test grid before the first _mapHealthResults() call in the memo test. Keep the existing audio-shift and grid-rebuild assertions unchanged so the test remains isolated and verifies identity changes on a valid grid.
🤖 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/ruler.js`:
- Around line 92-101: Update the status-message logic in the ruler update block
around _mapHealthResults and setStatus to distinguish missing audio, missing or
insufficient beat-grid data in S.beats, and genuinely zero detected transients.
Preserve the existing coverage-based message for positive coverage, but only
report “no transients detected” when an audio buffer and usable beat grid exist;
otherwise use status text that accurately identifies the unavailable input.
---
Nitpick comments:
In `@tests/map_health.test.mjs`:
- Around line 181-192: Initialize S.beats explicitly with the populated test
grid before the first _mapHealthResults() call in the memo test. Keep the
existing audio-shift and grid-rebuild assertions unchanged so the test remains
isolated and verifies identity changes on a valid grid.
🪄 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: 5b7d73b2-27c1-41ec-aa55-1b162b1fda49
📒 Files selected for processing (3)
src/map-health.jssrc/ruler.jstests/map_health.test.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/map-health.js
Comment on lines
+92
to
+101
| // An all-grey strip is the honest render of "no transients to judge against", | ||
| // but silently. Say so — otherwise the lens looks broken rather than blank. | ||
| if (next) { | ||
| const cov = _mapHealthResults().overall.coverage; | ||
| setStatus(cov > 0 | ||
| ? 'Map Health on — per-bar grid-vs-recording drift under the ruler (green agrees, red disagrees, grey = nothing to judge; review only)' | ||
| : 'Map Health on — no transients detected in the recording yet, so every bar reads grey (nothing to judge)'); | ||
| } else { | ||
| setStatus('Map Health off'); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Status message conflates "no audio" / "no beat grid" / "no transients found" into a single condition.
cov > 0 is the only branch condition, but coverage is also 0 when there's no audio buffer (_ensureOnsets() returns null) or no beat grid (S.beats is null/too short) — not just when onsets genuinely weren't detected. The draw path already treats !S.audioBuffer as a distinct case (Line 112); the status text should do the same instead of always claiming "no transients detected in the recording yet."
Proposed fix
if (next) {
- const cov = _mapHealthResults().overall.coverage;
- setStatus(cov > 0
- ? 'Map Health on — per-bar grid-vs-recording drift under the ruler (green agrees, red disagrees, grey = nothing to judge; review only)'
- : 'Map Health on — no transients detected in the recording yet, so every bar reads grey (nothing to judge)');
+ const cov = S.audioBuffer ? _mapHealthResults().overall.coverage : 0;
+ setStatus(cov > 0
+ ? 'Map Health on — per-bar grid-vs-recording drift under the ruler (green agrees, red disagrees, grey = nothing to judge; review only)'
+ : S.audioBuffer
+ ? 'Map Health on — no transients detected in the recording yet, so every bar reads grey (nothing to judge)'
+ : 'Map Health on — no recording loaded, so every bar reads grey (nothing to judge)');
} else {🧰 Tools
🪛 ast-grep (0.44.1)
[error] 95-97: React's useState should not be directly called
Context: setStatus(cov > 0
? 'Map Health on — per-bar grid-vs-recording drift under the ruler (green agrees, red disagrees, grey = nothing to judge; review only)'
: 'Map Health on — no transients detected in the recording yet, so every bar reads grey (nothing to judge)')
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 99-99: React's useState should not be directly called
Context: setStatus('Map Health off')
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
🤖 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/ruler.js` around lines 92 - 101, Update the status-message logic in the
ruler update block around _mapHealthResults and setStatus to distinguish missing
audio, missing or insufficient beat-grid data in S.beats, and genuinely zero
detected transients. Preserve the existing coverage-based message for positive
coverage, but only report “no transients detected” when an audio buffer and
usable beat grid exist; otherwise use status text that accurately identifies the
unavailable input.
# Conflicts: # CHANGELOG.md
byrongamatos
added a commit
that referenced
this pull request
Jul 13, 2026
This branch predates the fixes that landed with its parent, and git would
merge the two cleanly while silently reverting them. Reconciled by hand:
· ruler.js keeps main's _ensureOnsetsShifted() + the audio-shift memo key
(taking this branch's side would restore the time-base bug that paints
the whole map red on any slid recording, and re-arm the per-frame scan)
· map-health.js keeps main's extrapolated final-measure endTime, and
regains this branch's beatIdx field (the click-through's Suggest anchor)
· MAP_HEALTH_BAND_H stays the single constant shared by the paint and the
hit-test, so the click target cannot drift from the painted wash
Kept main's test file (a superset) and re-added the beatIdx test. All four of
the parent's guard tests pass against the click-through.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
byrongamatos
added a commit
that referenced
this pull request
Jul 13, 2026
…ix (#251) * feat(editor): Map Health — per-bar grid-vs-onset drift review lens The whole "review an automatic map" 2nd pass had no review surface until now (P2-4). Map Health scores, per measure, how well the grid agrees with the detected onsets and paints a thin three-state wash under the ruler, so drift is visible wherever you chart. New pure src/map-health.js: _mapHealthPure(beats, onsets, opts) → per-measure {driftFrac, coverage, band} + overall. Per beat resid = |beat.time − nearestOnset|, reported as a FRACTION of the local beat interval (25ms is inaudible at 60bpm but half a subdivision on 200-bpm 16ths — colour by fraction, never raw ms). Per-measure = the MEDIAN driftFrac over EVIDENCED beats (median so one expressive off-beat note can't drag a bar red) + coverage. THREE states, the third non-negotiable: green (agrees, <5%), amber (drifting, 5-12%), red (disagrees with PRESENT onsets, >12%), and GREY when there are no onsets to judge (silence / sustained / held / pedaled) — NEUTRAL, never red. Colouring an unmeasurable held bar red is crying wolf; the author learns to ignore red, which is fatal. Wiring (ruler.js): a ~5px wash under the beat ticks, memoized on editGen + the onset-cache identity (no per-frame recompute), off by default, a view flag (no history). Toggle in the Tempo/Grid menu (audio-only). Reads only S.beats + _ensureOnsets() — so it rides #248's banded onsets once they land, and degrades gracefully to the RMS detector meanwhile (worse onsets → more grey, never wrong). Tests: tests/map_health.test.mjs (10) — aligned→green, offset 8%→amber (the signature fail-on-main case), onsets-removed→grey NOT red, held-bar→grey, median robustness (one expressive onset doesn't flag the timekeeper bar), >12%→red, band thresholds, tempo/meter independence, degenerate input. 135 JS green, lint 0-err, routes.py untouched. Live-verified on AC/DC: 105 measures, overall green @ 88.6% coverage, 68 green / 26 amber / 2 red / 9 grey (not crying wolf), toggle + persistence + stable memo, wash renders on the ruler. FOLLOW-UP (noted): click a hot bar → jump to Tempo Map + offer the G suggest; LCD pill; distinct-signature reporting (lay-back vs nudge vs ramp); octave "reads 2×, halve?" one-click. Core review surface ships here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q * feat(editor): Map Health click-through — a hot bar takes you to the fix Follow-up to #249. "Tempo is wrong here" → "click the red bar, the fix is waiting." Clicking a DRIFTING (amber/red) bar in the Map Health wash now enters Tempo Map mode, scrolls the bar into view, anchors Suggest on that bar's downbeat (S.tempoSel), and tells the user G is waiting — so the fix is one keypress away. Green/grey bars aren't actionable and fall through to the normal ruler scrub. map-health.js: each measure now carries beatIdx (the S.beats downbeat index) so the click-through can anchor Suggest exactly. ruler.js: _mapHealthBarAt(t) + _mapHealthClickThrough(t) (enter Tempo Map first — it clears the selection — THEN set tempoSel), wired into rulerOnMouseDown for clicks on the thin wash at the ruler's bottom edge. Tests: map_health.test.mjs gains a beatIdx case (downbeat indices 0/4/8 across 3 bars). 135 JS green, lint 0-err, routes.py untouched. Live-verified on AC/DC: clicking amber bar 6 enters Tempo Map, sets tempoSel to its downbeat (idx 20), and posts "Bar 6 drifts 7% … press G"; green + grey bars return false (scrub). Stacks on #249 (feat/editor-map-health); rebase onto main when it merges. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q * fix(editor): Map Health click-through — hit-test the painted wash, not a guess Review fixes on the click-through: · The click target now IS the painted rect. MAP_HEALTH_BAND_H is one constant shared by _drawMapHealthBand and the mousedown gate (was: paints 5px, clicks 6px), and the gate demands x >= LABEL_W — the wash clamps its spans to the label gutter's right, so a click on the bare gutter was claiming whatever bar xToTime() happened to land in and teleporting you there. · A click-through arriving while ALREADY in Tempo Map now clears tempoSelMulti. _editorTempoSuggestFit re-anchors on a live multi-range's first downbeat, which outranks tempoSel — so with a range selected, Suggest silently fit from the wrong bar while the status line promised the bar you clicked. · The lead-in scroll is bounded by the viewport. Zoom caps at 2000 px/s, so a viewport can be shorter than 0.5s: the flat 0.5s lead pushed the very downbeat you clicked off the RIGHT edge. · The status copy resolves the Suggest key from the command registry instead of hardcoding "G" — two shortcut profiles exist. Copy also no longer claims G "fits" the barlines; it proposes a fit (a ghost-handle click accepts). Six regression tests in tests/ruler.test.mjs drive the real onset detector; five fail on the pre-fix code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: ChrisBeWithYou <chris@rifflarr.local> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com>
This was referenced Jul 14, 2026
This was referenced Jul 14, 2026
Merged
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.
The 2nd-pass thesis is "review an automatic map, don't hand-enter it" — but there was no review surface until now (P2-4). Map Health scores, per measure, how well the beat grid agrees with the detected onsets and paints a thin three-state wash under the ruler, so drift is visible wherever you chart. A DAW-plus — no mainstream DAW shows per-bar drift heat.
The metric (pure
src/map-health.js)_mapHealthPure(beats, onsets, opts) → {measures:[{driftFrac, coverage, band}], overall}.resid = |beat.time − nearestOnset|, reported as a fraction of the local beat interval — 25 ms is inaudible at 60 bpm but half a subdivision on 200-bpm 16ths, so it colours by fraction of a beat, never raw ms (reads correctly across tempo ramps + meter changes).coverage = evidenced/total.Three states — the third is non-negotiable
< 5%).5–12%).> 12%).Wiring
A ~5 px wash under the beat ticks (
ruler.js), memoized oneditGen+ the onset-cache identity (no per-frame recompute), off by default, a view flag with no history. Toggle in the Tempo/Grid menu (audio-only). Reads onlyS.beats+_ensureOnsets(), so it rides #248's banded onsets once they land and degrades gracefully to the RMS detector meanwhile (worse onsets → more grey, never wrong).Tests
tests/map_health.test.mjs(10) — aligned→green; offset 8% → amber (the signature fail-on-main case); onsets removed → grey, NOT red; a held bar → grey; median robustness (one expressive onset does not flag the timekeeper bar); >12% → red; band thresholds; tempo/meter independence (8% reads the same at 120 and 240 bpm); degenerate input.routes.pyuntouched.Follow-up (noted, not in this PR)
Click a hot bar → jump into Tempo Map + auto-offer the
Gsuggest for that span; an LCD pill; distinct-signature reporting (lay-back vs nudge vs ramp vs isolated fill); the octave "reads 2×, halve?" one-click. The core review surface ships here.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests