test: api-level segno/coda round-trip coverage#235
Merged
Conversation
…suite Closes #204. Registers 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 DocumentManager read path over them. The 3.1 variants additionally carry id and smufl; all four load and yield a part. segno/coda write fidelity is already covered at the impl layer by the segnoAndCodaRoundTrip DirectionWriter test (#203).
Adds dedicated minimal fixtures custom/segno_coda_roundtrip.{3.0,3.1}.xml that
isolate the segno/coda surface (3.0 base attributes; 3.1 adds id + smufl) in a
score that round-trips losslessly through the public DocumentManager
createFromFile -> getData -> createFromScore -> writeToStream path, and pins
them in roundtrip-baseline.txt so the test-api-roundtrip CI gate enforces
strict DOM-equal read->write->read for segno and coda.
The audit synthetic/segno.*.xml and synthetic/coda.*.xml probes cannot serve
this gate: they carry editorial part-group/direction children
(footnote/level/voice) the api omits and have no notes (so the api synthesizes
<divisions>), diverging for reasons unrelated to segno/coda. Those remain
covered for load survival via the file-walking suite (previous commit).
Also registers the two fixtures in MxFileRepository and adds their audit
*.features.xml sidecars.
gen-quality
|
The two custom/segno_coda_roundtrip.{3.0,3.1}.xml fixtures are eligible corpus
files, so the corert pinned-counts case (intentionally fails on count drift)
must be updated. Both round-trip through mx::core cleanly (832 corert cases
pass). Also regenerates the audit data/corpus.xml aggregate (831 total, 442
wild) via 'python3 -m audit all'.
webern
commented
Jun 22, 2026
Renames ApiLoadSurvivalTest -> ApiLoadSmokeTest (class, file, display name) and documents that it only proves a file imports without crashing, not that the data is correct -- the read->write->read gate is the correctness check. Also adds, in the fewest lines possible, the data/ corpus invariants that bit this PR's CI: adding/removing a corpus file requires bumping the CoreRoundtripTest.cpp pinned count and running 'make audit', and 'make check' is clang-format only.
Per review: every numeric/color attribute now carries a distinct, non-default value (default-x=1.1, default-y=2.2, relative-x=3.3, relative-y=4.4, font-size differs per element, color=#12345678 / #9ABCDEF0, halign/valign differ) so a writer or reader that assigns a value to the wrong attribute is caught instead of hiding behind uniform 1s and #FF000000. Colors are uppercase per the MusicXML color schema type. Both variants still round-trip exactly.
gen-quality
|
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.9% | 28539 / 36624 |
| Functions | 74.4% | 6360 / 8550 |
| Branches | 50.7% | 22672 / 44725 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 78.0% | 5860 / 7509 |
| Functions | 62.1% | 1962 / 3159 |
| Branches | 47.4% | 4939 / 10419 |
Core HTML report | API HTML report
Commit 53e533695913cca23fc99e5daa32d17ab07fa28e.
This was referenced Jun 22, 2026
webern
commented
Jun 22, 2026
Address review feedback: reword the ApiLoadSmokeTest comment so it no longer says the import 'yields a part' (it asserts a non-empty parts list), and trim the segno/coda baseline comment to state what the fixtures are rather than transitory context about other files.
gen-quality
|
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.9% | 28539 / 36624 |
| Functions | 74.4% | 6360 / 8550 |
| Branches | 50.7% | 22672 / 44725 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 78.0% | 5860 / 7509 |
| Functions | 62.1% | 1962 / 3159 |
| Branches | 47.4% | 4939 / 10419 |
Core HTML report | API HTML report
Commit 6bc7f972df029771941e77c1c0f06a9cb2552603.
This was referenced Jun 22, 2026
webern
added a commit
that referenced
this pull request
Jun 23, 2026
…tures (#238) Brings the audit's synthetic `segno`/`coda` fixtures onto the strict api read -> write -> read gate. ## What 1. **Round-trip the editorial-voice `<voice>` on a `<direction>`** (read from `editorialVoiceDirection().voice()`, written back). Today a direction's own `<voice>` is dropped. The write is emitted only when the direction has direction-type content, so a voice carried in from a figured-bass/harmony element never produces an empty `<direction>`. `<voice>` is common in the wild corpus, so this is real fidelity, not synthetic-only. 2. **Pin `synthetic/{segno,coda}.{3.0,3.1}.xml`** in `roundtrip-baseline.txt`. To make them round-trip losslessly the fixtures are tidied: - stripped the `<footnote>`/`<level>` editorial boilerplate the audit generator stamps onto every probe (editorial appears in **zero** real-world files, so the api does not model it — see #236, closed) - added a `<divisions>` + a `<note>` so the writer's first-measure `<divisions>` is legitimate instead of a spurious injection into a note-less measure They still isolate the segno/coda surface (3.0 base attributes; 3.1 adds id + smufl) and now also exercise a part-group and direction `<voice>`/`<staff>` through the public `DocumentManager` path. ## Why a real note instead of an impl fix for `<divisions>` A note-less measure that the writer fills with a synthesized `<divisions>` is the #228 writer-policy issue. A naive "suppress divisions when the part has no notes" guard regresses `ksuite/k016a_Miscellaneous_Fields.xml`, which legitimately carries `<divisions>` in a note-less measure — distinguishing "source had it" from "api invented it" needs divisions-presence tracking in the public api, out of scope here. So the fixtures carry a real note; the writer-policy gap stays open under #228. ## Follow-up A separate PR on top of this one strips the editorial boilerplate from every *other* synthetic file that isn't the dedicated `footnote`/`level` probe, so the synthetic corpus stops implying api features the wild corpus never uses. ## Testing - [x] api round-trip regression gate: 38 passed, 0 failed (was 34; +4 synthetic) - [x] discovery: no regressions - [x] Full `mxtest`: 4268 assertions in 307 test cases, all pass - [x] corert: 832 test cases pass, pinned corpus count unchanged - [x] `clang-format` clean; audit `*.features.xml` sidecars + `corpus.xml` regenerated ## References - Stacked on #235 / #204 (segno/coda api round-trip) - Supersedes #236 (editorial dropped); resolves the direction-voice part of #237, with the `<divisions>` writer-policy part staying with #228 - Part of #208 (api round-trip coverage)
webern
added a commit
that referenced
this pull request
Jun 23, 2026
) ## Summary The audit generator stamps `<footnote>`/`<level>` editorial onto nearly every synthetic single-feature fixture, but editorial appears in zero real-world corpus files — all 382 editorial-bearing files were under `data/synthetic/`. That boilerplate implied mx::api needed editorial support it does not actually need for the wild corpus, which is why #236 was closed without merging. This removes the `<footnote>x</footnote>`/`<level>x</level>` boilerplate from every synthetic fixture except the dedicated footnote/level probes, which keep only their own target element. One uniform rule does this safely: the boilerplate is always the plain `>x<` form, while each probe's target is always attributed (`<footnote default-x=...>`, `<level reference=...>`), so deleting the plain form strips the noise everywhere and preserves every probe's intent. - 382 fixtures changed (379 stripped fully; the footnote/level probes keep their attributed target) - `*.features.xml` sidecars and `corpus.xml` regenerated ## Testing - [x] corert: 832 test cases pass, pinned corpus count unchanged (the now-empty part-groups round-trip) - [x] api round-trip regression gate: 38/38 - [x] Full mxtest: 4268 assertions in 307 test cases (ApiLoadSmokeTest walks every stripped file) - [x] No code changes; `clang-format` not applicable ## Notes While regenerating sidecars, `audit files --force` also corrected a stale `<audited-version>` (3.0 → 3.1) in `data/custom/segno_coda_roundtrip.3.1.features.xml`. That file belongs to #235, so the correction was reverted out of this PR to keep it focused — worth fixing on the #235 branch. ## References - Stacked on #238 (base branch `claude/issue-237-roundtrip-vsvcas`) - Follows from #236 (editorial dropped from mx::api; closed) - Progresses #221 — footnote/level are no longer dropped across ~373 classifier files now that the synthetic boilerplate is gone - Part of #208 (api round-trip corpus coverage)
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.
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
DocumentManagerpath, in two commits.Commit 1 — file-walking load coverage
Registers the audit fixtures
segno.{3.0,3.1}.xmlandcoda.{3.0,3.1}.xml(underdata/synthetic/) inMxFileRepository::initializeNameSubdirectoryMap()so the api file-walking suite (ApiLoadSurvivalTest) exercises the public read path (createFromFile -> getData) over them. The 3.1 variants additionally carryidandsmufl; 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 editorialpart-group/directionchildren (footnote/level/voice) the api omits and have no notes (so the api synthesizes<divisions>) — divergences unrelated to segno/coda. The value-onlyFixercan't patch structural diffs, and the existing real-world files (testDalSegno.xml,testDCalCoda.xml) fail on an unrelatedencoding-dateordering 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.xmldata/custom/segno_coda_roundtrip.3.1.xmlThey are pinned in
roundtrip-baseline.txtso thetest-api-roundtripCI gate enforces strictcreateFromFile -> getData -> createFromScore -> writeToStreamDOM-equality for segno and coda, registered inMxFileRepository, and carry their audit*.features.xmlsidecars.Testing
test-api-roundtripregression gate: 34 passed, 0 failed (incl. both new fixtures).mxtest: 4268 assertions in 307 test cases, all pass.clang-format --dry-run --Werrorclean; audit sidecars generated viapython3 -m audit files(nocorpus.xmlchurn).References
🤖 Generated with Claude Code
Generated by Claude Code