Skip to content

docs: follow up GP import doc coverage - #91

Merged
PhysShell merged 1 commit into
claude/modest-maxwell-0zecwnfrom
codex/github-mention-feat-import-guitar-pro-tapping-as-a-tapped
Jun 20, 2026
Merged

docs: follow up GP import doc coverage#91
PhysShell merged 1 commit into
claude/modest-maxwell-0zecwnfrom
codex/github-mention-feat-import-guitar-pro-tapping-as-a-tapped

Conversation

@PhysShell

Copy link
Copy Markdown
Owner

Motivation

  • Code coverage tooling flagged missing docstrings on private Guitar Pro import helpers and a Clippy nursery lint surfaced during verification.
  • Small follow-up changes were needed to satisfy the project's documentation and linting gates without changing behaviour.

Description

  • Added Rust doc comments to the Guitar Pro import helpers in core/src/gp.rs including gp_song_to_score, build_gp_master_bars, build_gp_track, build_gp_voice, append_beat, and rest_group to improve doc coverage.
  • Rewrote the accumulator updates in rotated_correlation (core/src/complement.rs) to use f64::mul_add for numerator, var_x, and var_y, addressing the clippy::suboptimal_flops nursery lint.
  • Committed the changes on the branch and prepared this follow-up PR; no public API or schema changes were made.

Testing

  • Ran cargo fmt --all --check which succeeded.
  • Ran cargo clippy --workspace --all-targets -- -D warnings which succeeded with the nursery lint fixed.
  • Ran the full test suite via cargo test --workspace which completed with all tests passing.

Codex Task

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 57d96053-5760-4fed-82ff-32ae043788fa

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/github-mention-feat-import-guitar-pro-tapping-as-a-tapped

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a77b7e03ec

ℹ️ 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".

Comment thread core/src/complement.rs
numerator += dx * dy;
var_x += dx * dx;
var_y += dy * dy;
numerator = dx.mul_add(dy, numerator);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve exact-tie key ordering

When a part's duration-weighted pitch-class histogram gives an exact tie between two rotated KK profiles, the documented tie-breaker is the first key scanned. The fused accumulation can split that exact tie by one ulp: for example histogram [194,498,821,804,875,237,243,964,12,952,316,475] makes D major and G major exact ties, but this line rounds D's numerator lower and estimate_harmony now picks G major, changing PartProfile::harmony and downstream complement pitch material for existing ambiguous parts. Either keep the previous accumulation or add an explicit epsilon/tie comparison so lint cleanup doesn't alter key ordering.

Useful? React with 👍 / 👎.

@PhysShell
PhysShell merged commit b745c5e into claude/modest-maxwell-0zecwn Jun 20, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant