feat: round-trip arpeggiate and non-arpeggiate attributes#340
Merged
Conversation
A source that declares no <divisions> anywhere reads as ticksPerQuarter = 0 (LCM of an empty set). The writer then emitted divisions for the first measure of every part, and core::PositiveDivisions clamped the 0 to a bogus 1e-06. The writer now omits <divisions> when the score has no time resolution; an otherwise-empty <attributes> is no longer flushed. Pins the 41 corpus files this flips to PASS (212 total).
Adds ArpeggiateMarkData (number, unbroken, id) and NonArpeggiateMarkData (top/bottom placement, number, id) as MarkDataChoice alternatives and wires them through the reader and writer. The non-arpeggiate bracket end previously always wrote as type="top". Pins the 5 corpus files this flips to PASS (217 total).
3 tasks
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28514 / 36646 |
| Functions | 74.2% | 6352 / 8555 |
| Branches | 50.7% | 22680 / 44751 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 82.5% | 7035 / 8526 |
| Functions | 71.7% | 2404 / 3353 |
| Branches | 51.2% | 6144 / 12007 |
Core HTML report | API HTML report
Commit baeea48fa068338ad253bbf9a0f300e87ba9343b.
gen-quality
|
webern
added a commit
that referenced
this pull request
Jul 12, 2026
## Human Summary A quick glance and it looks like this is correctly following the pattern for adding `MarkData` features. ## Summary The `<caesura>` articulation's optional text value (MusicXML 3.1+: normal, thick, short, curved, single) was dropped on read, and the writer always emitted `<caesura>normal</caesura>` — so the common empty form `<caesura/>` gained a spurious value and the other variants were flattened to normal. `mx::core` already models the empty form (`core::CaesuraValue::empty()`); only the api impl dropped it. Following the tremoloSingle/fermata-variant precedent, `MarkType::caesura` now means the common empty form and new `caesuraNormal` / `caesuraThick` / `caesuraShort` / `caesuraCurved` / `caesuraSingle` enumerators carry the explicit values, wired through `ArticulationsFunctions` (read) and `NotationsWriter` (write). `isMarkArticulation` and the articulations converter map cover the new enumerators. Stacked on #340 to keep the `roundtrip-baseline.txt` additions conflict-free; the 3 files this flips are pinned (217 -> 220). ## Testing - [x] New `CaesuraEmpty` / `CaesuraVariants` round-trip tests (CaesuraEmpty fails before, passes after) - [x] Full mxtest suite passes (4950 assertions in 420 test cases) - [x] Discovery: 220 PASS, zero regressions; regression mode: 220/220 ## References - Closes #341 - Stacked on #340; 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
It looks like the agent followed the pattern I established with tremolo marks in
MarkDataChoicecorrectly and thus this follows the api standard, such as it is, and adds a couple of missing features.Summary
Arpeggiate marks already round-tripped their direction (encoded in
MarkType::arpeggiate/arpeggiateUp/arpeggiateDown) and shared position attributes, but droppednumber,unbroken(4.0), andid(3.1); non-arpeggiate additionally always wrotetype="top", losing the bottom end of the bracket.Following the
TremoloMarkDataprecedent, this adds twoMarkDataChoicealternatives:ArpeggiateMarkData:number(optional),unbroken(Bool),id(optional)NonArpeggiateMarkData:placement(NonArpeggiatePlacement::top/bottom, the requiredMusicXML
typeattribute),number(optional),id(optional)wired through
ArpeggiateFunctions/NonArpeggiateFunctions(read) andNotationsWriter(write). Authoring without the payload behaves exactly as before.
Stacked on #337 (the arpeggiate corpus fixtures also needed the spurious-divisions fix); the 5
files this flips are pinned in
roundtrip-baseline.txt(212 -> 217).Testing
ArpeggiateAttributes/NonArpeggiateAttributesround-trip tests (fail before, pass after)References