feat: round-trip <encoding> faithfully and add a writeMxVersion toggle#243
Merged
Conversation
…#220) mx::api flattens <encoding> into typed buckets (api::EncodingData) and re-emits its children in a fixed canonical order (encoder, encoding-date, encoding-description, software, supports). Real-world files list those children in other orders, so the round-trip diverged only by <encoding> child order -- a deliberate property of the simplified api, not a fidelity loss. The reorder also misaligned the user <software> against mx's trailing provenance stamp, surfacing a spurious <software> value mismatch. Fix in the api test harness: stable-sort each document's <encoding> children into the api's canonical order before comparing (both expected and actual). Genuine drops/adds/value changes still surface; only pure reordering is normalized away. corert stays order-faithful (untouched). Pins the 48 files this unblocks. api round-trip PASS: 46 -> 94.
…tools mx::api re-emits <encoding> children in a fixed canonical order. Move <software> ahead of <encoding-date> (the order MuseScore/Finale/Sibelius write), which minimizes the number of corpus files whose <encoding> needs reordering on round-trip. MusicXML's encoding choice imposes no order, so any order is schema-valid. The api round-trip harness canonicalizes <encoding> child order on both sides (#220), so this is order-agnostic for the gate: discovery holds at 94 PASS / 737 FAIL, full unit suite green. The harness rank is kept in sync with the writer order for clarity.
mx stamps its own provenance <software> into <encoding> on every api write.
Add api::EncodingData::writeMxVersion (defaults true, including for files
parsed without the stamp) so callers can turn it off. When false, writeToFile/
writeToStream serialize without the stamp via core::serialize() instead of
core::serializeWithAttribution(); the user's own <software> entries are still
written.
The directive cannot ride on the core model, so DocumentManager stores it per
document (StoredDocument) alongside the core::Document and branches at write
time. createFrom{File,Stream} default it true; createFromScore takes it from
score.encoding.writeMxVersion.
Adds a standalone api round-trip test: parse a file (whose source lacked the
stamp), confirm the default write stamps it, then turn writeMxVersion off and
assert the written output no longer contains the mx <software> marker.
webern
added
bug
Claude
gen-quality
|
webern
commented
Jun 26, 2026
| std::vector<SupportedItem> supportedItems; | ||
| std::vector<MiscellaneousField> miscelaneousFields; | ||
|
|
||
| // When true (the default, including for files parsed without it), mx writes its |
Owner
Author
There was a problem hiding this comment.
This should explain that the intent is to aid in fixing mx defects. In other words, since this is a bit of an intrusive defect we should include our reason for doing so.
Owner
Author
There was a problem hiding this comment.
Expanded the comment in 97fb7a2 to give the rationale: the stamp is deliberately intrusive (it injects a <software> node the source never had into every written file), it exists to tie bad output back to the exact mx build that produced it, and this flag is the escape hatch for byte-faithful output when diagnosing mx defects.
Generated by Claude Code
Expand the EncodingData::writeMxVersion comment to state the rationale: mx's provenance stamp is deliberately intrusive (it injects a node the source never had into every written file) and exists to tie bad output back to the mx build that produced it; the flag is the escape hatch for byte-faithful output when diagnosing mx defects.
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% | 5913 / 7535 |
| Functions | 63.5% | 2006 / 3159 |
| Branches | 47.8% | 4998 / 10455 |
Core HTML report | API HTML report
Commit 707dae8bd505fdd482c9607fb23977498b944fd9.
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
Resolves the
<encoding>child-order round-trip divergence (#220) and adds a way to suppress mx's own provenance stamp. Three parts:Encoding child order is treated as insignificant in the api round-trip.
mx::apiflattens<encoding>into typed buckets (EncodingData) and re-emits its children in a fixed canonical order, so files that list those children in any other order round-tripped with an identical multiset but a different order — the sole blocker for a batch of files. The api round-trip harness now stable-sorts<encoding>children into the api's canonical order on both sides before comparing; corert stays order-faithful and is untouched. This also realigns the user<software>against mx's trailing stamp, clearing a secondary spurious value mismatch. Pins the 48 files this unblocks (discovery PASS 46 to 94).The writer now emits
<software>before<encoding-date>, matching what MuseScore, Finale, and Sibelius write, so fewer real-world files need any reordering. MusicXML's encoding choice imposes no order, so this is schema-valid.New
api::EncodingData::writeMxVersion(defaults true, including for files parsed without the stamp). When set false,writeToFile/writeToStreamserialize without mx's provenance<software>while still writing the user's own software entries. The directive cannot ride on the core model, soDocumentManagercarries it per document (StoredDocument) and branches at write time.Testing
make test-api-roundtrip— 94 / 94 pinned files pass (was 46)make test— full unit suite (4275 assertions in 309 cases), including 2 newwriteMxVersionround-trip tests (default-on stamps; off suppresses)make discover-api-roundtrip— 94 PASS / 737 FAIL, no regressions from the writer reordermake check— clang-format cleanReferences