From e0d5112153025d3b8e41bd7169e94a1a4fef8f93 Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Wed, 22 Jul 2026 10:47:53 -0500 Subject: [PATCH] fix(editor): an auto-sync import keeps the front of the chart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Guitar Pro import aligned to a recording with a lead-in used to lose its opening notes. The scalar-offset path baked the audio offset into the converted XML, so the pre-roll landed at a negative time that core's parse_arrangement slices off at t>=0 — silently dropping the front of the chart (one report lost the first 16 notes). If auto-sync wasn't used it imported fine, which pinned the cause to the offset application. Guitar Pro is now always converted at offset 0.0 and the shift is applied to the parsed arrangement in memory via the new _apply_gp_import_offset (built on the existing, tested _apply_chart_offset, which preserves negative times) — the same in-place retime the per-bar warp path already used and was immune because of. This repairs the scalar-offset fallback the warp path hands off to (GP5 with repeats, degenerate/absent sync points, or an older core). Notes, beats, sections and the keys notation sidecars all ride the shift. Tests: tests/test_gp_import_offset.py (front notes survive at negative time; sidecars retimed by the same offset; zero-offset no-op). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix Signed-off-by: ChrisBeWithYou --- CHANGELOG.md | 12 +++++ routes.py | 52 ++++++++++++++++--- tests/test_gp_import_offset.py | 91 ++++++++++++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 6 deletions(-) create mode 100644 tests/test_gp_import_offset.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f6f2d2..7ff4ea5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -119,6 +119,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `_gp_drum_arrs_to_parts` (covered by `tests/test_gp_multidrum_split.py`); the frontend adopts the extras via the existing `adoptDrumParts` load path. This brings the GP create-import to parity with the MIDI drum auto-split. +- **An auto-sync import no longer deletes the front of the chart.** A Guitar + Pro import aligned to a recording with a lead-in (the audio starts before the + tab's bar 1) used to lose its opening notes: the scalar-offset path baked the + offset into the converted XML, pushing that pre-roll to a negative time that + core's `parse_arrangement` then sliced off at `t ≥ 0` — silently dropping, in + one report, the first 16 notes. Guitar Pro is now always converted at offset 0 + and the shift is applied to the parsed arrangement **in memory** (the same + in-place retime the per-bar warp path already used), so a lead-in's notes, + beats, sections and keys notation all survive. The per-bar warp path was + already immune; this repairs the scalar-offset fallback it hands off to (GP5 + with repeats, degenerate or absent sync points, or an older core). Tests: + `tests/test_gp_import_offset.py`. - **A drum edit no longer strips another tool's authored keys from drum-part manifest entries.** Saving with drum changes rebuilds the `type: drums` diff --git a/routes.py b/routes.py index 2df781e..02cc82f 100644 --- a/routes.py +++ b/routes.py @@ -2260,6 +2260,34 @@ def _tones(t): song.offset = 0.0 +def _apply_gp_import_offset(song, xml_paths, provided_offset: float) -> None: + """Apply a scalar GP-import audio offset to an ALREADY-PARSED ``song`` (and + its on-disk notation sidecars) in memory — never baked into the converted + XML. + + ``convert_file`` adds ``audio_offset`` to every note time, so any lead-in + (a negative resolved offset) lands the front of the chart at a NEGATIVE XML + time. Core's ``parse_arrangement`` then slices each level at ``t >= 0`` and + silently drops those notes — the reported "auto-sync deleted the front 16 + notes" data loss. Converting at ``audio_offset=0.0`` and shifting the parsed + Song here keeps the pre-roll: ``_apply_chart_offset`` rewrites absolute + times only and preserves negatives (the same in-memory retime the per-bar + warp path already relies on). + + ``convert_file`` adds the offset, whereas ``_apply_chart_offset`` subtracts + its delta, so the delta is ``-provided_offset``. Zero offset is a no-op. + Module-level so pytest can reach it. + """ + if not provided_offset: + return + _apply_chart_offset(song, -provided_offset) + # Sidecars were written by convert_file on the unshifted (0.0) timeline; + # move them by the same amount the notes moved so _attach_gp_notation reads + # times consistent with the shifted notes. + for _xp in xml_paths: + _warp_notation_sidecar(_xp, lambda t: round(t + provided_offset, 6)) + + def _arrangement_xml_candidates(tmp_dir): """Return the source XMLs that represent playable arrangements. @@ -7740,11 +7768,15 @@ def _convert(): "offset-only sync" ) - # Convert GP to XMLs + # Convert GP to XMLs. Always at offset 0.0 — the per-bar warp and + # the scalar-offset fallback both retime the PARSED Song below + # instead of baking the shift into the XML, whose negative lead-in + # times core's parse_arrangement would slice off at t>=0 (dropping + # the front of the chart). xml_paths = convert_file( gp_path, tmp, track_indices=indices, - audio_offset=0.0 if _warp_anchors else _provided_offset, + audio_offset=0.0, arrangement_names=names_map, ) @@ -7789,11 +7821,14 @@ def _convert(): start_time=float(s.get("startTime", "0")), )) - # Apply the per-bar warp: retime every note/beat/section from the - # tab's authored timeline onto the recording's (Songsterr-style - # piecewise mapping — the recording's tempo drift is followed - # instead of accumulating). + # Retime the parsed Song onto the recording's timeline. Both paths + # converted at offset 0.0 (above) and apply the shift here, in + # memory, so a lead-in's negative pre-roll survives parse. if _warp_anchors: + # Per-bar warp: retime every note/beat/section from the tab's + # authored timeline onto the recording's (Songsterr-style + # piecewise mapping — the recording's tempo drift is followed + # instead of accumulating). from lib.gp_autosync import warp_song_times, warp_time warp_song_times(song, lambda t: warp_time(t, _warp_anchors)) # The GP notation sidecars (keys tracks) were written by @@ -7803,6 +7838,11 @@ def _convert(): for _xp in xml_paths: _warp_notation_sidecar( _xp, lambda t: warp_time(t, _warp_anchors)) + else: + # Scalar-offset fallback (per-bar warp unavailable: GP5 repeats, + # degenerate/absent sync points, or an older core). Applied to + # the parsed Song so the front of the chart is never dropped. + _apply_gp_import_offset(song, xml_paths, _provided_offset) # If we have a local audio file path, copy to static if audio_path and Path(audio_path).exists(): diff --git a/tests/test_gp_import_offset.py b/tests/test_gp_import_offset.py new file mode 100644 index 0000000..cb05e54 --- /dev/null +++ b/tests/test_gp_import_offset.py @@ -0,0 +1,91 @@ +"""Regression: a GP import with an audio-sync lead-in must NOT drop the front +of the chart. + +The scalar-offset import path used to bake the offset into the converted XML +(``convert_file(audio_offset=...)``). For any lead-in the resolved offset is +negative, so the front notes land at a NEGATIVE XML time — and core's +``parse_arrangement`` slices each level at ``t >= 0``, silently discarding +them (the reported "auto-sync deleted the front 16 notes" bug). + +The fix converts at ``audio_offset=0.0`` and applies the offset to the PARSED +Song in memory via ``_apply_gp_import_offset`` (built on ``_apply_chart_offset``, +which preserves negative times). These assert the front notes survive and the +notation sidecars are retimed by the same amount. They fail on main, where +``_apply_gp_import_offset`` does not exist. +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from types import SimpleNamespace as NS + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import routes # noqa: E402 +from routes import _apply_gp_import_offset # noqa: E402 + + +def _note(t): + return NS(time=t) + + +def _song(note_times): + """A minimal but structurally complete Song for _apply_chart_offset.""" + arr = NS( + notes=[_note(t) for t in note_times], + chords=[], + anchors=[], + hand_shapes=[], + phrases=[], + tones=None, + ) + return NS(arrangements=[arr], beats=[_note(0.0)], + sections=[NS(start_time=0.0)], offset=0.0, song_length=10.0) + + +def test_leadin_offset_keeps_the_front_notes_at_negative_time(): + # A 1.0s lead-in: GP-sign negation resolves to provided_offset = -1.0. + # convert_file would ADD that, so notes near t=0 go negative and were + # dropped. The in-memory shift keeps every one. + song = _song([0.0, 0.2, 1.5, 3.0]) + _apply_gp_import_offset(song, [], provided_offset=-1.0) + times = [n.time for n in song.arrangements[0].notes] + # Nothing dropped, and the front notes are preserved at negative time. + assert len(times) == 4 + assert times == [-1.0, -0.8, 0.5, 2.0] + # Beats and sections ride along; the residual chart offset is cleared. + assert song.beats[0].time == -1.0 + assert song.sections[0].start_time == -1.0 + assert song.offset == 0.0 + + +def test_positive_offset_shifts_forward_and_drops_nothing(): + song = _song([0.0, 1.0]) + _apply_gp_import_offset(song, [], provided_offset=0.5) + assert [n.time for n in song.arrangements[0].notes] == [0.5, 1.5] + + +def test_zero_offset_is_a_noop(): + song = _song([0.0, 1.0]) + _apply_gp_import_offset(song, [], provided_offset=0.0) + assert [n.time for n in song.arrangements[0].notes] == [0.0, 1.0] + # song_length and offset untouched on the no-op path. + assert song.song_length == 10.0 + + +def test_sidecars_are_retimed_by_the_same_offset(monkeypatch): + # Prove the sidecar path actually uses provided_offset (not just the notes): + # capture the mapping callable handed to _warp_notation_sidecar. + captured = {} + + def _fake_sidecar(xml_path, warp): + captured[xml_path] = warp + + monkeypatch.setattr(routes, "_warp_notation_sidecar", _fake_sidecar) + song = _song([0.0]) + _apply_gp_import_offset(song, ["a.xml", "b.xml"], provided_offset=-1.0) + assert set(captured) == {"a.xml", "b.xml"} + # convert_file ADDS the offset, so the sidecar remap is t -> t + offset. + assert captured["a.xml"](5.0) == 4.0 + assert captured["b.xml"](0.0) == -1.0