fix(api): clef number round-trip#267
Merged
Merged
Conversation
gen-quality
|
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28487 / 36624 |
| Functions | 74.3% | 6349 / 8550 |
| Branches | 50.6% | 22632 / 44725 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 78.5% | 6025 / 7679 |
| Functions | 63.7% | 2045 / 3209 |
| Branches | 47.6% | 5101 / 10717 |
Core HTML report | API HTML report
Commit 40ac16f9ce99d45c0edab5d4c2bbcd6e528c079e.
gen-quality
|
dump-api-roundtrip only writes pairs for failing files, so a file that flips to passing leaves a stale pair behind. The classifier then reads it as a current failure. rm -rf the dump dir, both in the build volume and the host copy, before each dump.
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28487 / 36624 |
| Functions | 74.3% | 6349 / 8550 |
| Branches | 50.6% | 22632 / 44725 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 78.4% | 6025 / 7683 |
| Functions | 63.7% | 2045 / 3209 |
| Branches | 47.6% | 5109 / 10729 |
Core HTML report | API HTML report
Commit 70839ceb60f3c67e93622d59b8be056690b62b28.
gen-quality
|
webern
changed the title
webern
commented
Jun 30, 2026
webern
commented
Jun 30, 2026
webern
added
bug
Stop emitting a spurious number="1" on single-staff parts and preserve the source's choice to include or omit the attribute. The writer derived the staff number from position but, for mid-measure clef changes in single-staff parts, passed the staff index straight through instead of suppressing it the way the measure-start path already did. Centralize the single/multi-staff rule in clefStaffIndex(), and add ClefData::writeStaffNumber - a ternary defaulting to an automatic rule (omit the implied 1 on a single-staff part, include otherwise) that round-trip uses to preserve a source that diverged from it. Grows the api round-trip baseline by 23 clef files.
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28487 / 36624 |
| Functions | 74.3% | 6349 / 8550 |
| Branches | 50.6% | 22632 / 44725 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 78.4% | 6024 / 7682 |
| Functions | 63.7% | 2045 / 3209 |
| Branches | 47.6% | 5109 / 10729 |
Core HTML report | API HTML report
Commit 52794e60a1ac0c1a2fb50c4e43f4f2f3dcf7d2e9.
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.
Summary
Round-trip added a spurious
number="1"to clefs in single-staff parts. The issue frames this as adropped attribute, but for the affected corpus files it is the reverse: the source omits the clef
number (1 is implied on a single staff) and mx emitted it anyway on mid-measure clef changes. The
measure-start path already suppressed it; the two mid-measure write paths passed the staff index
straight through.
The fix centralizes the single- vs multi-staff rule in
MeasureWriter::clefStaffIndex()so allthree clef write sites agree, and adds
ClefData::writeStaffNumber- a ternary whereunspecifiedapplies the rule automatically (omit the implied 1 on a single-staff part, include otherwise) and
yes/noforce it. Reading setsyes/noonly when the source diverges from the rule, so itstays
unspecifiedin the common case and most users can ignore it.Also includes a tooling fix:
make dump-api-roundtripnow clears its output directory first. Itonly writes pairs for failing files, so a file that flipped to passing left a stale pair that the
classifier re-counted as a current failure.
Testing
MultiStaffClefKeepsNumbertest;PropertiesButNoNotesupdated (it pinned the old bug)References
numberround-trip)