feat(gp_autosync): piecewise time-warp helpers + refine_sync onset pass

[byrongamatos]

What

auto_sync() computes per-bar sync points (chroma DTW), but consumers could only apply the scalar bar-1 audio_offset — any tempo difference between the recording and the tab's authored tempo accumulated audibly over the song. This PR adds the librosa-free helpers needed to apply the full Songsterr-style piecewise mapping, plus a working refine_sync().

New public API in lib/gp_autosync.py

• bar_start_times(gp_path) — per-bar score times on the same axis as the sync points (GPIF bar-resolution map for .gp/.gpx, per-tick integration for GP3/4/5)
• build_warp_anchors(points, bar_starts) — strictly-monotonic (score, audio) anchor pairs, with a slope sanity gate (0.2x–5x) so DTW folds / clamped refine runs can't crush or absurdly stretch a span
• warp_time(t, anchors) — piecewise-linear map, edge-slope extrapolation for count-ins/tails
• warp_song_times(song, warp) — retimes a lib.song.Song in place (notes+sustains, chords, beats, sections, anchors, handshapes, phrase levels, tone changes, tempo overrides); identity-safe for objects shared between flat lists and phrase levels
• gp_has_expandable_repeats(gp_path) — detects GP3/4/5 repeat/volta/direction markup (both jump sources fromDirection and targets direction) whose playback expansion the as-written sync points can't map; callers fall back to offset-only sync
• refine_sync(sync, audio_path, bars_per_point, gp_path=None) — the editor plugin's refine-sync endpoint has imported this since the snapshot, but it never existed in core (the Refine button 500'd). Densifies the DTW points to every Nth bar and re-times each with a local onset phase sweep: sweep radius clamped under half a beat (no one-beat locks on periodic material), short scoring grid + median residual snap, spurious-lock guard (<3 matched onsets keeps the coarse estimate).

Fixes

• GP3/4/5 tick-origin skew (pre-existing): PyGuitarPro absolute ticks start at quarterTime (measure 1 = tick 960), but the module's cumulative bar math is 0-based. Tempo events and chroma synthesis used raw beat.start, applying every mid-song tempo change a quarter note late and skewing the synthesised chroma against the bar timeline. Both are now origin-normalised.
• Duplicated bar-start integration loops factored into _gpif_bar_starts / _gp345_measure_start_ticks (used by both auto_sync and _extract_sync_points).

Validation

Synthetic click-track runs (117.3 / 120.0 / 122.9 BPM recordings of a 120 BPM tab, ±180ms noise on coarse points, 3 seeds): refine_sync lands ~13ms mean / ~40ms max error, offset within ±30ms. Tests: tests/test_gp_autosync_warp.py (18 fixture-free tests); existing tests/test_gp_audio_sync.py (23) still green.

Consumer

Editor plugin PR (got-feedback/feedback-plugin-editor) consumes these helpers in convert-gp; it degrades gracefully to offset-only sync when running against a core without this PR.

Review note

Codex preflight was unavailable (usage limit until Jul 7); an 8-angle multi-agent Claude review ran instead — findings (fromDirection miss, tick origin, slope gate, double-warp identity, CHANGELOG) are fixed in this PR.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added an optional Guitar Pro sync refinement workflow for denser, more accurate score-to-audio alignment.
  • Introduced bar-start timing and piecewise warp utilities to improve refined bar placement.

• Bug Fixes

  • Corrected tempo/bar timing for Guitar Pro 3/4/5 with a consistent tick origin, reducing editor retiming drift.
  • Improved repeat/jump awareness for timing models and stabilized refinements by enforcing monotonic warped times and safer fallbacks.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a librosa-free piecewise time-warping layer to lib/gp_autosync.py with new helpers and refine_sync(), corrects GP3/4/5 tempo tick handling, adds documentation notes, and introduces tests for the warp utilities.

Changes

GP Autosync Warp Refinement

Layer / File(s)	Summary
GP3/4/5 tick origin correction lib/gp_autosync.py	Introduces _GP345_TICK_ORIGIN and normalizes tempo event ticks and chroma synthesis beat timing to a consistent 0-based axis.
Bar-start helpers and wiring lib/gp_autosync.py	Adds _gpif_bar_starts() and _gp345_measure_start_ticks(), replacing inline cumulative tick/time loops in _extract_sync_points() and auto_sync().
Public warp API and refine_sync() lib/gp_autosync.py	Adds exported bar_start_times, gp_has_expandable_repeats, build_warp_anchors, warp_time, warp_song_times, and the new refine_sync() function for denser, onset-refined per-bar alignment; updates module docs.
Documentation and changelog CHANGELOG.md, lib/gp2rs_gpx.py	Adds a changelog entry and an inline warning comment tying future GPIF repeat expansion to gp_has_expandable_repeats().
Warp utility tests tests/test_gp_autosync_warp.py	New test module validating bar-start computation, anchor construction, interpolation, song-time warping, repeat detection, and refine_sync fallbacks.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant refine_sync
  participant bar_start_times
  participant build_warp_anchors
  participant warp_time
  participant AudioOnsets

  Caller->>refine_sync: refine_sync(sync, audio_path, gp_path)
  refine_sync->>bar_start_times: bar_start_times(gp_path)
  bar_start_times-->>refine_sync: exact bar starts
  refine_sync->>build_warp_anchors: build_warp_anchors(sync_points, bar_starts)
  build_warp_anchors-->>refine_sync: warp anchors
  refine_sync->>warp_time: warp_time(bar_time, anchors)
  warp_time-->>refine_sync: estimated audio time
  refine_sync->>AudioOnsets: onset-phase sweep around estimate
  AudioOnsets-->>refine_sync: refined bar time
  refine_sync-->>Caller: new GpSyncData with updated audio_offset

Loading

Related issues: #39

Suggested labels: enhancement, audio-sync, tests

Suggested reviewers: None specified

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The changes implement gp_autosync sync/warp features, not the prewarm dependency workflow required by #39.	Implement an optional prewarm/install command or job, make startup skip installs when prewarm is complete, and add clear cache-usage logging/status.
Out of Scope Changes check	⚠️ Warning	Most code changes add sync-warp helpers and tests unrelated to the requested plugin dependency prewarm workflow.	Move the gp_autosync work to a separate PR and keep this change focused on #39's prewarm/install workflow and status reporting.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: new gp_autosync time-warp helpers and refine_sync.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/autosync-piecewise-warp

---

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

This PR expands lib/gp_autosync.py beyond a single scalar audio_offset by adding librosa-free helpers that build/apply a piecewise score-time → audio-time warp (Songsterr-style), plus a working refine_sync() implementation. It also fixes a GP3/4/5 tick-origin skew affecting tempo-event timing and chroma alignment, and adds targeted, fixture-free tests for the new warp helpers.

Changes:

• Add public warp helper APIs (bar_start_times, build_warp_anchors, warp_time, warp_song_times, gp_has_expandable_repeats) and implement refine_sync() in lib/gp_autosync.py.
• Normalize GP3/4/5 tick origin (PyGuitarPro tick axis) and factor bar-start integration logic into shared helpers.
• Add new unit tests for the warp helpers and document the feature in CHANGELOG.md.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
lib/gp_autosync.py	Adds warp helper APIs + refine_sync(), fixes GP3/4/5 tick-origin skew, factors shared bar-start logic.
tests/test_gp_autosync_warp.py	New fixture-free tests covering warp helpers and refine_sync early-return behavior.
lib/gp2rs_gpx.py	Adds a warning comment clarifying single-pass GPIF repeat behavior and its coupling to sync warping.
CHANGELOG.md	Documents the new warp helpers and refine_sync() under Unreleased.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

🧹 Nitpick comments (1)

tests/test_gp_autosync_warp.py (1)

241-270: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Minor duplication between repeat-detection tests.

test_gp345_repeats_detected and test_gp345_plain_song_no_repeats share nearly identical song/track/header construction, differing only in whether isRepeatOpen is set. Could be factored into a small helper (e.g. _build_gp345_song(tmp_path, name, repeat_open=False)) to reduce duplication.

🤖 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/test_gp_autosync_warp.py` around lines 241 - 270, The two
repeat-detection tests duplicate the same Song/Track/MeasureHeader setup in
test_gp_autosync_warp.py. Refactor the shared setup into a small helper near
test_gp345_repeats_detected and test_gp345_plain_song_no_repeats (for example, a
helper that builds the GP345 song and accepts a repeat-open flag), then have
both tests call it and only vary whether MeasureHeader.isRepeatOpen is set.

🤖 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.

Nitpick comments:
In `@tests/test_gp_autosync_warp.py`:
- Around line 241-270: The two repeat-detection tests duplicate the same
Song/Track/MeasureHeader setup in test_gp_autosync_warp.py. Refactor the shared
setup into a small helper near test_gp345_repeats_detected and
test_gp345_plain_song_no_repeats (for example, a helper that builds the GP345
song and accepts a repeat-open flag), then have both tests call it and only vary
whether MeasureHeader.isRepeatOpen is set.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 24138a4a-9550-475d-9663-a2f719d0676f

📥 Commits

Reviewing files that changed from the base of the PR and between 74cff4e and d18282e.

📒 Files selected for processing (4)

• CHANGELOG.md
• lib/gp2rs_gpx.py
• lib/gp_autosync.py
• tests/test_gp_autosync_warp.py

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

lib/gp_autosync.py (1)

395-406: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep the explicit opening tempo when ticks collide

A bar-1 mixTableChange normalizes to tick 0, but the seeded (0, song.tempo) entry is added first and the dedup keeps the first event per tick. That silently drops an authored opening tempo change and leaves song.tempo driving the early timeline. Prefer the later event on duplicate ticks so the explicit tempo wins.

🤖 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 `@lib/gp_autosync.py` around lines 395 - 406, The dedup logic in the tempo
event merge drops later events at the same tick, which causes the seeded opening
tempo to override an authored bar-1 mixTableChange. Update the unique-building
logic in the tempo collection flow around events.sort/seen_ticks so duplicate
tick entries keep the later event instead of the first, ensuring the explicit
opening tempo from the mixTableChange wins over song.tempo.

🤖 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.

Outside diff comments:
In `@lib/gp_autosync.py`:
- Around line 395-406: The dedup logic in the tempo event merge drops later
events at the same tick, which causes the seeded opening tempo to override an
authored bar-1 mixTableChange. Update the unique-building logic in the tempo
collection flow around events.sort/seen_ticks so duplicate tick entries keep the
later event instead of the first, ensuring the explicit opening tempo from the
mixTableChange wins over song.tempo.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0b780e33-6d33-43bc-8692-782b2b9e8f86

📥 Commits

Reviewing files that changed from the base of the PR and between d18282e and 4c5f6e5.

📒 Files selected for processing (5)

• CHANGELOG.md
• lib/gp2rs_gpx.py
• lib/gp_autosync.py
• static/tailwind.min.css
• tests/test_gp_autosync_warp.py

✅ Files skipped from review due to trivial changes (2)

• lib/gp2rs_gpx.py
• CHANGELOG.md

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/test_gp_autosync_warp.py