fix(editor): auto anchors look ahead and stop thrashing on lone notes#353
Conversation
The auto fret-hand anchor generator (`_compute_anchors`) was a single
forward-greedy pass: fixed width 4, no look-ahead, relocating one fret
below any note outside the current window. It over-split positions it
didn't need to (frets 3, 1, 5 -> two anchors instead of one at fret 1)
and thrashed on a single high grace note, emitting an up-then-back pair
of shifts for one reached note. Auto anchors feed position-suggest,
chord-grip, fingering and the stretch/legato lints, so weak anchors
quietly degraded all of them.
It now grows each run to the longest span of consecutive note-groups
that still fits one window before choosing the fret, and tolerates a
lone out-of-window group whose next note returns to the window as a
reach rather than a position change (a real >=2-note excursion still
moves). Chord and coincident notes are grouped by time, so the hand can
no longer relocate mid-chord. The {time, fret, width} shape and
inclusive-width semantics are unchanged.
Pure and deterministic; covered by tests/test_anchor_compute.py. Full
pytest green (401 passed); lint clean; the only JS failures are the two
known-on-main ones (mixer_meter_teardown, song_fit).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix
Signed-off-by: ChrisBeWithYou <chris@rifflarr.local>
|
Warning Review limit reached
Next review available in: 27 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 (2)
📝 WalkthroughWalkthroughThe fret-hand anchor engine now groups simultaneous fretted events, computes anchors with look-ahead and reach handling, preserves time-zero coverage, and avoids redundant shifts. New tests cover placement, chords, outliers, monotonicity, robustness, and window invariants. ChangesFret-hand anchor engine
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Notes
participant FrettedGroups
participant ComputeAnchors
participant XMLConversion
Notes->>FrettedGroups: provide notes and chord notes
FrettedGroups->>ComputeAnchors: provide grouped fret spans
ComputeAnchors->>ComputeAnchors: grow runs and handle reaches
ComputeAnchors-->>XMLConversion: return deduplicated anchors
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@routes.py`:
- Around line 2835-2848: Update the return check in the anchor computation loop
around reachable and returns to compare nxt against the effective window
produced by _place(win_lo, win_hi), not the raw win_lo/win_hi bounds. Preserve
the existing reachability and run-absorption behavior, including accepting close
return notes that fall within the eventual placed anchor window.
🪄 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: 929079cd-8491-4e39-8f91-887a0f54db5e
📒 Files selected for processing (3)
CHANGELOG.mdroutes.pytests/test_anchor_compute.py
What
Rewrites the auto fret-hand anchor generator
_compute_anchors(routes.py) to add look-ahead run placement and lone-outlier tolerance. This is the "Anchor look-ahead" card (the flagged START-HERE next-win): auto anchors silently drive position-suggest, chord-grip, fingering, and the stretch/legato lints, so weak anchors degraded all of them at once.Why the old engine was weak
_compute_anchorswas a single forward-greedy pass — fixedwidth=4, no look-ahead, relocating one fret below any note that fell outside the current window. Two concrete failures:3, 1, 5(all within a 4-fret span) became two anchors because it committed to a position on the3before it ever saw the1.3 3 3 12 3 3 3) forced an up-then-back pair of phantom shifts (3 → 11 → 3) for one reached note.It also walked chord notes individually, so a wide chord could "relocate" the hand in the middle of itself.
What changed
width-wide window before its fret is chosen — so3, 1, 5is one anchor at fret 1.(lo, hi)group, so the hand can never relocate mid-chord.The
{time, fret, width}output shape and the inclusive[fret, fret+width]width semantics (matching the stretch lint and the highway) are unchanged. First anchor still coverst=0; adjacent same-fret shifts are still dropped.Scope note
src/position.js::_suggestPositionPureuses a half-open[fret, fret+width)window — one fret narrower than the generator + stretch lint. That off-by-one is pre-existing and left untouched here to keep this PR focused on the generator; worth a follow-up to unify the width predicate across consumers.Tests
tests/test_anchor_compute.py(~20 cases): the two headline wins (both fail on the old engine), placement, chord grouping, the excursion-vs-reach boundary, monotonic scale shifts, structural invariants, determinism, and coverage.pytestgreen (401 passed, 5 skipped). The existingtest_xml_export.pyanchor tests (which compare dynamically against_compute_anchors) still pass.npm run lintclean (0 errors). JS suite unchanged (Python-only diff); the only red suites are the two known-on-mainfailures (mixer_meter_teardown,song_fit).🤖 Generated with Claude Code
Summary by CodeRabbit
Enhancements
Tests