api: segno and coda suport#203
Merged
Merged
Conversation
Widen mx::api SegnoData and CodaData to carry font, smufl, and id in addition to position and color, and add the writer path so segno and coda now survive an api round-trip. The reader set colorData but never isColorSpecified, so color was dropped; the writer emitted nothing for segno or coda.
webern
added
feature
gen-quality
|
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.9% | 28539 / 36620 |
| Functions | 74.4% | 6360 / 8548 |
| Branches | 50.7% | 22672 / 44725 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 71.4% | 5181 / 7260 |
| Functions | 59.9% | 1815 / 3029 |
| Branches | 42.5% | 4322 / 10169 |
Core HTML report | API HTML report
Commit 711beff9b90591806b14f870563027242bf34185.
webern
changed the title
This was referenced Jun 17, 2026
webern
added a commit
that referenced
this pull request
Jun 20, 2026
Fixes #205 and closes #134. `DirectionWriter` was missing a write path for `RehearsalData`, causing rehearsal marks to be silently dropped on output. Add a loop over `directionData.rehearsals` that emits `<rehearsal>` elements, mirroring the segno/coda pattern from PR #203. While wiring up the round-trip test, also fix two gaps in `DirectionReader::parseRehearsal`: it was not reading `fontData` (calling `getFontData`) and was not setting `isColorSpecified` before copying color attributes, so those fields were always lost on the read path. Tests added: - `rehearsalRoundTrip` in `DirectionWriterTest.cpp`: api → core → api round-trip at the impl layer, covering text, position, font, color, and enclosure. - `RehearsalSyntheticFileRead` in `DirectionDataTest.cpp`: loads `data/synthetic/rehearsal.3.1.xml` and asserts the read path surfaces text and enclosure. - `RehearsalRoundTripXml` in `DirectionDataTest.cpp`: full MusicXML serialization/deserialization round-trip via `mxtest::roundTrip` covering text, enclosure, and font weight.
webern
added a commit
that referenced
this pull request
Jun 20, 2026
## Summary `DirectionWriter` had no write path for `RehearsalData`, so rehearsal marks were silently dropped whenever a score was serialized. This adds a loop over `directionData.rehearsals` that emits `<rehearsal>` elements, mirroring the segno/coda pattern from #203. While wiring up the round-trip test, two gaps in `DirectionReader::parseRehearsal` were also found and fixed: it was not calling `getFontData`, and it was not setting `isColorSpecified` before copying color attributes, so both fields were always lost on the read path. The `isColorSpecified` gap is the same bug class described in #207 (which covers the same pattern in `parseWords` — left for a separate PR). A post-review fix also adds `RehearsalEnclosure::unspecified` as the new default for `RehearsalData::enclosure`. Previously the default was `rectangle`, so any rehearsal with no `enclosure` attribute in the source XML would silently gain `enclosure="rectangle"` on write-back. ## Changes - `RehearsalData.h` — added `RehearsalEnclosure::unspecified`; changed `RehearsalData` default enclosure from `rectangle` to `unspecified`. - `DirectionWriter::getDirectionLikeThings` — new loop over `rehearsals` emitting `<rehearsal>` with text, position, font, color, and enclosure (skipped when `unspecified`). - `DirectionReader::parseRehearsal` — now reads `fontData` via `getFontData` and correctly gates `colorData` behind `isColorSpecified`. ## Testing - `rehearsalRoundTrip` (`DirectionWriterTest.cpp`) — api → core → api at the impl layer; covers text, position, font, color, and enclosure shapes. Red before the fix, green after. - `RehearsalSyntheticFileRead` (`DirectionDataTest.cpp`) — loads `data/synthetic/rehearsal.3.1.xml` and asserts the read path surfaces text and enclosure. Pins the core → api direction. - `RehearsalRoundTripXml` (`DirectionDataTest.cpp`) — full MusicXML serialization/deserialization round-trip; covers text, enclosure, and font weight. - `RehearsalUnspecifiedEnclosureNoPhantomAttribute` (`DirectionDataTest.cpp`) — asserts that `enclosure="rectangle"` does not appear in serialized XML when enclosure is unspecified, and that the field round-trips as `unspecified`. - Full suite: 4101 assertions in 269 test cases, all pass. - `make test-api-roundtrip` baseline (1 pinned file): still passes. - `make check` (fmt-check): passes. ## References - Closes #205 - Closes #134 - Partially addresses #207 (fixes the `parseRehearsal` `isColorSpecified` gap; the `parseWords` half of #207 is left for a separate PR) - Related to #203 (segno/coda writer; rehearsal was scoped out of that PR)
webern
added a commit
that referenced
this pull request
Jun 23, 2026
Closes #204. PR #203 added segno/coda support with impl-layer round-trip tests but deferred api-level coverage. This adds it through the public `DocumentManager` path, in two commits. ## Commit 1 — file-walking load coverage Registers the audit fixtures `segno.{3.0,3.1}.xml` and `coda.{3.0,3.1}.xml` (under `data/synthetic/`) in `MxFileRepository::initializeNameSubdirectoryMap()` so the api file-walking suite (`ApiLoadSurvivalTest`) exercises the public read path (`createFromFile -> getData`) over them. The 3.1 variants additionally carry `id` and `smufl`; all four load and yield a part. ## Commit 2 — strict read→write→read gate The synthetic probes cannot serve a strict DOM-equal gate: I dumped expected-vs-actual and confirmed the `<segno>`/`<coda>` elements round-trip byte-identically, but the probes also carry editorial `part-group`/`direction` children (`footnote`/`level`/`voice`) the api omits and have no notes (so the api synthesizes `<divisions>`) — divergences unrelated to segno/coda. The value-only `Fixer` can't patch structural diffs, and the existing real-world files (`testDalSegno.xml`, `testDCalCoda.xml`) fail on an unrelated `encoding-date` ordering issue. So this adds two dedicated minimal fixtures that isolate the segno/coda surface (3.0 base attributes; 3.1 adds `id` + `smufl`) in a score that round-trips losslessly: - `data/custom/segno_coda_roundtrip.3.0.xml` - `data/custom/segno_coda_roundtrip.3.1.xml` They are pinned in `roundtrip-baseline.txt` so the `test-api-roundtrip` CI gate enforces strict `createFromFile -> getData -> createFromScore -> writeToStream` DOM-equality for segno and coda, registered in `MxFileRepository`, and carry their audit `*.features.xml` sidecars. ## Testing - `test-api-roundtrip` regression gate: **34 passed, 0 failed** (incl. both new fixtures). - Full `mxtest`: **4268 assertions in 307 test cases, all pass**. - `clang-format --dry-run --Werror` clean; audit sidecars generated via `python3 -m audit files` (no `corpus.xml` churn). ## References - Closes #204 - Follows #203 (segno/coda support) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- _Generated by [Claude Code](https://claude.ai/code/session_0163ZKdf1FtkiVkpXG2K9C1s)_
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
mx::apiexposedSegnoData/CodaDatabut support was partial: the reader only captured positionand color (and even then never set
isColorSpecified, so color was dropped), and the writer emittednothing for segno or coda, silently losing them on write.
This widens both data classes to the full
<segno>/<coda>attribute surface and makes the triplossless in both directions:
fontData,smufl(+isSmuflSpecified), andid(+isIdSpecified) toSegnoDataandCodaData. Position and halign/valign already ride along inpositionData.DirectionReader::parseSegno/parseCodanow read font, smufl, id, and setisColorSpecified.DirectionWriternow writes segno and coda (position, font, color, smufl, id), replacing thelossy
MX_UNUSEDstub proposed in the superseded PR.Note:
positionDatacarries aplacementmember that does not apply to<segno>/<coda>(placement lives on the parent
<direction>). The read/write helpers are attribute-tolerant(
if constexpr (requires ...)), so this is inert, not a data bug. Documented in the headers.Testing
segnoAndCodaRoundTrip_DirectionWriter: builds a segno and a codawith every field set, writes via
DirectionWriter, reads back viaDirectionReader, assertsequality. Passes (8 assertions in 2 test cases with the existing ottava test).
make devbuilds clean;make fmtapplied.make testdeferred to CI.TODO (planned, not done in this PR)
data/synthetic/segno.{3.0,3.1}.xmlandcoda.{3.0,3.1}.xml(register them inMxFileRepository) so the publicDocumentManagerpath isexercised, not just the impl layer. These fixtures already carry every attribute (the 3.1 variants
add
idandsmufl).DirectionWriter#205 Implement rehearsal writing inDirectionWriter. Rehearsals are read but not written; PR Add support for segno reading #134bundled a lossy rehearsal writer. Scoped out here to keep this segno/coda-only.
DirectionWriterignoresorderedComponents— interleaved direction-type children lose original order on write #206 HonororderedComponentsinDirectionWriter. The writer emits each component kind in its ownloop (segnos, then codas), so direction-type children interleaved with other kinds are not
guaranteed to round-trip in original order. Pre-existing limitation surfaced by this work.
parseWordsnever setsWordsData.isColorSpecified— words color does not round-trip #207 Fix the same latent bug inparseWords:WordsData.isColorSpecifiedis never set on read, sowords color does not round-trip (same bug class fixed here for segno/coda).
References