fix(gp): GP6/GPIF note strings are 0-indexed (correct pitch, recover dropped notes)

[PhysShell]

Problem

A Guitar Pro 6 (.gpx) import came out ≈4–5 semitones flat with notes
missing — most audible as "everything shifted down" and a thinned-out low riff.

Root cause

The guitarpro crate exposes a note's string field 0-indexed for the
GP6/GPIF reader, but 1-indexed for the GP3/4/5 binary readers — the same
per-format divergence griff already normalises for repeat counts
(version_major >= 6).

gp_note_midi_pitch / gp_note_position assumed 1-indexing unconditionally
(strings[note.string - 1], plus a note.string <= 0 reject). For GP6 that:

1. read the open tuning of the string one position too low → every note
   transposed down by that string interval (~4–5 semitones); and
2. dropped every note on raw string 0 (the lowest string) via the <= 0
   guard, logged as "GP note pitch out of range; note skipped".

Evidence (raw crate data for the reported drop-D .gpx)

Track::strings = [(1,38),(2,45),(3,50),(4,55),(5,59),(6,64)], notes carry
string ∈ 0..5. Brute-forcing the index against Guitar Pro's own MIDI export
of the same file: only strings[note.string] (0-indexed) reproduces the
reference pitch set — e.g. lead bar 1 → {51,58,62,65,67,77,79}, exact match;
the old strings[note.string-1] gave {44,53,57,60,62,72,74}.

Fix

Normalise the raw string to griff's 1-indexed convention per source format:

fn gp_one_indexed_string(raw: i8, zero_indexed: bool) -> i16 {
    i16::from(raw).saturating_add(i16::from(zero_indexed))
}

zero_indexed = version_major >= 6 is derived in gp_song_to_score (next to the
existing repeat-count split) and threaded down through a small read-only
StringCtx { strings, zero_indexed }. The GP3/4/5 binary path is byte-for-byte
unchanged (zero_indexed = false).

Verification (real .gpx, through the public import_gp_score path)

track	before	after	GP reference
0	39–72, 11 pitch-classes (incl. out-of-scale)	41–77, {0,2,3,5,7,9,10}	41–77, {0,2,3,5,7,9,10}
1	38–79, all 12 classes	38–84, {0,2,3,5,7,9,10}	38–84, {0,2,3,5,7,9,10}

Ranges and pitch-class sets now match Guitar Pro's own export exactly, and the
~140 dropped low-string notes are recovered (0 "out of range" losses, was ~140).

Tests

• gp6_strings_are_zero_indexed — raw string 0 maps to the first entry (not
  dropped); raw string 1 + fret 6 = 51 (not 44).
• gp6_note_position_normalises_string_number — raw 0 → griff string 1.
• gp_one_indexed_string_normalises_per_format — both bases.
• Existing binary-path pitch/position tests pass unchanged (, false).

Full workspace suite + clippy --all-targets clean. gp.rs is rustfmt-clean
(two unrelated pre-existing dump.rs/dump_golden.rs rustfmt diffs left
untouched — they predate this branch).

Second of the two fixes for the reported issues; the MIDI Type-0 export fix is
#61. With clean GP6 input, the complement "low copy" observation is worth a
fresh look — happy to follow up.

https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95

---

Generated by Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Fixed string numbering normalization when importing Guitar Pro files to ensure correct note positioning and MIDI pitch mapping across all supported format versions.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7ec764b3-a248-4e50-b009-5512378fa856

📥 Commits

Reviewing files that changed from the base of the PR and between 44c3cc7 and 9073de1.

📒 Files selected for processing (1)

• core/src/gp.rs

---

📝 Walkthrough

Walkthrough

core/src/gp.rs normalizes Guitar Pro string numbering across GP3/4/5 (1-indexed) and GP6/GPIF (0-indexed) formats. A zero_indexed boolean is computed from the GP major version and propagated through gp_song_to_score → build_gp_track → build_gp_voice → append_beat via a new StringCtx struct. New helper gp_one_indexed_string is added, and gp_note_midi_pitch, gp_note_position, and extend_tie are updated to accept and apply the flag.

Changes

GP String Index Normalization

Layer / File(s)	Summary
Normalization math helpers core/src/gp.rs	Adds gp_one_indexed_string(raw, zero_indexed) and updates gp_note_midi_pitch and gp_note_position to accept zero_indexed, normalizing string numbers and rejecting non-positive post-normalization indices.
StringCtx struct, append_beat, and extend_tie core/src/gp.rs	Introduces StringCtx<'a> bundling the strings slice and zero_indexed. Updates append_beat to accept it, routes pitch/position calls through it, normalizes string IDs for tie tracking, and adds zero_indexed to extend_tie for held-note lookup.
Pipeline wiring: gp_song_to_score → build_gp_voice core/src/gp.rs	Computes zero_indexed_strings from GP major version in gp_song_to_score and threads it through build_gp_track and build_gp_voice as a new parameter, where StringCtx is constructed for append_beat.
Updated and new unit tests core/src/gp.rs	Updates MIDI pitch and note-position tests for the new zero_indexed parameter, adds GP6 zero-indexed coverage, and updates dead-note and tied-note append_beat calls to pass StringCtx.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• PhysShell/griff#53: Modifies the same append_beat/held-note state and extend_tie code paths in core/src/gp.rs that this PR also refactors for zero-indexed normalization.
• PhysShell/griff#51: Modifies the gp_note_position and dead-note import path in core/src/gp.rs, which this PR also updates when threading zero_indexed through the note-position conversion.

Poem

🐇 Strings start at zero, strings start at one,
Which GP format? Let me check — ah, done!
A flag threads through, context in paw,
No off-by-one bugs shall I gnaw.
Normalized notes, each fret correct,
The rabbit hops on — pitch perfect! 🎸

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: fixing a GP6/GPIF issue where string numbering is 0-indexed (causing incorrect pitch and dropped notes). It directly reflects the core problem and solution.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 claude/gp6-string-index

---

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