fix: preserve explicit staff on single-staff parts#323
Open
webern wants to merge 1 commit into
Open
Conversation
webern
added
bug
Claude
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28513 / 36640 |
| Functions | 74.3% | 6352 / 8551 |
| Branches | 50.7% | 22678 / 44751 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 80.8% | 6396 / 7918 |
| Functions | 67.7% | 2218 / 3276 |
| Branches | 49.6% | 5478 / 11054 |
Core HTML report | API HTML report
Commit dfe9185c493f76563deb828adeb87f8dbac6a396.
gen-quality
|
This was referenced Jul 7, 2026
webern
changed the title
staff on single-staff parts
NoteWriter::setStaffAndVoice only wrote <staff> when the part had more than one staff, since staff number is otherwise redundant with containment (measure -> staves -> voices -> notes). But <staff> is legal MusicXML on a single-staff part too, and many real-world exporters write it unconditionally -- so a source's explicit <staff> was silently dropped on round-trip on every single-staff part, the single largest drop signature in the api round-trip corpus. Added NoteData::isStaffValueSpecified (mirrors the existing DirectionData::isStaffValueSpecified), set by NoteReader whenever the source note carried an explicit <staff>. NoteWriter now emits <staff> whenever the part is multi-staff (unchanged, structurally required) or the source had it explicitly, matching the same source-had-it-explicitly convention already used for <voice> (NoteData::userRequestedVoiceNumber). Closes #275
webern
force-pushed
the
fix-note-staff-drop
branch
from
d08a608 to
fa268e0
Compare
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28513 / 36640 |
| Functions | 74.3% | 6352 / 8551 |
| Branches | 50.7% | 22678 / 44751 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 80.9% | 6429 / 7948 |
| Functions | 67.8% | 2222 / 3277 |
| Branches | 49.6% | 5506 / 11102 |
Core HTML report | API HTML report
Commit 0a6fe0968dd76663538ff05cb0550d46a92a66c8.
gen-quality
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Human Summary
I don't think I like this one. Hold. Possible revision needed.
Summary
NoteWriter::setStaffAndVoiceonly wrote<staff>when the part had more than one staff, sincestaff number is otherwise redundant with containment (
measure -> staves -> voices -> notes). But<staff>is legal MusicXML on a single-staff part too, and many real-world exporters(Finale/Sibelius/MuseScore) write it unconditionally -- so a source's explicit
<staff>wassilently dropped on every single-staff-part note on round-trip. This was the single largest drop
signature in the api round-trip corpus (
drop:staff, ~285 of 675 currently-failing files).Added
NoteData::isStaffValueSpecified(mirrors the existingDirectionData::isStaffValueSpecified), set byNoteReaderwhenever the source note carried anexplicit
<staff>.NoteWriternow emits<staff>whenever the part is multi-staff (unchanged,structurally required there) or the source had it explicitly -- the same source-had-it-explicitly
convention already used for
<voice>(NoteData::userRequestedVoiceNumber).Testing
explicitStaffOnSingleStaffPartRoundTrips/implicitStaffOnSingleStaffPartOmitsElementtests (
NoteDataTest.cpp)make test: all pass (4728 assertions in 380 test cases, plus the three examples)make test-api-roundtrip: 159 passed, 0 failed (of 159 pinned; 1 newly unlocked)make fmt/make check: cleanReferences