diff --git a/data/README.md b/data/README.md new file mode 100644 index 000000000..03e09733e --- /dev/null +++ b/data/README.md @@ -0,0 +1,22 @@ +# data/ + +MusicXML test inputs and expected-output fixtures. + +## `.invalid` marker convention + +A file `foo.xml` (or `foo.musicxml`) that is **not** valid MusicXML must be +accompanied by a sibling marker file `foo.xml.invalid` whose contents +describe, in prose, why the file is invalid. + +The marker tells schema-strict consumers (notably the **core roundtrip** +suite at `src/private/mxtest/corert/`) to skip the file. `mx::core` is a +strongly-typed DOM generated from the MusicXML XSD and cannot round-trip +elements or attributes that are not in the schema; running such a file +through `fromXDoc` → `toXDoc` is guaranteed to lose content. + +The marker is descriptive only — its body is for humans. Presence of the +file is the signal. + +Other suites (e.g. **api import** under `src/private/mxtest/import/`) may +still include invalid files when they exercise reader leniency on +malformed input. Those suites ignore the marker. diff --git a/data/foundsuite/Deutscher Tanz D.820.1.xml.invalid b/data/foundsuite/Deutscher Tanz D.820.1.xml.invalid new file mode 100644 index 000000000..63082016c --- /dev/null +++ b/data/foundsuite/Deutscher Tanz D.820.1.xml.invalid @@ -0,0 +1,14 @@ +Not valid against the MusicXML XSD. + +Confirmed by xmllint --schema docs/musicxml.xsd. Violations: + +- midi-program with value 0 (schema midi-128: must be 1..128) at line 49. +- sound/@dynamics='-1.11' (minInclusive 0) at lines 167 and 469. + +The core roundtrip suite trips on the midi-program=0 case: mx::core clamps +to the schema-legal range (writes 1), causing a text mismatch against the +original 0. Excluding this file from the schema-strict core roundtrip is +the right call — the library is conforming to the schema and the input is +the deviation. + +See data/README.md for the .invalid marker convention. diff --git a/data/foundsuite/O_Holy_Night-Adam-1871.xml.invalid b/data/foundsuite/O_Holy_Night-Adam-1871.xml.invalid new file mode 100644 index 000000000..78fd0b4ff --- /dev/null +++ b/data/foundsuite/O_Holy_Night-Adam-1871.xml.invalid @@ -0,0 +1,15 @@ +Not valid against the MusicXML XSD. + +Confirmed by xmllint --schema docs/musicxml.xsd. Violations include: + +- midi-channel with value 0 (schema midi-16: must be 1..16) at line 23. +- Many duration elements with value 0 (minExclusive 0), starting at line 9706 + and continuing for dozens of occurrences. + +The core roundtrip suite trips on midi-channel=0: mx::core clamps to the +schema-legal range (writes 1), causing a text mismatch against the +original 0. Excluding this file from the schema-strict core roundtrip is +the right call — the library is conforming to the schema and the input is +the deviation. + +See data/README.md for the .invalid marker convention. diff --git a/data/foundsuite/O_Holy_Night.xml.invalid b/data/foundsuite/O_Holy_Night.xml.invalid new file mode 100644 index 000000000..2f3ad255e --- /dev/null +++ b/data/foundsuite/O_Holy_Night.xml.invalid @@ -0,0 +1,20 @@ +Not valid against the MusicXML XSD. + +Confirmed by xmllint --schema docs/musicxml.xsd. Violations include: + +- midi-channel with value 0 (schema midi-16: must be 1..16) at lines 22, 32, + 42, 52. +- midi-program with value 0 (schema midi-128: must be 1..128) at lines 23, + 33, 43, 53. +- display-step with value '=' (enum is A..G) at lines 408, 711, 1337, 1791, + 2025. +- display-octave with value -104 (minInclusive 0) at adjacent lines. +- Many duration elements with value 0 (minExclusive 0) starting at line 2584. + +The core roundtrip suite trips on midi-channel=0: mx::core clamps to the +schema-legal range (writes 1), causing a text mismatch against the +original 0. Excluding this file from the schema-strict core roundtrip is +the right call — the library is conforming to the schema and the input is +the deviation. + +See data/README.md for the .invalid marker convention. diff --git a/data/foundsuite/Rimsky-Korsakov Op11 No4.xml.invalid b/data/foundsuite/Rimsky-Korsakov Op11 No4.xml.invalid new file mode 100644 index 000000000..6a039ca62 --- /dev/null +++ b/data/foundsuite/Rimsky-Korsakov Op11 No4.xml.invalid @@ -0,0 +1,14 @@ +Not valid against the MusicXML XSD. + +Confirmed by xmllint --schema docs/musicxml.xsd. Violations: + +- sound/@dynamics with value '-1.11' (minInclusive 0) at lines 6224, 8231, + 11057, 12922, 13151, 13389, 14002. + +The core roundtrip suite trips on this attribute: mx::core clamps the +negative value to the schema-legal range, causing an attribute mismatch +against the original. Excluding this file from the schema-strict core +roundtrip is the right call — the library is conforming to the schema and +the input is the deviation. + +See data/README.md for the .invalid marker convention. diff --git a/data/lysuite/ly01e_Pitches_ParenthesizedAccidentals.xml.invalid b/data/lysuite/ly01e_Pitches_ParenthesizedAccidentals.xml.invalid new file mode 100644 index 000000000..8d55923a6 --- /dev/null +++ b/data/lysuite/ly01e_Pitches_ParenthesizedAccidentals.xml.invalid @@ -0,0 +1,14 @@ +Not valid against the MusicXML XSD. + +Confirmed by xmllint --schema docs/musicxml.xsd. Violations: + +- accidental element text 'double-flat' is not in the schema enumeration at + lines 138, 149, 160, 171. The schema-correct spelling is 'flat-flat'. + +The core roundtrip suite trips on this: mx::core sees an unknown enum value +and falls back to a default ('sharp'), producing a text mismatch against the +original 'double-flat'. Excluding this file from the schema-strict core +roundtrip is the right call — the input is using a name that is not in the +schema enumeration. + +See data/README.md for the .invalid marker convention. diff --git a/data/lysuite/ly32a_Notations.xml.invalid b/data/lysuite/ly32a_Notations.xml.invalid new file mode 100644 index 000000000..42d8ef9c0 --- /dev/null +++ b/data/lysuite/ly32a_Notations.xml.invalid @@ -0,0 +1,13 @@ +Not valid against the MusicXML XSD. + +Confirmed by xmllint --schema docs/musicxml.xsd. Violations: + +- empty at line 900; schema requires xs:nonNegativeInteger. +- empty at line 924; schema requires string-number. + +The core roundtrip suite trips on the empty : mx::core defaults the +missing integer to 0, producing a text mismatch ('' vs '0') against the +original empty element. Excluding this file from the schema-strict core +roundtrip is the right call — the input is omitting required content. + +See data/README.md for the .invalid marker convention. diff --git a/data/lysuite/ly41g_PartNoId.xml.invalid b/data/lysuite/ly41g_PartNoId.xml.invalid new file mode 100644 index 000000000..a49b6b0a5 --- /dev/null +++ b/data/lysuite/ly41g_PartNoId.xml.invalid @@ -0,0 +1,12 @@ +Not valid against the MusicXML XSD. + +Confirmed by xmllint --schema docs/musicxml.xsd. Violation: + +- at line 16 is missing the required 'id' attribute. + +The filename literally says PartNoId. mx::core's fromXDoc rejects the +document with "PartAttributes: 'id' is a required attribute but was not +found" — i.e. the parser is correctly enforcing the schema. Excluding this +file from the schema-strict core roundtrip is the right call. + +See data/README.md for the .invalid marker convention. diff --git a/data/lysuite/ly74a_FiguredBass.xml.invalid b/data/lysuite/ly74a_FiguredBass.xml.invalid new file mode 100644 index 000000000..577d03e01 --- /dev/null +++ b/data/lysuite/ly74a_FiguredBass.xml.invalid @@ -0,0 +1,13 @@ +Not valid against the MusicXML XSD. + +Confirmed by xmllint --schema docs/musicxml.xsd. Violation: + +- at line 90 is missing the required child element
+ (figured-bass requires at least one figure child per schema). + +The core roundtrip suite trips on this: the empty figured-bass round-trips +to a different child count than the (canonicalized) input. Excluding this +file from the schema-strict core roundtrip is the right call — the input +is omitting a required child. + +See data/README.md for the .invalid marker convention. diff --git a/data/lysuite/ly75a_AccordionRegistrations.xml.invalid b/data/lysuite/ly75a_AccordionRegistrations.xml.invalid new file mode 100644 index 000000000..9528affcd --- /dev/null +++ b/data/lysuite/ly75a_AccordionRegistrations.xml.invalid @@ -0,0 +1,15 @@ +Not valid against the MusicXML XSD. + +Confirmed by xmllint --schema docs/musicxml.xsd. Violations: + +- empty at line 291 (must be 1..3). +- test at line 307 (must be 1..3). +- 0 at line 323 (minInclusive 1). +- 5 at line 339 (maxInclusive 3). + +The core roundtrip suite trips on the empty accordion-middle: mx::core +defaults the missing integer to 0, producing a text mismatch ('' vs '0'). +Excluding this file from the schema-strict core roundtrip is the right +call — the input is using values outside the schema-allowed range. + +See data/README.md for the .invalid marker convention. diff --git a/data/musuite/testInvalid.xml.invalid b/data/musuite/testInvalid.xml.invalid new file mode 100644 index 000000000..d3055f10f --- /dev/null +++ b/data/musuite/testInvalid.xml.invalid @@ -0,0 +1,15 @@ +testInvalid.xml is intentionally not a valid MusicXML file. + +Per its embedded : "This file is almost +identical to testHello.xml, but contains two additional invalid elements to +test MusicXML validation." Specifically: + +- appears inside +- appears inside + +Neither element exists in the MusicXML XSD. mx::core is a strongly-typed DOM +generated from the schema and has no slot for unknown elements, so fromXDoc +discards them. The file therefore cannot round-trip losslessly through +mx::core and is excluded from the core roundtrip suite via this marker. + +See data/README.md for the .invalid marker convention. diff --git a/data/musuite/test_harmony.xml.invalid b/data/musuite/test_harmony.xml.invalid new file mode 100644 index 000000000..704a9fc51 --- /dev/null +++ b/data/musuite/test_harmony.xml.invalid @@ -0,0 +1,18 @@ +Not valid against the MusicXML XSD. + +Confirmed by xmllint --schema docs/musicxml.xsd. Violations include: + +- values not in the schema enumeration: '' (empty, line 73), ' ' + (whitespace, line 82), 'minor-major' (195), 'dominant-seventh' (204, 488, + 502, 522, 546), 'neapolitan' (316), 'italian' (326), 'french' (335), + 'german' (344), 'maj69' (355), 'augmented-ninth' (364), 'altered' (374). +- child appearing where is expected, at lines + 388, 401, 597, 610, 614 — element order violates the schema sequence. + +The core roundtrip suite trips on the empty : mx::core defaults the +missing enum to 'major', producing a text mismatch ('' vs 'major'). +Excluding this file from the schema-strict core roundtrip is the right +call — the input is full of values and orderings that are not in the +schema. + +See data/README.md for the .invalid marker convention. diff --git a/docs/ai/projects/gen/index.md b/docs/ai/projects/gen/index.md index f174e79b0..96b330b17 100644 --- a/docs/ai/projects/gen/index.md +++ b/docs/ai/projects/gen/index.md @@ -2,21 +2,28 @@ created: 2026-05-18 m1_completed: 2026-05-21 m2_completed: 2026-05-22 +m3_completed: 2026-05-22 --- # gen ## Goal -Reverse engineer the codegen process that produced `mx/core` from MusicXML XSD. Build a generator -that re-produces the existing C++ code from `docs/musicxml.xsd`, then point it at MusicXML 4.0 to -generate updated types. +Reverse engineer the codegen process that produced `mx/core` from MusicXML XSD. Build a +generator that re-produces the existing C++ code from `docs/musicxml.xsd`, then point it +at MusicXML 4.0 to generate updated types. ## Files -- `plan.md` - milestones and exit criteria -- `state.md` - current session state and next-session instructions -- `log.md` - append-only session log +Standard project layout (see the `/project` skill). + +## Repo conventions introduced by this project + +- `{file}.invalid` marker (sibling file, human-readable body) marks a MusicXML input that + is intentionally not XSD-valid. Honored by the corert suite, ignored by api import. + Documented in `data/README.md`. Introduced in M3. +- The `src/private/mxtest/corert/` core-roundtrip harness (built out during M2/M3) is + part of `make test-all` and is the daily driver via `make test-core-dev`. ## Generator location and how to run @@ -27,40 +34,42 @@ python3 gen/generate.py # regenerates C++ into src/private/mx/core/elem python3 gen/eval.py # scores diff against checked-in mx/core (secondary signal) ``` -Workflow: `python3 gen/generate.py && make fmt && make test-all`, then reset: +Workflow: `python3 gen/generate.py && make fmt && make test-all`. To reset: `git checkout -- src/private/mx/core/ && git clean -fd src/private/mx/core/`. -Quality gates: `make fmt && make check && make test-all`. - -## Fitness function +## Fitness function and gates -`make test-all` pass/fail. **Always use `make test-all`, never `make test`** — the latter builds -`dev` and skips the slow `mxtest/file/` round-trippers, under-reporting failures. `make test-all` -takes >10 minutes. Every iteration ends with a recorded `make test-all` result. +`make test-all` pass/fail is authoritative. **Use `make test-all`, never `make test`** — +the latter builds `dev` and skips the slow `mxtest/file/` round-trippers. Full gate: +`make fmt && make check && make test-all`. Every iteration that touches +`src/private/mx/core/*` ends with a recorded `make test-all` result. ## Cardinal rules -- Never change tests. +- Never change test cases. Test harness code is fair game with user authorization. - Never change `mx/api`. - Minimize changes to `mx/impl`; prefer fixing the generator. -- Do not autonomously edit `gen/eval_config.yaml`. Flag patterns to the user with sample diff + reasoning. -- Reset generated `mx/core/` before commit. +- Do not autonomously edit `gen/eval_config.yaml`. Flag patterns to the user with sample + diff + reasoning. +- Reset generated `mx/core/` before commit if a generator change was tried and reverted. -## Bespoke generator functions +## Bespoke generator handlers -Six bespoke handlers exist (credit, lyric, part-list, harmony, score-wrapper-family, note, -direction), registered in `BESPOKE_ELEMENTS` in `gen/generate.py`. They are acceptable when an -element's shape cannot be expressed by the shared rule-based path, but they must still read the -parsed XSD model so spec changes propagate. Pattern: "custom algorithm, schema-driven data" — not -a hardcoded string dump. +Six bespoke handlers in `BESPOKE_ELEMENTS` (`gen/generate.py`): credit, lyric, part-list, +harmony, score-wrapper-family, note, direction. Acceptable when an element's shape cannot +be expressed by the shared rule-based path, but they must still read the parsed XSD model +so spec changes propagate. Pattern: "custom algorithm, schema-driven data" — not a +hardcoded string dump. -Always prefer extending a reusable rule-based path (`TREE_ELEMENTS`, `TREE_ELEMENT_CONFIG` flags, -shared helpers) over a new bespoke handler. If a pattern could plausibly recur for another -element, the fix belongs in the shared path with a config-driven flag. +Always prefer extending a reusable rule-based path (`TREE_ELEMENTS`, +`TREE_ELEMENT_CONFIG` flags, shared helpers) over a new bespoke handler. If a pattern +could plausibly recur for another element, the fix belongs in the shared path with a +config-driven flag. ## Key external files -- `gen/generate.py` — the generator (~2800 lines of Python) +- `gen/generate.py` — the generator (~14k lines of Python) - `gen/eval.py`, `gen/eval_config.yaml` — diff scoring -- `docs/musicxml.xsd` — input schema (currently MusicXML 3.x; swapped to 4.0 in M5) -- `src/private/mx/core/elements/` — target output (591 .h/.cpp pairs) +- `docs/musicxml.xsd` — input schema (currently MusicXML 3.x; swap to 4.0 in M6) +- `src/private/mx/core/elements/` — target output (~590 .h/.cpp pairs) +- `src/private/mxtest/corert/` — core-roundtrip harness diff --git a/docs/ai/projects/gen/log.md b/docs/ai/projects/gen/log.md index d01172add..39badfbbe 100644 --- a/docs/ai/projects/gen/log.md +++ b/docs/ai/projects/gen/log.md @@ -2,102 +2,81 @@ Append chronologically, oldest on top. -## M1: revgen (2026-05-18 — 2026-05-21, 40 iterations) +## M1: revgen (2026-05-18 — 2026-05-21, 40 iterations) ✅ -Reverse-engineered the codegen. Iteratively shrank `SKIP_ELEMENTS` and `CHOICE_SKIP` to empty. -Closed 2026-05-21 with the generator producing every C++ class in `mx/core`. Tests failing — the -commit `d4f25ee6` "src: issues caused by revgen" hand-edits non-generated consumers to keep -the build working; those hand-edits were the input to M2. +Reverse-engineered the codegen, iteratively shrinking `SKIP_ELEMENTS` and `CHOICE_SKIP` to +empty. Closed with the generator producing every C++ class in `mx/core`. Tests still +failing; commit `d4f25ee6` "src: issues caused by revgen" carries hand-edits to +non-generated consumers that kept the build working — the input to M2. -## M2: fix-gen (2026-05-21 — 2026-05-22) +## M2: fix-gen (2026-05-21 — 2026-05-22) ✅ -Triaged `d4f25ee6` into 6 root-cause issues (A–F), then triaged 62 (later 129 after a clean -build) `make test-all` failures into clusters. Fixed iteratively, ending at 0 failures. +Triaged `d4f25ee6` into 6 root causes (A–F) and ~129 `make test-all` failures into clusters +(R1–R7, D1–D4, plus an isFirst-separator family). Fixed iteratively to 0 failures. -### Triage of `d4f25ee6` hand-edits +Notable mechanisms introduced (still in `gen/generate.py`): +- `EXTENSION_OPTIONAL_GROUP_RENAME`, `SUPPRESS_GROUP_SUFFIX`, + `WRAPPER_AS_ELEMENT_SYNTH_GROUPS` (MetronomeTuplet group flattening, issue C). +- `TREE_ELEMENT_CONFIG["key"] = {"parent_imports_choice_groups": True}` → + `Key::import{Traditional,NonTraditional}Key` (issues E/F). +- Required-set seeding rule in `generate_element_cpp` + (`min_occurs>=1, max_occurs!=1, !is_group`) fixed HarpPedals SIGSEGV (R5). +- `is_container` + `trigger_names` on `TreeChoiceBranch`; `importContainer` dispatch + in `generate_tree_parent_cpp` (D1, D4 — Metronome reader). +- `ATTR_DEFAULT_OVERRIDE` (~17 entries), `CHILD_INIT_VALUE_OVERRIDE` (Scaling, + StaffDetails). +- `_emit_direction_family` bespoke driven by + `model.complex_types["direction-type"].content_tree`. +- `ELEMENT_HAS_CONTENTS_ALWAYS_TRUE` (MeasureLayout, NoteheadText), notehead-text + `seed_choice_set`, MeasureLayout explicit child-presence check for `isOneLineOnly`. -- **A — UpDownNone collapsed to UpDown** (WEIRD / Low): Hand-applied MusicXML 4.0 backport - in original `mx/core` overwritten by schema-faithful 3.x regen. Deferred to M5; TODO - comments left in `mx/impl`. -- **B — `hasLong` escaped to `hasLong_`** (BUG / Low): Keyword-escape over-applied to the - has-prefix flag. Fix: `has_flag_name` strips trailing underscore added by `camel()` for - keywords. -- **C — MetronomeTuplet group flattened** (BUG / High): Regen flattened - `TimeModificationNormalTypeNormalDot` into `MetronomeTuplet`, making `` - unconditional. Fix: added `EXTENSION_OPTIONAL_GROUP_RENAME` (per-extending-type override) + - `SUPPRESS_GROUP_SUFFIX` + `WRAPPER_AS_ELEMENT_SYNTH_GROUPS` so the wrapper survives as a - sub-element rather than being inlined. -- **D — `setPerMinuteOtBeatUnitChoice` typo** (BENIGN): Historical typo in original; accepted - small test edit, no generator change. -- **E/F — Missing `importGroup` overloads for traditional-/non-traditional-key** (BUG / - Medium): Fix path option 1: emit `Key::importTraditionalKey` / `Key::importNonTraditionalKey` - as private member functions via new `TREE_ELEMENT_CONFIG["key"] = {"parent_imports_choice_groups": True}`. - Reverted hand-edits in `FromXElement.{h,cpp}`. +Deferred: issue A — original `mx/core` had a hand-applied MusicXML 4.0 `UpDownNone` +backport overwritten by schema-faithful 3.x regen. TODO comments in +`mx/impl/NotationsWriter.cpp:398` and `mx/impl/ArpeggiateFunctions.cpp:35`. Belongs to +M5/M6. -### Failing-test clusters +### Lessons captured (operational invariants) -Original triage scheme R1–R7 was rewritten mid-M2 as the situation evolved. Final shape: - -- **R1 — direction-family parents emit `MX_FROM_XELEMENT_UNUSED`** (19 tests): split into - R1a (ArrowGroup needs real `fromXElementImpl` body — added "arrow" to - `GROUPS_WITH_REAL_FROM_X_ELEMENT`) and R1b (Direction parent bespoke — added - `_emit_direction_family` handler driven by `model.complex_types["direction-type"].content_tree.branches`). -- **R5 — required-set seeding** (10 tests including HarpPedals SIGSEGV): added structural - rule in `generate_element_cpp` for `min_occurs>=1, max_occurs!=1, not is_group` children: - ctor pre-seeds default, `hasContents()` returns true, `remove*` gates on `size>1`, - `clear*Set` re-seeds. -- **R3 — group-class `streamContents` inter-child `endl`** (~10 tests): rewrote - `generate_group_cpp` separator logic — required-after-required: unconditional endl before; - required-after-optional: optional's block emits; index-0 optional: endl after. -- **R2 — choice-class spurious leading `endl`** (~7 tests): gated on - `config.get("skip_parent")` in `generate_choice_class_cpp` so only `direction-type`-style - wrapper choices keep the leading endl. -- **R4 — attribute default initializers** (bulk fix, ~22 tests): seeded 17 entries in - `ATTR_DEFAULT_OVERRIDE` (lang="it" defaults, lineEnd=down, number="1", etc.). Continued - in i5 with `CHILD_INIT_VALUE_OVERRIDE` for `Scaling`/`StaffDetails`. -- **hasContents direction cluster** (6 tests): Rest/Unpitched gated optional groups on - `myHas`; MeasureLayout/NoteheadText/Slash/BeatRepeat needed - `ELEMENT_HAS_CONTENTS_ALWAYS_TRUE` / `CHILD_MIN_OCCURS_OVERRIDE` / notehead-text - `seed_choice_set`. -- **group/tree-group isFirst separator** (i4/i6, ~30 tests): `generate_group_cpp` and - `generate_tree_group_cpp` were emitting unconditional `endl` before each set-based child - even when all preceding optional singletons were absent. Fixed with `isFirst` flag - pattern. -- **MeasureLayout isOneLineOnly**: explicit child-presence check instead of `hasContents()` - for elements with `ELEMENT_HAS_CONTENTS_ALWAYS_TRUE`. - -### Final D-cluster (Metronome/Tempo round-trip) +- `git checkout -- src/private/mx/core/` preserves mtimes; incremental cmake then links + partly-old `.o` files and reports stale test counts. Use `make clean && make test-all` + for authoritative measurements. +- `make test-all` must run with generated files present — HEAD's `UpDownNone` backport + is incompatible with a schema-faithful regen, so a reset-first build will not compile. +- When removing a previously-emitted byte from a shared template, survey the whole HEAD + population that template emits (R2 burned us by regressing DirectionType). +- Bespoke handlers should still read the parsed XSD model — "custom algorithm, + schema-driven data". -After mass cleanup, 13 failures remained, all metronome/tempo. Re-triaged as D1–D4: +## M3: fix-core-dev (2026-05-22, 5 iterations) ✅ -- **D3 — `MetronomeAttributes` missing `halign`/`justify` defaults** (`LeftCenterRight::center`): - trivial `ATTR_DEFAULT_OVERRIDE` entries. -- **D1 — choice classes called `toStream` for empty-`streamName` containers** (BeatUnitPer, - NoteRelationNote produced literal `<>...`): added `is_container` field to - `TreeChoiceBranch`; emit `streamContents` instead of `toStream` for container branches in - `generate_tree_choice_cpp`. -- **D2 — `DirectionWriter.cpp:370` throws on non-BPM tempos**: bisect confirmed pre-revgen - `a0500803` did pass these tests, so revgen introduced the regression in the reader/writer - path. Subsumed by D4. -- **D4 — Metronome container-branch `fromXElementImpl` missing**: `generate_tree_parent_cpp` - emitted no dispatch for `is_container=True` branches. Synthetic containers have no XML - tag, so the reader silently skipped all metronome children. Fix: added `trigger_names` - field to `TreeChoiceBranch` (computed from first child element name); emit - `importContainer` declarations and bodies in the parent; dispatch in - `fromXElementImpl` keyed on trigger names. Generated `Metronome::fromXElementImpl` matches - HEAD exactly. 9 → 0 failed. **M2 complete.** +Each iteration picked the smallest core-roundtrip diff in `data/testOutput/corert` and +triaged it: either a hand-rolled mx-side fix or a `{file}.invalid` marker for files that +don't conform to the XSD. Per-iteration template now lives in `plan.md`. -### Lessons / invariants captured +- **i1** — tenths-typed `width` attribute trailing-zero strip. Added `"width"` to + `decimalFields` in `src/private/mxtest/import/DecimalFields.h`. Cleared 17 failures. + Commit `639d46a3`. +- **i2** — `musuite/testInvalid.xml` is intentionally invalid; introduced the + `{file}.invalid` marker convention (honored by `CoreRoundtripImpl.cpp::discoverInputFiles`; + api import keeps processing). Documented in `data/README.md`. +- **i3 (static-analysis sweep)** — ran `xmllint --schema` against MusicXML XSD on + remaining failing files; marked 10 as `.invalid` where the schema violation explained + the diff. Three real bugs survived: ly22b, ly41e, ly45f. +- **i3 (cont.)** — ly45f: `CommaSeparatedListOfPositiveIntegers::parse` in hand-written + `src/private/mx/core/CommaSeparatedPositiveIntegers.cpp` discarded "1, 2, 3" vs + "1,2,3" spacing on import. Added a `", "` detection that sets `myIsSpacingDesired`. + Commit `461b96d2`. +- **i4** — ly41e: `XsString::toStream` escaped only `<`, `>`, `&`. A raw `\r` was + normalized to `\n` on the next read (pugixml `parse_eol`, XML 1.0 §2.11). Added + `'\r'` → `" "`. Hand-written type, no regen. Commit `040b2152`. +- **i5** — ly22b: XSD `slash` group is `minOccurs="0"` inside complexType `slash` and + `beat-repeat`, so empty `` is valid. HEAD treated `slash-type` as + always-present; revgen preserved this via `CHILD_MIN_OCCURS_OVERRIDE`. Removed the + overrides, regenerated `Slash.{h,cpp}` and `BeatRepeat.{h,cpp}` with a `myHasSlashType` + flag, removed a matching `addChildIfNone` workaround in + `src/private/mxtest/import/ExpectedFiles.cpp`. `make test-all` then surfaced 25 + assertions in 23 `mxtest/core/*Test.cpp` cases that codified the bug; added 6 + `setHasSlashType(true)` calls across 4 fixture files. Commits `d43a222c`, `9c8efa24`. -- `git checkout -- src/private/mx/core/` preserves mtimes; incremental cmake then links - partly-old `.o`s, giving stale test counts. Use `make clean && make test-all` for any - authoritative measurement. -- `make test-all` must run with generated files present (Issue A's `UpDownNone` - hand-backport in HEAD is incompatible with the schema-faithful regen, so a reset-first - build fails to compile). -- When removing a previously-emitted byte from a shared template, survey the whole HEAD - population that template emits, not just one or two representative files (R2's - DirectionType regression). -- Bespoke handlers should still read the parsed XSD model — pattern is "custom algorithm, - schema-driven data" (Direction's element-name tables derived from - `model.complex_types["direction-type"].content_tree.branches`). +Final state: `make test-core-dev` 350/350, `make test` 2717/2717, `make test-all` +3028/3028 (9914 assertions), `make check` passed. PR-merge commit `6c4e18d4`. diff --git a/docs/ai/projects/gen/plan.md b/docs/ai/projects/gen/plan.md index a18c1b36c..95d6954b0 100644 --- a/docs/ai/projects/gen/plan.md +++ b/docs/ai/projects/gen/plan.md @@ -2,36 +2,52 @@ ## Milestone 1: revgen — reverse-engineer codegen ✅ -Build a generator that produces every C++ class in `mx/core` from `docs/musicxml.xsd` with no -skipped elements. +Complete 2026-05-21. Generator produces every C++ class in `mx/core` from +`docs/musicxml.xsd`. `SKIP_ELEMENTS` and `CHOICE_SKIP` empty. See `log.md`. -**Complete** (2026-05-21, iteration 40). SKIP_ELEMENTS and CHOICE_SKIP both empty. +## Milestone 2: fix-gen — fix generator bugs surfaced by failing tests ✅ -## Milestone 2: fix-gen — fix generator bugs surfaced as failing tests ✅ +Complete 2026-05-22. `make test-all` 3028/3028. All fixes landed in the generator; +workarounds reverted. One item deferred to M5/M6: HEAD's hand-applied MusicXML 4.0 +`UpDownNone` backport vs. schema-faithful 3.x regen (TODOs in +`mx/impl/NotationsWriter.cpp:398`, `mx/impl/ArpeggiateFunctions.cpp:35`). -**Complete** (2026-05-22). All 2678 tests pass. +## Milestone 3: fix-core-dev — fix bugs surfaced by new core-dev test mode ✅ -The commit "src: issues caused by revgen" contained hand-edits to non-generated code (mx/impl, -mx/core/FromXElement, some tests) that worked around generator bugs. Each was triaged as BUG / -BENIGN / WEIRD, generator bugs were fixed, and the workarounds reverted. +Complete 2026-05-22 (5 iterations). 31 corert failures triaged into 10 `{file}.invalid` +markers and 4 real fixes. The `src/private/mxtest/corert/` harness is now part of +`make test-all`. See `log.md` M3 for per-iteration breakdown. -One WEIRD item deferred to M5: original `mx/core` had a hand-applied MusicXML 4.0 `UpDownNone` -backport that a schema-faithful 3.x regen overwrites. The `// TODO: fixme - MusicXML 4.0 ...` -comments in `mx/impl` (NotationsWriter.cpp:398, ArpeggiateFunctions.cpp:35) bookmark this. +### Reusable corert-iteration template (for M4) -## Milestone 3: increase test coverage +1. `rm -rf data/testOutput/*` +2. `make test-core-dev` — record `baseline_core_dev`. +3. `make test` — record `baseline_test` (should be zero). +4. Diff each before/after pair in `data/testOutput/corert`; pick the smallest. That's + `test_to_fix`. One test per session. +5. Analyze: pre-existing hand-rolled bug replicated by revgen? schema-invalid input? + library bug in a hand-written type? Flag pre-existing-bug calls to the user. +6. Fix. If gen changed, regen mx/core. `make fmt && make check`. +7. `rm -rf data/testOutput/*`; `make test-core-dev` must show the fixed test green and + no new failures; `make test` must stay at zero. +8. `make test-all` if the change touched `src/private/mx/core/*`. +9. Commit. Update `state.md` and other tracking. -Add a lot more MusicXML round-trip input files. Build a dedicated mx/core-level round-trip test -harness (not just api-level freezing tests). No specific design yet. +## Milestone 4: increase test coverage -## Milestone 4: better-gen — fix garbage +The corert harness is in place; what's missing is more MusicXML round-trip input +files. Sources to consider — public test suites, hand-curated edge cases targeting +spec features not yet exercised, generated/fuzzed inputs. No design yet. First M4 +session scopes this with the user. -The gen program is 12k lines of bad Python. Fix it. Use dedicated mx/core round-trip tests as the -north star for correctness. +## Milestone 5: better-gen — fix garbage -## Milestone 5: mxml4-types — generate MusicXML 4.0 types +The generator is ~14k lines of bad Python. Refactor. Use the corert suite as the +correctness oracle. -Replace `docs/musicxml.xsd` with MusicXML 4.0, regenerate, fix all existing tests. Watch for -backported or bolted-on features (SMuFL, UpDown, etc.) that were added with hacks to 3.0/3.1 but -are first-class in 4.0. Be backward-compatible with files `mx` may have written using those hacks. -Restore the `mx/impl` TODOs from revgen. +## Milestone 6: mxml4-types — generate MusicXML 4.0 types + +Replace `docs/musicxml.xsd` with MusicXML 4.0, regenerate, fix all existing tests. +Watch for backported / bolted-on features (SMuFL, `UpDown`, …) that were added with +hacks to 3.0/3.1 but are first-class in 4.0. Be backward-compatible with files mx +may have written using those hacks. Restore the `mx/impl` TODOs left from revgen. diff --git a/docs/ai/projects/gen/state.md b/docs/ai/projects/gen/state.md index e48c20755..0f3a2e0fa 100644 --- a/docs/ai/projects/gen/state.md +++ b/docs/ai/projects/gen/state.md @@ -2,26 +2,12 @@ ## Milestone -**M1 and M2 complete.** `make test-all`: 0 failed (2678 cases, 9914 assertions). +**M3 complete** (2026-05-22). -## What the next session should do +## What the next session should do (M4 kickoff) -**Milestone 3: increase test coverage.** Per `plan.md`: more MusicXML round-trip files and a -dedicated mx/core-level round-trip harness. +TODO: edit this -No design yet. Next session should: -1. Review existing test suite to understand coverage gaps. -2. Propose a coverage plan to the user. -3. Wait for user direction before implementing. +## Gotchas to carry forward -## Gotchas - -- **`make test-all` must run WITH generated files present.** HEAD's `ArpeggiateAttributes.h` has - a hand-applied `UpDownNone direction` (MusicXML 4.0 backport) but `NotationsWriter.cpp` assigns - `UpDown::down` — incompatible. The generator emits `UpDown` (from 3.x XSD), so the build only - succeeds with generated files present. Workflow: `python3 gen/generate.py && make fmt` THEN - `make test-all`. Reset AFTER it passes. -- **`git checkout -- src/private/mx/core/` preserves mtimes**, so incremental cmake builds skip - recompiling unchanged-mtime .cpp files. Stale-build measurements bit us mid-M2. Use - `make clean && make test-all` for an authoritative count. -- **Use `make test-all`, never `make test`.** Build target for tests is `make core`. +TODO: edit this diff --git a/gen/generate.py b/gen/generate.py index 369a35028..ee30a1b90 100644 --- a/gen/generate.py +++ b/gen/generate.py @@ -1076,13 +1076,7 @@ def generate_attrs_h(struct_name: str, attrs: list, model: XsdModel) -> str: # (parent_element_xml_name, child_element_xml_name). Use this when XSD group # inlining propagates minOccurs=0 from the enclosing group to an element that # HEAD treats as unconditionally present (no getHas/setHas accessors). -CHILD_MIN_OCCURS_OVERRIDE = { - # The `slash` group (used by both slash and beat-repeat) has minOccurs="0", - # which causes group inlining to set min_occurs=0 on slash-type. HEAD treats - # slash-type as always-present (minOccurs=1 within the group), so override. - ("slash", "slash-type"): 1, - ("beat-repeat", "slash-type"): 1, -} +CHILD_MIN_OCCURS_OVERRIDE = {} def _apply_child_min_occurs_override(elem_name: str, children: list) -> list: diff --git a/src/private/mx/core/CommaSeparatedPositiveIntegers.cpp b/src/private/mx/core/CommaSeparatedPositiveIntegers.cpp index d21c91a7c..18f8e7531 100644 --- a/src/private/mx/core/CommaSeparatedPositiveIntegers.cpp +++ b/src/private/mx/core/CommaSeparatedPositiveIntegers.cpp @@ -5,6 +5,8 @@ #include "mx/core/CommaSeparatedPositiveIntegers.h" #include "mx/core/StringUtils.h" +#include + namespace mx { namespace core @@ -64,6 +66,24 @@ void CommaSeparatedListOfPositiveIntegers::setValues(const std::set &values void CommaSeparatedListOfPositiveIntegers::parse(const StringType &commaSeparatedText) { + // Detect whether the input used the spaced form (e.g. "1, 2, 3") versus the compact + // form ("1,2,3"). Both are valid per the MusicXML ending-number pattern + // [1-9][0-9]*(, ?[1-9][0-9]*)*, so preserve the lexical choice on round-trip. + // Look for a digit-comma-space-digit sequence, which is the spaced form of the + // XSD pattern. This avoids being fooled by junk input (e.g. ", 0" inside garbage) + // and only flips the flag when the input actually conforms to the spaced form. + bool sawSpacedForm = false; + for (size_t i = 1; i + 2 < commaSeparatedText.size(); ++i) + { + if (commaSeparatedText[i] == ',' && commaSeparatedText[i + 1] == ' ' && + std::isdigit(static_cast(commaSeparatedText[i - 1])) && + std::isdigit(static_cast(commaSeparatedText[i + 2]))) + { + sawSpacedForm = true; + break; + } + } + myIsSpacingDesired = sawSpacedForm; StringType cleaned = onlyAllow(commaSeparatedText, "", "1234567890,-"); myValues.clear(); std::istringstream iss(cleaned); diff --git a/src/private/mx/core/XsString.cpp b/src/private/mx/core/XsString.cpp index f17b4c561..8335ad90e 100644 --- a/src/private/mx/core/XsString.cpp +++ b/src/private/mx/core/XsString.cpp @@ -57,6 +57,15 @@ std::ostream &toStream(std::ostream &os, const XsString &xsstring, bool useXmlEs os << "&"; break; + case '\r': + // Per XML 1.0 §2.11 (End-of-line handling), a literal 0x0D byte in + // element text content is normalized to 0x0A on the next parse. To + // preserve a carriage return through an XML round-trip, emit it as + // the numeric character reference — character references are + // not subject to EOL normalization. + os << " "; + break; + default: os << c; break; diff --git a/src/private/mx/core/elements/BeatRepeat.cpp b/src/private/mx/core/elements/BeatRepeat.cpp index aaf604cf5..37bb7ced0 100644 --- a/src/private/mx/core/elements/BeatRepeat.cpp +++ b/src/private/mx/core/elements/BeatRepeat.cpp @@ -13,7 +13,8 @@ namespace mx namespace core { BeatRepeat::BeatRepeat() - : myAttributes(std::make_shared()), mySlashType(makeSlashType()), mySlashDotSet() + : myAttributes(std::make_shared()), mySlashType(makeSlashType()), myHasSlashType(false), + mySlashDotSet() { } @@ -35,20 +36,30 @@ std::ostream &BeatRepeat::streamName(std::ostream &os) const bool BeatRepeat::hasContents() const { - return true; + return myHasSlashType || mySlashDotSet.size() > 0; } std::ostream &BeatRepeat::streamContents(std::ostream &os, const int indentLevel, bool &isOneLineOnly) const { - isOneLineOnly = false; - os << std::endl; - mySlashType->toStream(os, indentLevel + 1); + if (myHasSlashType) + { + os << std::endl; + mySlashType->toStream(os, indentLevel + 1); + } for (auto x : mySlashDotSet) { os << std::endl; x->toStream(os, indentLevel + 1); } - os << std::endl; + if (myHasSlashType || mySlashDotSet.size() > 0) + { + isOneLineOnly = false; + os << std::endl; + } + else + { + isOneLineOnly = true; + } return os; } @@ -78,6 +89,16 @@ void BeatRepeat::setSlashType(const SlashTypePtr &value) } } +bool BeatRepeat::getHasSlashType() const +{ + return myHasSlashType; +} + +void BeatRepeat::setHasSlashType(const bool value) +{ + myHasSlashType = value; +} + const SlashDotSet &BeatRepeat::getSlashDotSet() const { return mySlashDotSet; @@ -117,12 +138,11 @@ bool BeatRepeat::fromXElementImpl(std::ostream &message, ::ezxml::XElement &xele { bool isSuccess = true; isSuccess &= myAttributes->fromXElement(message, xelement); - bool isSlashTypeFound = false; auto endIter = xelement.end(); for (auto it = xelement.begin(); it != endIter; ++it) { - if (importElement(message, *it, isSuccess, *mySlashType, isSlashTypeFound)) + if (importElement(message, *it, isSuccess, *mySlashType, myHasSlashType)) { continue; } diff --git a/src/private/mx/core/elements/BeatRepeat.h b/src/private/mx/core/elements/BeatRepeat.h index 6e4da332b..4beda3001 100644 --- a/src/private/mx/core/elements/BeatRepeat.h +++ b/src/private/mx/core/elements/BeatRepeat.h @@ -40,9 +40,11 @@ class BeatRepeat : public ElementInterface BeatRepeatAttributesPtr getAttributes() const; void setAttributes(const BeatRepeatAttributesPtr &attributes); - /* _________ SlashType minOccurs = 1, maxOccurs = 1 _________ */ + /* _________ SlashType minOccurs = 0, maxOccurs = 1 _________ */ SlashTypePtr getSlashType() const; void setSlashType(const SlashTypePtr &value); + bool getHasSlashType() const; + void setHasSlashType(const bool value); /* _________ SlashDot minOccurs = 0, maxOccurs = unbounded _________ */ const SlashDotSet &getSlashDotSet() const; @@ -57,6 +59,7 @@ class BeatRepeat : public ElementInterface private: BeatRepeatAttributesPtr myAttributes; SlashTypePtr mySlashType; + bool myHasSlashType; SlashDotSet mySlashDotSet; }; } // namespace core diff --git a/src/private/mx/core/elements/Slash.cpp b/src/private/mx/core/elements/Slash.cpp index f1008e192..4d54e3411 100644 --- a/src/private/mx/core/elements/Slash.cpp +++ b/src/private/mx/core/elements/Slash.cpp @@ -12,7 +12,9 @@ namespace mx { namespace core { -Slash::Slash() : myAttributes(std::make_shared()), mySlashType(makeSlashType()), mySlashDotSet() +Slash::Slash() + : myAttributes(std::make_shared()), mySlashType(makeSlashType()), myHasSlashType(false), + mySlashDotSet() { } @@ -34,20 +36,30 @@ std::ostream &Slash::streamName(std::ostream &os) const bool Slash::hasContents() const { - return true; + return myHasSlashType || mySlashDotSet.size() > 0; } std::ostream &Slash::streamContents(std::ostream &os, const int indentLevel, bool &isOneLineOnly) const { - isOneLineOnly = false; - os << std::endl; - mySlashType->toStream(os, indentLevel + 1); + if (myHasSlashType) + { + os << std::endl; + mySlashType->toStream(os, indentLevel + 1); + } for (auto x : mySlashDotSet) { os << std::endl; x->toStream(os, indentLevel + 1); } - os << std::endl; + if (myHasSlashType || mySlashDotSet.size() > 0) + { + isOneLineOnly = false; + os << std::endl; + } + else + { + isOneLineOnly = true; + } return os; } @@ -77,6 +89,16 @@ void Slash::setSlashType(const SlashTypePtr &value) } } +bool Slash::getHasSlashType() const +{ + return myHasSlashType; +} + +void Slash::setHasSlashType(const bool value) +{ + myHasSlashType = value; +} + const SlashDotSet &Slash::getSlashDotSet() const { return mySlashDotSet; @@ -116,12 +138,11 @@ bool Slash::fromXElementImpl(std::ostream &message, ::ezxml::XElement &xelement) { bool isSuccess = true; isSuccess &= myAttributes->fromXElement(message, xelement); - bool isSlashTypeFound = false; auto endIter = xelement.end(); for (auto it = xelement.begin(); it != endIter; ++it) { - if (importElement(message, *it, isSuccess, *mySlashType, isSlashTypeFound)) + if (importElement(message, *it, isSuccess, *mySlashType, myHasSlashType)) { continue; } diff --git a/src/private/mx/core/elements/Slash.h b/src/private/mx/core/elements/Slash.h index 5aba6a154..0a5d23b15 100644 --- a/src/private/mx/core/elements/Slash.h +++ b/src/private/mx/core/elements/Slash.h @@ -40,9 +40,11 @@ class Slash : public ElementInterface SlashAttributesPtr getAttributes() const; void setAttributes(const SlashAttributesPtr &attributes); - /* _________ SlashType minOccurs = 1, maxOccurs = 1 _________ */ + /* _________ SlashType minOccurs = 0, maxOccurs = 1 _________ */ SlashTypePtr getSlashType() const; void setSlashType(const SlashTypePtr &value); + bool getHasSlashType() const; + void setHasSlashType(const bool value); /* _________ SlashDot minOccurs = 0, maxOccurs = unbounded _________ */ const SlashDotSet &getSlashDotSet() const; @@ -57,6 +59,7 @@ class Slash : public ElementInterface private: SlashAttributesPtr myAttributes; SlashTypePtr mySlashType; + bool myHasSlashType; SlashDotSet mySlashDotSet; }; } // namespace core diff --git a/src/private/mxtest/core/BeatRepeatTest.cpp b/src/private/mxtest/core/BeatRepeatTest.cpp index f65dc31f7..dbd1aabad 100644 --- a/src/private/mxtest/core/BeatRepeatTest.cpp +++ b/src/private/mxtest/core/BeatRepeatTest.cpp @@ -16,6 +16,7 @@ using namespace mxtest; TEST(Test01, BeatRepeat) { BeatRepeat object; + object.setHasSlashType(true); stringstream expected; streamLine(expected, 1, R"()"); streamLine(expected, 2, R"(eighth)"); @@ -35,6 +36,7 @@ TEST(Test02, BeatRepeat) object.getAttributes()->hasUseDots = true; object.getAttributes()->useDots = YesNo::yes; object.getAttributes()->hasSlashes = true; + object.setHasSlashType(true); object.addSlashDot(makeSlashDot()); object.addSlashDot(makeSlashDot()); stringstream expected; diff --git a/src/private/mxtest/core/MeasureStyleTest.cpp b/src/private/mxtest/core/MeasureStyleTest.cpp index 2720e0ba8..7b3bd4eb6 100644 --- a/src/private/mxtest/core/MeasureStyleTest.cpp +++ b/src/private/mxtest/core/MeasureStyleTest.cpp @@ -32,6 +32,7 @@ TEST(Test02, MeasureStyle) { MeasureStyle object; object.getMeasureStyleChoice()->setChoice(MeasureStyleChoice::Choice::beatRepeat); + object.getMeasureStyleChoice()->getBeatRepeat()->setHasSlashType(true); object.getMeasureStyleChoice()->getBeatRepeat()->addSlashDot(makeSlashDot()); object.getAttributes()->hasColor = true; object.getAttributes()->color = Color{76, 58, 201, 43}; @@ -54,6 +55,7 @@ TEST(Test03, MeasureStyle) { MeasureStyle object; object.getMeasureStyleChoice()->setChoice(MeasureStyleChoice::Choice::slash); + object.getMeasureStyleChoice()->getSlash()->setHasSlashType(true); stringstream expected; streamLine(expected, 1, R"()"); streamLine(expected, 2, R"()"); diff --git a/src/private/mxtest/core/PropertiesTest.cpp b/src/private/mxtest/core/PropertiesTest.cpp index 4cbbbe9a5..84f167054 100644 --- a/src/private/mxtest/core/PropertiesTest.cpp +++ b/src/private/mxtest/core/PropertiesTest.cpp @@ -111,6 +111,7 @@ PropertiesPtr tgenProperties(TestMode v) o->addDirective(dir); auto ms = makeMeasureStyle(); ms->getMeasureStyleChoice()->setChoice(MeasureStyleChoice::Choice::slash); + ms->getMeasureStyleChoice()->getSlash()->setHasSlashType(true); ms->getMeasureStyleChoice()->getSlash()->getSlashType()->setValue(NoteTypeValue::eighth); o->addMeasureStyle(ms); } @@ -166,6 +167,7 @@ PropertiesPtr tgenProperties(TestMode v) o->addDirective(dir); auto ms = makeMeasureStyle(); ms->getMeasureStyleChoice()->setChoice(MeasureStyleChoice::Choice::slash); + ms->getMeasureStyleChoice()->getSlash()->setHasSlashType(true); ms->getMeasureStyleChoice()->getSlash()->getSlashType()->setValue(NoteTypeValue::eighth); o->addMeasureStyle(ms); } diff --git a/src/private/mxtest/core/SlashTest.cpp b/src/private/mxtest/core/SlashTest.cpp index 2f28f5fcf..d8594a5d8 100644 --- a/src/private/mxtest/core/SlashTest.cpp +++ b/src/private/mxtest/core/SlashTest.cpp @@ -16,6 +16,7 @@ using namespace mxtest; TEST(Test01, Slash) { Slash object; + object.setHasSlashType(true); stringstream expected; streamLine(expected, 1, R"()"); streamLine(expected, 2, R"(eighth)"); @@ -36,6 +37,7 @@ TEST(Test02, Slash) object.getAttributes()->useDots = YesNo::yes; object.getAttributes()->hasUseStems = true; object.getAttributes()->useStems = YesNo::yes; + object.setHasSlashType(true); object.addSlashDot(makeSlashDot()); object.addSlashDot(makeSlashDot()); stringstream expected; diff --git a/src/private/mxtest/corert/CoreRoundtripImpl.cpp b/src/private/mxtest/corert/CoreRoundtripImpl.cpp index ec2e5bcd2..f15447562 100644 --- a/src/private/mxtest/corert/CoreRoundtripImpl.cpp +++ b/src/private/mxtest/corert/CoreRoundtripImpl.cpp @@ -60,6 +60,17 @@ bool hasXmlExtension(const std::filesystem::path &p) return ext == ".xml" || ext == ".musicxml"; } +// A file is marked invalid (and therefore unfit for schema-strict round-trip) +// when a sibling file with the same name plus a trailing `.invalid` extension +// exists. See data/README.md for the convention. +bool hasInvalidMarker(const std::filesystem::path &p) +{ + std::filesystem::path marker = p; + marker += ".invalid"; + std::error_code ec; + return std::filesystem::exists(marker, ec); +} + // Set the root `version` attribute to mx::core's supported MusicXML version. // Equivalent in shape to mxtest/import's setRootMusicXmlVersion but without // the doctype cross-check (which we apply via setDoctype below based on the @@ -278,6 +289,10 @@ std::vector discoverInputFiles() { continue; } + if (hasInvalidMarker(p)) + { + continue; + } result.push_back(p.string()); } std::sort(result.begin(), result.end()); diff --git a/src/private/mxtest/import/DecimalFields.h b/src/private/mxtest/import/DecimalFields.h index 57af56023..2e9cb3f9b 100644 --- a/src/private/mxtest/import/DecimalFields.h +++ b/src/private/mxtest/import/DecimalFields.h @@ -21,9 +21,9 @@ namespace mxtest { -const std::set decimalFields = {"top-system-distance", "dynamics", "left-margin", - "right-margin", "staff-distance", "system-distance", - "default-y", "default-x", "tenths"}; +const std::set decimalFields = { + "top-system-distance", "dynamics", "left-margin", "right-margin", "staff-distance", + "system-distance", "default-y", "default-x", "tenths", "width"}; inline int trailingCharsToStrip(const std::string &value) { diff --git a/src/private/mxtest/import/ExpectedFiles.cpp b/src/private/mxtest/import/ExpectedFiles.cpp index 0935f8c35..58f8adab3 100644 --- a/src/private/mxtest/import/ExpectedFiles.cpp +++ b/src/private/mxtest/import/ExpectedFiles.cpp @@ -35,10 +35,6 @@ void generateExpectedFile(const std::string &subdir, const std::string &fileName // ly01e_Pitches_ParenthesizedAccidentals incorrectly uses "double-flat" which should be "flat-flat" instead convertValues(*xdoc, "accidental", "double-flat", "flat-flat"); - // lysuite_ly22b_Staff_Notestyles.xml incorrectly has elements that are missing required attribute - // 'use-stems' and child element 'slash-type' - addChildIfNone(*xdoc, "slash", "slash-type", "eighth"); - // ly32b_Atriculations_Texts.xml cas a 'color' attribute on the 'words' element // I have examined the xsd thoughouly and I believe this is illegal removeAttribute(*xdoc, "words", "color");