Add pitch-change split and enable refinement by default

[MrDix]

Summary

• Pitch-change split (--pitch_change_split): New experimental feature that splits notes at pitch change boundaries within word segments. Detects sustained pitch changes (>2 semitones, >80ms) and creates separate notes for each pitch region. First sub-note keeps the text, continuations use ~ markers. Disabled by default.
• Refinement enabled by default: --refine_from_vocal is now on by default (was opt-in). Added --disable_refine flag for opt-out. This uses the C++ ptAKF pitch detector to find and fix poorly-scoring notes.
• --youtube_url documentation: Detailed help text and README section explaining the browser-based download workflow where audio is pre-downloaded but the YouTube URL is still needed for metadata.

Test Results

Tested with live performance song (high vocal variance — good stress test):

• Baseline (no pitch-split, no refine): Simple 6143 (61.4%), Accurate 5969 (59.7%)
• With pitch-split + refine default: Simple 6559 (65.6%), Accurate 6308 (63.1%)
• Improvement: +416 points simple (+4.2%), +339 points accurate (+3.4%)

Test plan

• 20 unit tests for pitch-change splitter (edge cases, merging, text preservation)
• Integration test with real song shows score improvement
• --disable_refine correctly disables refinement
• --pitch_change_split flag works from CLI and GUI settings
• Existing tests pass (uv run pytest)

Summary by CodeRabbit

Release Notes

• New Features

  • Added optional pitch-change split feature to split sustained notes at detected pitch boundaries within syllables (disabled by default). Enable via --pitch_change_split CLI flag or UI settings in Experimental Features.

• Changes

  • Refinement processing is now enabled by default. Use --disable_refine to disable.

[coderabbitai]

Warning

Rate limit exceeded

@MrDix has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 29 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6e3c0de9-d26f-4def-9113-c21b12fb40c1

📥 Commits

Reviewing files that changed from the base of the PR and between 9576bd1 and 2a923a0.

📒 Files selected for processing (1)

• src/modules/Pitcher/pitch_change_splitter.py

📝 Walkthrough

Walkthrough

This PR adds a new feature to split musical notes at pitch-change boundaries (melismas/runs). It introduces a pitch_change_split setting (default False), implements the splitting logic in a new module with comprehensive test coverage, integrates it into the CLI/GUI/settings systems, and enables refinement-from-vocal by default.

Changes

Cohort / File(s)	Summary
Pitch-Change Splitter Implementation src/modules/Pitcher/pitch_change_splitter.py	New module that detects pitch-change boundaries within MIDI segments using confidence-weighted pitch analysis, splits segments at transitions above min_semitone_change, merges adjacent sub-segments with identical computed notes, and merges short fragments below min_note_duration_ms with nearest neighbors. Exports split_notes_at_pitch_changes() entrypoint.
Pitch-Change Splitter Tests pytest/modules/Pitcher/test_pitch_change_splitter.py	Comprehensive test suite covering constant-pitch (no split), empty input, short segments, multi-pitch splits, glitch/variation tolerance, text handling with continuation markers ("~"), fragment merging, edge cases (low confidence, silence frames, large octave jumps), and boundary preservation.
Settings & Configuration src/Settings.py, src/gui/config.py	Added boolean pitch_change_split setting (default False). Updated refine_from_vocal default from False to True to enable reverse-scoring refinement by default.
Main Processing Pipeline src/UltraSinger.py	Integrated pitch-change splitting post-processing into main pipeline when enabled. Added CLI support for --pitch_change_split flag and --disable_refine to toggle the new default refinement behavior.
GUI Integration src/gui/settings_tab.py, src/gui/ultrasinger_runner.py	Added "Pitch-Change Split" toggle switch in Experimental Features section. Updated config collection and CLI argument building to include the new setting.
Help Text src/modules/common_print.py	Updated print_help() to document --pitch_change_split under post-processing and reframed refinement as enabled-by-default with --disable_refine flag; marked --refine_from_vocal as legacy.

Sequence Diagram

sequenceDiagram
    participant Main as Main Processor
    participant Config as Settings/Config
    participant Splitter as split_notes_at_pitch_changes
    participant Analyzer as Pitch Analyzer
    participant Segments as MIDI Segments

    Main->>Config: Check settings.pitch_change_split & audio enabled
    Config-->>Main: True/False
    
    alt pitch_change_split enabled
        Main->>Splitter: Call with segments + pitched_data
        Splitter->>Analyzer: Extract per-frame pitch for each segment
        Analyzer->>Analyzer: Detect pitch-change boundaries
        Analyzer-->>Splitter: Split points identified
        Splitter->>Segments: Create sub-segments at boundaries
        Segments->>Segments: Merge short fragments & identical notes
        Splitter-->>Main: Return expanded segment list
    else disabled
        Main-->>Main: Skip processing, use original segments
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Pitch notes dance and leap so high,
A rabbit splits them with a try—
Where melodies their colors change,
We break them now across the range!
Refinement flows by default's might,
Making music ever right. ✨

🚥 Pre-merge checks | ✅ 3

✅ 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 captures the two main changes: adding pitch-change split feature and enabling refinement by default, directly reflected in the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 94.74% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/pitch-change-split

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[MrDix]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

🧹 Nitpick comments (3)

src/modules/Pitcher/pitch_change_splitter.py (3)

8-8: Unused import: math

The math module is imported but not used anywhere in this file.

🔧 Proposed fix

-import math

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/modules/Pitcher/pitch_change_splitter.py` at line 8, Remove the unused
import of the math module from pitch_change_splitter.py; locate the top-level
import statement "import math" and delete it so there are no unused imports
remaining in the module (no other code changes required).

---

177-177: Consider using list spread syntax for clarity (style preference).

The static analysis suggests using spread syntax instead of list concatenation.

🔧 Proposed fix

-    boundaries = [0] + change_points + [len(times)]
+    boundaries = [0, *change_points, len(times)]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/modules/Pitcher/pitch_change_splitter.py` at line 177, Replace the list
concatenation that builds boundaries with list spread syntax for clarity: in
pitch_change_splitter.py where the variable boundaries is constructed from 0,
change_points, and len(times), change the expression to use a single list
literal that spreads change_points between 0 and len(times) (i.e., [0,
*change_points, len(times)]), ensuring change_points is iterable and preserving
the original order and types.

---

92-96: Consider adding strict=True to zip() for defensive programming.

While frequencies and confidences should always have the same length (both come from sliced PitchedData), adding strict=True would catch potential data inconsistencies earlier.

🔧 Proposed fix

-    for freq, conf in zip(frequencies, confidences):
+    for freq, conf in zip(frequencies, confidences, strict=True):

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/modules/Pitcher/pitch_change_splitter.py` around lines 92 - 96, The loop
using zip(frequencies, confidences) can silently mask length mismatches; update
it to use zip(..., strict=True) so any unequal lengths raise an error early,
e.g., replace zip(frequencies, confidences) with zip(frequencies, confidences,
strict=True) in the function that builds midi_values (the block calling
_freq_to_midi_safe and appending to midi_values) to defensively catch data
inconsistencies between frequencies and confidences.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/modules/Pitcher/pitch_change_splitter.py`:
- Line 8: Remove the unused import of the math module from
pitch_change_splitter.py; locate the top-level import statement "import math"
and delete it so there are no unused imports remaining in the module (no other
code changes required).
- Line 177: Replace the list concatenation that builds boundaries with list
spread syntax for clarity: in pitch_change_splitter.py where the variable
boundaries is constructed from 0, change_points, and len(times), change the
expression to use a single list literal that spreads change_points between 0 and
len(times) (i.e., [0, *change_points, len(times)]), ensuring change_points is
iterable and preserving the original order and types.
- Around line 92-96: The loop using zip(frequencies, confidences) can silently
mask length mismatches; update it to use zip(..., strict=True) so any unequal
lengths raise an error early, e.g., replace zip(frequencies, confidences) with
zip(frequencies, confidences, strict=True) in the function that builds
midi_values (the block calling _freq_to_midi_safe and appending to midi_values)
to defensively catch data inconsistencies between frequencies and confidences.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: eebfb31a-f452-4368-afe2-182fe5a3ed84

📥 Commits

Reviewing files that changed from the base of the PR and between b5532c0 and 9576bd1.

📒 Files selected for processing (8)

• pytest/modules/Pitcher/test_pitch_change_splitter.py
• src/Settings.py
• src/UltraSinger.py
• src/gui/config.py
• src/gui/settings_tab.py
• src/gui/ultrasinger_runner.py
• src/modules/Pitcher/pitch_change_splitter.py
• src/modules/common_print.py