feat(core): import Guitar Pro 7/8 (.gp) - #133
Conversation
GP7/8 `.gp` was the one format the importer refused: 79 of 410 tabs in the target collection (19%) returned UnsupportedFormat. A `.gp` file is a ZIP (`PK\x03\x04`) carrying `Content/score.gpif`, and the `guitarpro` 0.4.2 crate griff already depends on ships `Song::read_gp`, which unzips it and runs the same GPIF conversion the GP6 `.gpx` path uses. Three failing tests fix the contract: - `detect_version_gp7`: the ZIP magic maps to Some(7) (was None). - `import_gp7_dispatches_to_the_reader`: ZIP magic + garbage must reach `read_gp` and fail *there* with a typed Parse error, not UnsupportedFormat. This proves the branch is wired without a real `.gp` fixture — the corpus is copyrighted community tabs (redistributable:false), so no `.gp` can be committed as a test asset, and the repo has no committed GP binary fixtures. - `source_format_follows_the_importer_tag`: the "GP7" importer tag maps to the new `SourceFormat::Gp`, not the MIDI fallback. `detect_version_unknown` drops its PK line, since `PK\x03\x04` is no longer unknown. `SourceFormat::Gp` is added as an inert data-only variant so the CLI test compiles; the mapping that would satisfy it is deliberately not wired yet. No exhaustive match on SourceFormat exists, so the variant breaks nothing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM
`detect_gp_version` maps the ZIP magic `PK\x03\x04` to major version 7, and `import_gp_score` dispatches it to `guitarpro::Song::read_gp`, which unzips `Content/score.gpif` and runs the same GPIF conversion `read_gpx` (GP6) already uses. `gp_song_to_score` needs no change: the Song's `version.number.0` is 7, so it tags `source_meta.format = "GP7"` and takes the existing `>= 6` branch (zero-indexed strings, raw repeat counts). The CLI maps that tag to `SourceFormat::Gp`. Local acceptance over the target collection (the 79 real `.gp` files this machine has, not committed): all 79 import, 0 panics — the F-003 guitarpro out-of-bounds did not fire on any. Whole-collection import went 328/410 (80%) to 407/410 (99%); the remaining 3 are truncated `.gp5` files (genuine EOF corruption, not a format gap). Notes read rose 1.32M to 1.65M. No new dependency: `guitarpro` 0.4.2 was already resolved and already compiles `read_gp` (the crate declares no features). Docs updated: the support matrix, the `import_gp_score` doc, and the five CLI help strings that list formats. Validation: fmt, clippy -D warnings, workspace tests (core 210), cargo doc (15 warnings, identical to base) — all clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM
|
Warning Review limit reached
Next review available in: 52 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 (4)
✨ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da8fdbeb6f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…t midi Review finding on this PR (Codex): the CLI's `source_format` gained a `"GP7" => SourceFormat::Gp` arm, but the cockpit/capture path has its own copy of the same mapper in `ui-core/src/capture.rs`, and it was left with only GP3..GP6. A `.gp` score curated through the UI therefore records `source.format = "midi"` — the exact silent-mislabel the CLI change was meant to prevent, on the other path. The test maps a GP7-tagged score through the capture mapper and expects `SourceFormat::Gp`; it fails today with `midi`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM
The cockpit/capture `source_format` mapper gains the `"GP7" => SourceFormat::Gp` arm, matching the CLI. A `.gp` score curated through the UI now records its true format instead of falling back to `midi`. The RED test passes. Both copies of this mapping now carry GP7; they stay parallel by convention (the cockpit path is wasm and cannot share the CLI's binary-only helpers). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM
Brings PR #133's Guitar Pro 7/8 support and the cockpit capture provenance fix onto this branch, so griff ingest reads the 79 .gp files in the target collection instead of skipping them as unrecognised. No conflicts: #133 touched core/src/gp.rs, core/src/corpus.rs (the SourceFormat::Gp variant), cli/src/main.rs (the source_format arm) and ui-core/src/capture.rs; this branch's ingest work is in core/src/ingest.rs and separate cli functions. This branch's 12 commits and their hashes are unchanged.
Guitar Pro 7/8 (
.gp) importRecovers the one format the importer refused. In the target tab collection 79 of 410 files (19%) are
.gpand all returnedUnsupportedFormat; this makes them import.18ec861(main)da8fdbeWhy it's small
A
.gpfile is a plain ZIP (PK\x03\x04) carryingContent/score.gpif. Theguitarpro0.4.2 crate griff already depends on shipsSong::read_gp, which unzips it and runs the same GPIF conversion the GP6.gpxpath already uses. The Song'sversion.number.0becomes 7, sogp_song_to_scoreneeds no change: it tagssource_meta.format = "GP7"and takes the existing>= 6branch (zero-indexed strings, raw repeat counts). No new dependency —guitarprowas already resolved and compilesread_gpunconditionally (the crate declares no features).A subagent spike established feasibility (found
read_gp, proved the GPIF path, swept all 79 files); every claim here was then re-verified on this actual change, not taken from the spike.RED → GREEN
a225a85REDda8fdbeGREENdetect_gp_version+import_gp_score+ CLI mapping + docsTests:
detect_version_gp7— ZIP magic →Some(7)(wasNone;detect_version_unknowndrops its stale PK line).import_gp7_dispatches_to_the_reader— ZIP magic + garbage must reachread_gpand fail there with a typedParseerror, notUnsupportedFormat. This proves the branch is wired without a real.gpfixture — see below.source_format_follows_the_importer_tag— the"GP7"importer tag maps toSourceFormat::Gp, not the MIDI fallback.No committed
.gptest asset, by policy. The corpus is copyrighted community tabs (redistributable: false), so no.gpcan be committed, and the repo has no committed GP binary fixtures at all (the GP files undercorpus/are local and git-ignored). The dispatch test covers the wiring with a synthetic ZIP-magic input instead.Change
core/src/gp.rs:detect_gp_versionrecognisesPK\x03\x04→ 7;import_gp_scoreaddsSome(7) => song.read_gp(data)?. Support-matrix and function docs updated.core/src/corpus.rs:SourceFormat::Gpvariant ("gp"under the existingrename_all). Additive — no exhaustive match onSourceFormatexists, so nothing breaks.cli/src/main.rs:source_formatmaps"GP7" => SourceFormat::Gp; five help strings now list.gp.Local acceptance (files not committed)
Re-ran the ingest inventory over the real collection on this build:
The 3 that still fail are truncated
.gp5files (genuine EOF corruption, unrelated to this change).Validation
cargo fmt --all --check0 ·cargo clippy --workspace --all-targets -- -D warnings0 ·cargo test --workspace --exclude griff-cli0 (core 210) ·cargo doc --no-deps --workspace15 warnings, identical to base18ec861·cargo +1.92 check(MSRV) 0 ·fuzznightly check 0.cargo test -p griff-clifails onlymissing_file_golden— the pre-existing Russian-locale golden (os error 2renders localised on this machine); identical at base, unaffected by this change, green on CI's English runner.Blast radius
Three files, all additive.
import_score_autois content-sniffed, so.gproutes automatically once the branch lands — no extension-dispatch change. Nothing outside the GP import path is touched.Not self-accepted, not merged.
🤖 Generated with Claude Code
https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM