chore: strip editorial boilerplate from synthetic feature fixtures#239
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.
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'.
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.
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.
Read and write the editorial-voice <voice> on a <direction> so a direction's voice assignment survives the api round-trip. It is emitted only when the direction also writes direction-type content, so a voice carried in from a figured-bass or harmony element never produces an empty <direction>. Strip the footnote/level editorial boilerplate from the synthetic segno/coda fixtures -- no file in the wild corpus uses editorial, so the api does not model it -- and give each measure a divisions plus a note so it round-trips. Pin the four fixtures in the api read->write->read gate.
The audit generator stamps <footnote>/<level> onto nearly every synthetic single-feature fixture, even though editorial appears in zero real-world corpus files. That noise implied mx::api needed editorial support it does not (see #236, closed). Remove the <footnote>x</footnote>/<level>x</level> boilerplate from every synthetic file except the dedicated footnote/level probes, which keep only their own attributed target. Regenerate the *.features.xml sidecars and corpus.xml.
webern
added
testing
non-breaking
Claude
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.1% | 5874 / 7518 |
| Functions | 62.1% | 1962 / 3159 |
| Branches | 47.5% | 4956 / 10431 |
Core HTML report | API HTML report
Commit ac8922bdc871b8a6a2890b302a9b8ba4b86635bd.
webern
force-pushed
the
claude/issue-237-roundtrip-vsvcas
branch
from
86e5978 to
1366899
Compare
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.1% | 5874 / 7518 |
| Functions | 62.1% | 1962 / 3159 |
| Branches | 47.5% | 4956 / 10431 |
Core HTML report | API HTML report
Commit 8f9b9c9ad3c79b1b50c862ba9c4f5f849e920f17.
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
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 underdata/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.*.features.xmlsidecars andcorpus.xmlregeneratedTesting
clang-formatnot applicableNotes
While regenerating sidecars,
audit files --forcealso corrected a stale<audited-version>(3.0 → 3.1) indata/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
claude/issue-237-roundtrip-vsvcas)