fix: round-trip the metronome parentheses attribute#344
Merged
Conversation
3 tasks
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28514 / 36648 |
| Functions | 74.2% | 6352 / 8556 |
| Branches | 50.7% | 22680 / 44751 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 84.4% | 7229 / 8564 |
| Functions | 74.9% | 2512 / 3353 |
| Branches | 52.3% | 6300 / 12046 |
Core HTML report | API HTML report
Commit 5f502273614b0122ba0751336b75774b3196b83f.
gen-quality
|
TempoData::isParenthetical existed but dangled: the reader never set it and the writer never emitted it. Wire both sides. Pins the 2 corpus files this flips to PASS (222 total).
webern
force-pushed
the
claude/metronome-parentheses
branch
from
4c44177 to
302bca3
Compare
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28514 / 36648 |
| Functions | 74.2% | 6352 / 8556 |
| Branches | 50.7% | 22680 / 44751 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 84.6% | 7243 / 8566 |
| Functions | 74.9% | 2512 / 3353 |
| Branches | 52.4% | 6314 / 12050 |
Core HTML report | API HTML report
Commit 9961fb2812f3ff37c7a6d76bf25d90cd2351b5ac.
gen-quality
|
webern
added a commit
that referenced
this pull request
Jul 12, 2026
…346) ## Human Summary Bug fix related to grace notes not advancing the tick time position. ## Summary Grace notes occupy no ticks (#318), so a grace-note chord shares its `tickTimePosition` with the chord it ornaments. `MeasureWriter`'s chord-boundary detection compared tick positions only, so the first note of the main chord was treated as a continuation of the grace chord and got a spurious `<chord/>` tag — musically corrupting the output by merging the real chord into the zero-duration grace chord. The fix tracks whether the previous chord note was a grace note: a grace/non-grace transition at the same tick is also a chord boundary. Stacked on #344 to keep the `roundtrip-baseline.txt` additions conflict-free; the 2 files this flips are pinned (222 -> 224). ## Testing - [x] New `GraceChordThenChordAtSameTick` MeasureWriter test (fails before, passes after) - [x] Full mxtest suite passes - [x] Discovery: 224 PASS, zero regressions; regression mode: 224/224 ## References - Closes #345 - Stacked on #344; related to #318; part of #208
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
Looks like a bug fix where we purported to support the metronome parenthesis attribute, but dropped it. Simple fix.
Summary
<metronome parentheses="yes">lost its parentheses on an api round-trip. The api modelalready had the field —
TempoData::isParenthetical— but it dangled:MetronomeReadernever set it and
DirectionWriternever emitted it. This wires both sides: the reader mapscore::Metronome::parentheses()into theBool, and the writer emits the attribute when thefield is not
unspecified. No api surface change.Stacked on #342 to keep the
roundtrip-baseline.txtadditions conflict-free; the 2 filesthis flips are pinned (220 -> 222).
Testing
roundTripParenthesesMetronomeApi test (fails before, passes after)References
Generated by Claude Code