From aae54a7f4c8a06fbaae2cdd826e6b23a6fbbc904 Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Mon, 9 Nov 2026 08:59:34 +0100 Subject: [PATCH 1/3] build: run deslag in fmt-check Onboard deslag, the prose linter, for AGENTS.md, docs/ and the mx::api headers and sources. make fmt-check now runs deslag check after clang-format. Generated code is excluded. Fix the prose deslag reports in those files, and delete the old design docs that recorded finished work, along with the references to them (including the plan citations in gen/cpp templates, regenerated). --- .github/release/Package.swift.template | 2 +- .github/workflows/_release-target-swift.yml | 10 +- .github/workflows/ci.yaml | 3 +- .github/workflows/release-build.yml | 11 +- .github/workflows/release-publish.yml | 14 +- AGENTS.md | 114 +- CMakeLists.txt | 4 +- Makefile | 9 +- Package.swift | 2 +- deslag.toml | 122 ++ docs/ai/design/api-roundtrip-classifier.md | 174 +-- docs/ai/design/mx-core-gpt-5.0.md | 1114 --------------- docs/ai/design/mx-core-opus-4.7.md | 1208 ----------------- docs/ai/design/mx-core-plan.md | 393 ------ docs/ai/design/mx-impl-port-brief.md | 99 -- docs/ai/design/mx-impl-port-plan.md | 475 ------- docs/ai/design/newgen-integration-plan.md | 278 ---- docs/ai/design/release-process.md | 438 ------ docs/ai/design/xml-backend.md | 80 +- gen/cpp/config.toml | 16 +- gen/cpp/templates/attributes_parse.tmpl | 2 +- gen/cpp/templates/choice.h.tmpl | 2 +- gen/cpp/templates/choice_boxed.h.tmpl | 2 +- gen/cpp/templates/color.h.tmpl | 2 +- gen/cpp/templates/comma_separated_text.h.tmpl | 2 +- gen/cpp/templates/composite.h.tmpl | 2 +- gen/cpp/templates/derived.h.tmpl | 2 +- gen/cpp/templates/document.h.tmpl | 4 +- gen/cpp/templates/empty.h.tmpl | 2 +- gen/cpp/templates/ending_number.h.tmpl | 2 +- gen/cpp/templates/fields_api.tmpl | 2 +- gen/cpp/templates/group.h.tmpl | 2 +- gen/cpp/templates/number.h.tmpl | 2 +- gen/cpp/templates/smufl_prefixed.h.tmpl | 2 +- gen/cpp/templates/smufl_wavy.h.tmpl | 2 +- gen/cpp/templates/string.cpp.tmpl | 2 +- gen/cpp/templates/time_only.h.tmpl | 2 +- gen/cpp/templates/yyyy_mm_dd.h.tmpl | 2 +- gen/plates/build.py | 8 +- gen/plates/check.py | 2 +- gen/plates/model.py | 12 +- gen/press/render.py | 2 +- scripts/doctor.sh | 13 +- src/include/mx/api/ApiCommon.h | 11 +- src/include/mx/api/BarlineData.h | 16 +- src/include/mx/api/ChordData.h | 10 +- src/include/mx/api/CodaData.h | 4 +- src/include/mx/api/ComplexTimeSignature.h | 8 +- src/include/mx/api/DampData.h | 2 +- src/include/mx/api/Diagnostics.h | 2 +- src/include/mx/api/DirectionData.h | 18 +- src/include/mx/api/DynamicsData.h | 8 +- src/include/mx/api/EncodingData.h | 2 +- src/include/mx/api/EyeglassesData.h | 10 +- src/include/mx/api/FiguredBassData.h | 4 +- src/include/mx/api/GlissandoData.h | 4 +- src/include/mx/api/HarpPedalsData.h | 2 +- src/include/mx/api/Id.h | 2 +- src/include/mx/api/ImageData.h | 12 +- src/include/mx/api/KeyData.h | 4 +- src/include/mx/api/LyricData.h | 8 +- src/include/mx/api/MarkData.h | 18 +- src/include/mx/api/MarkDataChoice.h | 8 +- src/include/mx/api/NoteData.h | 14 +- src/include/mx/api/NoteRelationData.h | 2 +- src/include/mx/api/OtherDirectionData.h | 2 +- src/include/mx/api/OttavaData.h | 4 +- src/include/mx/api/PageTextData.h | 4 +- src/include/mx/api/PartData.h | 20 +- src/include/mx/api/PedalLineData.h | 8 +- src/include/mx/api/PercussionData.h | 16 +- src/include/mx/api/RehearsalData.h | 4 +- src/include/mx/api/Result.h | 4 +- src/include/mx/api/ScoreData.h | 12 +- src/include/mx/api/SegnoData.h | 4 +- src/include/mx/api/StaffDivideData.h | 8 +- src/include/mx/api/StringMuteData.h | 2 +- src/include/mx/api/SymbolData.h | 4 +- src/include/mx/api/TimeChoice.h | 2 +- src/include/mx/api/TimeSignatureData.h | 7 +- src/include/mx/api/TransposeData.h | 2 - src/include/mx/api/TupletData.h | 4 +- src/include/mx/api/WavyLineData.h | 2 +- src/include/mx/api/WordsData.h | 4 +- src/private/mx/core/Attribution.h | 2 +- src/private/mx/core/Decimal.h | 4 +- src/private/mx/core/Error.h | 4 +- src/private/mx/core/Lexical.h | 2 +- src/private/mx/core/OneOrMore.h | 4 +- src/private/mx/core/Result.h | 4 +- src/private/mx/core/Token.h | 2 +- src/private/mx/core/Version.h | 2 +- src/private/mx/core/Xml.cpp | 2 +- src/private/mx/core/Xml.h | 2 +- src/private/mx/core/generated/Accord.h | 2 +- .../mx/core/generated/AccordionMiddle.h | 2 +- .../mx/core/generated/AccordionRegistration.h | 2 +- .../mx/core/generated/AllMarginsGroup.h | 2 +- src/private/mx/core/generated/Appearance.h | 2 +- src/private/mx/core/generated/Arrow.h | 2 +- src/private/mx/core/generated/ArrowChoice.h | 2 +- .../mx/core/generated/ArrowChoiceGroup.h | 2 +- src/private/mx/core/generated/Articulations.h | 2 +- .../mx/core/generated/ArticulationsChoice.h | 2 +- src/private/mx/core/generated/Attributes.h | 2 +- .../mx/core/generated/AttributesChoice.h | 2 +- src/private/mx/core/generated/Backup.h | 2 +- src/private/mx/core/generated/Barline.h | 4 +- src/private/mx/core/generated/Bass.h | 2 +- src/private/mx/core/generated/BeamLevel.h | 2 +- src/private/mx/core/generated/BeatRepeat.h | 2 +- src/private/mx/core/generated/BeatUnitGroup.h | 2 +- src/private/mx/core/generated/BeatUnitTied.h | 2 +- src/private/mx/core/generated/Bend.h | 2 +- src/private/mx/core/generated/BendChoice.h | 2 +- src/private/mx/core/generated/Clef.h | 2 +- src/private/mx/core/generated/ClefGroup.h | 2 +- src/private/mx/core/generated/Color.h | 2 +- .../mx/core/generated/CommaSeparatedText.h | 2 +- src/private/mx/core/generated/Credit.h | 2 +- src/private/mx/core/generated/CreditChoice.h | 2 +- .../mx/core/generated/CreditChoiceGroup.h | 2 +- .../core/generated/CreditChoiceGroupChoice.h | 2 +- .../core/generated/CreditChoiceGroupGroup.h | 2 +- .../generated/CreditChoiceGroupGroupChoice.h | 2 +- src/private/mx/core/generated/CueNoteGroup.h | 2 +- src/private/mx/core/generated/Defaults.h | 2 +- src/private/mx/core/generated/Degree.h | 2 +- src/private/mx/core/generated/Direction.h | 2 +- src/private/mx/core/generated/DirectionType.h | 2 +- .../mx/core/generated/DirectionTypeChoice.h | 2 +- .../generated/DirectionTypeChoiceChoice.h | 2 +- .../core/generated/DisplayStepOctaveGroup.h | 2 +- src/private/mx/core/generated/Divisions.h | 2 +- src/private/mx/core/generated/Document.h | 4 +- src/private/mx/core/generated/Dynamics.h | 2 +- .../mx/core/generated/DynamicsChoice.h | 2 +- .../mx/core/generated/EditorialGroup.h | 2 +- .../generated/EditorialVoiceDirectionGroup.h | 2 +- .../mx/core/generated/EditorialVoiceGroup.h | 2 +- .../mx/core/generated/ElisionSyllabicGroup.h | 2 +- src/private/mx/core/generated/Empty.h | 2 +- src/private/mx/core/generated/Encoding.h | 2 +- .../mx/core/generated/EncodingChoice.h | 2 +- src/private/mx/core/generated/EndingNumber.h | 2 +- src/private/mx/core/generated/Fifths.h | 2 +- src/private/mx/core/generated/Figure.h | 2 +- src/private/mx/core/generated/FiguredBass.h | 2 +- src/private/mx/core/generated/FontFamily.h | 2 +- src/private/mx/core/generated/ForPart.h | 2 +- src/private/mx/core/generated/Forward.h | 2 +- src/private/mx/core/generated/Frame.h | 2 +- src/private/mx/core/generated/FrameNote.h | 2 +- src/private/mx/core/generated/FullNoteGroup.h | 2 +- .../mx/core/generated/FullNoteGroupChoice.h | 2 +- .../mx/core/generated/GraceCueNoteGroup.h | 2 +- .../mx/core/generated/GraceNormalNoteGroup.h | 4 +- .../mx/core/generated/GraceNoteChoice.h | 2 +- .../mx/core/generated/GraceNoteGroup.h | 2 +- src/private/mx/core/generated/Grouping.h | 2 +- src/private/mx/core/generated/HarmonMute.h | 2 +- src/private/mx/core/generated/Harmonic.h | 2 +- .../mx/core/generated/HarmonicChoice.h | 2 +- .../mx/core/generated/HarmonicChoice2.h | 2 +- src/private/mx/core/generated/Harmony.h | 2 +- .../mx/core/generated/HarmonyChordGroup.h | 2 +- .../core/generated/HarmonyChordGroupChoice.h | 2 +- src/private/mx/core/generated/HarpPedals.h | 2 +- src/private/mx/core/generated/HeelToe.h | 2 +- src/private/mx/core/generated/Hole.h | 2 +- .../mx/core/generated/Identification.h | 2 +- .../mx/core/generated/InstrumentChange.h | 2 +- .../mx/core/generated/Interchangeable.h | 2 +- src/private/mx/core/generated/Key.h | 2 +- src/private/mx/core/generated/KeyChoice.h | 2 +- src/private/mx/core/generated/LayoutGroup.h | 2 +- .../mx/core/generated/LeftRightMarginsGroup.h | 2 +- src/private/mx/core/generated/Listen.h | 2 +- src/private/mx/core/generated/ListenChoice.h | 2 +- src/private/mx/core/generated/Listening.h | 2 +- .../mx/core/generated/ListeningChoice.h | 2 +- src/private/mx/core/generated/Lyric.h | 2 +- src/private/mx/core/generated/LyricChoice.h | 2 +- .../mx/core/generated/LyricLanguage.cpp | 2 +- .../mx/core/generated/LyricSyllableGroup.h | 2 +- .../mx/core/generated/LyricTextGroup.h | 2 +- src/private/mx/core/generated/MIDI128.h | 2 +- src/private/mx/core/generated/MIDI16.h | 2 +- src/private/mx/core/generated/MIDI16384.h | 2 +- .../mx/core/generated/MIDIInstrument.h | 2 +- src/private/mx/core/generated/MeasureLayout.h | 2 +- src/private/mx/core/generated/MeasureStyle.h | 2 +- .../mx/core/generated/MeasureStyleChoice.h | 2 +- src/private/mx/core/generated/MeasureText.cpp | 2 +- src/private/mx/core/generated/Metronome.h | 2 +- .../mx/core/generated/MetronomeChoice.h | 2 +- .../mx/core/generated/MetronomeChoiceGroup.h | 2 +- .../mx/core/generated/MetronomeChoiceGroup2.h | 2 +- .../generated/MetronomeChoiceGroup2Group.h | 2 +- .../generated/MetronomeChoiceGroupChoice.h | 2 +- .../MetronomeChoiceGroupChoiceGroup.h | 2 +- src/private/mx/core/generated/MetronomeNote.h | 2 +- .../mx/core/generated/MetronomeTuplet.h | 2 +- src/private/mx/core/generated/Millimeters.h | 2 +- src/private/mx/core/generated/Milliseconds.h | 2 +- src/private/mx/core/generated/Miscellaneous.h | 2 +- src/private/mx/core/generated/Mordent.h | 2 +- .../mx/core/generated/MusicDataChoice.h | 2 +- src/private/mx/core/generated/NameDisplay.h | 2 +- .../mx/core/generated/NameDisplayChoice.h | 2 +- .../mx/core/generated/NonNegativeDecimal.h | 2 +- .../mx/core/generated/NonNegativeInteger.h | 2 +- .../core/generated/NonTraditionalKeyGroup.h | 2 +- .../mx/core/generated/NormalNoteGroup.h | 4 +- src/private/mx/core/generated/Notations.h | 2 +- .../mx/core/generated/NotationsChoice.h | 2 +- src/private/mx/core/generated/Note.h | 4 +- src/private/mx/core/generated/NoteChoice.h | 2 +- src/private/mx/core/generated/NoteheadText.h | 2 +- .../mx/core/generated/NoteheadTextChoice.h | 2 +- src/private/mx/core/generated/NumberLevel.h | 2 +- src/private/mx/core/generated/NumberOfLines.h | 2 +- src/private/mx/core/generated/Numeral.h | 2 +- src/private/mx/core/generated/NumeralKey.h | 2 +- src/private/mx/core/generated/NumeralValue.h | 2 +- src/private/mx/core/generated/Octave.h | 2 +- src/private/mx/core/generated/Ornaments.h | 2 +- .../mx/core/generated/OrnamentsGroup.h | 2 +- .../mx/core/generated/OrnamentsGroupChoice.h | 2 +- src/private/mx/core/generated/PageLayout.h | 4 +- .../mx/core/generated/PageLayoutGroup.h | 2 +- src/private/mx/core/generated/PageMargins.h | 2 +- src/private/mx/core/generated/PartClef.h | 2 +- src/private/mx/core/generated/PartGroup.h | 2 +- src/private/mx/core/generated/PartLink.h | 2 +- src/private/mx/core/generated/PartList.h | 2 +- .../mx/core/generated/PartListChoice.h | 2 +- src/private/mx/core/generated/PartTranspose.h | 2 +- .../mx/core/generated/PartwiseMeasure.h | 2 +- src/private/mx/core/generated/PartwisePart.h | 2 +- src/private/mx/core/generated/PedalTuning.h | 2 +- src/private/mx/core/generated/Percent.h | 2 +- src/private/mx/core/generated/Percussion.h | 2 +- .../mx/core/generated/PercussionChoice.h | 2 +- src/private/mx/core/generated/Pitch.h | 2 +- src/private/mx/core/generated/Play.h | 2 +- src/private/mx/core/generated/PlayChoice.h | 2 +- src/private/mx/core/generated/Player.h | 2 +- .../mx/core/generated/PositiveDivisions.h | 2 +- .../mx/core/generated/PositiveInteger.h | 2 +- src/private/mx/core/generated/Print.h | 2 +- src/private/mx/core/generated/Release.h | 2 +- src/private/mx/core/generated/Rest.h | 2 +- src/private/mx/core/generated/Root.h | 2 +- .../mx/core/generated/RotationDegrees.h | 2 +- src/private/mx/core/generated/Scaling.h | 2 +- src/private/mx/core/generated/Scordatura.h | 2 +- .../mx/core/generated/ScoreHeaderGroup.h | 2 +- .../mx/core/generated/ScoreInstrument.h | 2 +- src/private/mx/core/generated/ScorePart.h | 2 +- .../mx/core/generated/ScorePartMIDIGroup.h | 2 +- src/private/mx/core/generated/ScorePartwise.h | 2 +- src/private/mx/core/generated/ScoreTimewise.h | 2 +- src/private/mx/core/generated/Semitones.h | 2 +- src/private/mx/core/generated/Slash.h | 2 +- src/private/mx/core/generated/SlashGroup.h | 2 +- .../mx/core/generated/SlashGroupGroup.h | 2 +- .../core/generated/SmuflAccidentalGlyphName.h | 2 +- .../mx/core/generated/SmuflCodaGlyphName.h | 2 +- .../mx/core/generated/SmuflLyricsGlyphName.h | 2 +- .../core/generated/SmuflPictogramGlyphName.h | 2 +- .../mx/core/generated/SmuflSegnoGlyphName.h | 2 +- .../core/generated/SmuflWavyLineGlyphName.h | 2 +- src/private/mx/core/generated/Sound.h | 2 +- src/private/mx/core/generated/SoundGroup.h | 2 +- src/private/mx/core/generated/StaffDetails.h | 2 +- .../mx/core/generated/StaffDetailsGroup.h | 2 +- src/private/mx/core/generated/StaffLayout.h | 2 +- src/private/mx/core/generated/StaffLine.h | 2 +- .../mx/core/generated/StaffLinePosition.h | 2 +- src/private/mx/core/generated/StaffNumber.h | 2 +- src/private/mx/core/generated/StaffTuning.h | 2 +- src/private/mx/core/generated/Stick.h | 2 +- src/private/mx/core/generated/StringNumber.h | 2 +- src/private/mx/core/generated/StrongAccent.h | 2 +- src/private/mx/core/generated/Swing.h | 2 +- src/private/mx/core/generated/SwingChoice.h | 2 +- .../mx/core/generated/SwingChoiceGroup.h | 2 +- .../mx/core/generated/SystemDividers.h | 2 +- src/private/mx/core/generated/SystemLayout.h | 2 +- src/private/mx/core/generated/SystemMargins.h | 2 +- src/private/mx/core/generated/Technical.h | 2 +- .../mx/core/generated/TechnicalChoice.h | 2 +- src/private/mx/core/generated/Tenths.h | 2 +- src/private/mx/core/generated/Time.h | 2 +- src/private/mx/core/generated/TimeChoice.h | 2 +- .../mx/core/generated/TimeChoiceGroup.h | 2 +- .../mx/core/generated/TimeModification.h | 2 +- .../mx/core/generated/TimeModificationGroup.h | 2 +- src/private/mx/core/generated/TimeOnly.h | 2 +- .../mx/core/generated/TimeSignatureGroup.h | 2 +- .../mx/core/generated/TimewiseMeasure.h | 2 +- src/private/mx/core/generated/TimewisePart.h | 2 +- .../mx/core/generated/TraditionalKeyGroup.h | 2 +- src/private/mx/core/generated/Transpose.h | 2 +- .../mx/core/generated/TransposeGroup.h | 2 +- src/private/mx/core/generated/TremoloMarks.h | 2 +- src/private/mx/core/generated/TrillBeats.h | 2 +- src/private/mx/core/generated/TuningGroup.h | 2 +- src/private/mx/core/generated/Tuplet.h | 2 +- src/private/mx/core/generated/TupletPortion.h | 2 +- src/private/mx/core/generated/Unpitched.h | 2 +- .../mx/core/generated/VirtualInstrument.h | 2 +- .../generated/VirtualInstrumentDataGroup.h | 2 +- .../VirtualInstrumentDataGroupChoice.h | 2 +- src/private/mx/core/generated/Work.h | 2 +- src/private/mx/core/generated/YyyyMmDd.h | 2 +- .../mxtest/api/ChordDataSaveAndLoadTest.cpp | 2 +- .../mxtest/corert/CoreRoundtripTest.cpp | 2 +- 319 files changed, 775 insertions(+), 4638 deletions(-) create mode 100644 deslag.toml delete mode 100644 docs/ai/design/mx-core-gpt-5.0.md delete mode 100644 docs/ai/design/mx-core-opus-4.7.md delete mode 100644 docs/ai/design/mx-core-plan.md delete mode 100644 docs/ai/design/mx-impl-port-brief.md delete mode 100644 docs/ai/design/mx-impl-port-plan.md delete mode 100644 docs/ai/design/newgen-integration-plan.md delete mode 100644 docs/ai/design/release-process.md diff --git a/.github/release/Package.swift.template b/.github/release/Package.swift.template index 015d3d33d..ee2423a45 100644 --- a/.github/release/Package.swift.template +++ b/.github/release/Package.swift.template @@ -1,7 +1,7 @@ // swift-tools-version: 5.9 // TEMPLATE for the machine-generated release manifest -- the Package.swift that -// exists only at a tagged release commit ("B" in docs/ai/design/release-process.md). +// exists only at a tagged release commit, called B. // The _release-target-swift workflow interpolates the three MX_RELEASE // placeholders (version, url, checksum) and the release-build prep job commits // the result as B; commit R then restores the source manifest, so every tip of diff --git a/.github/workflows/_release-target-swift.yml b/.github/workflows/_release-target-swift.yml index 9bd8deb34..de8a08c02 100644 --- a/.github/workflows/_release-target-swift.yml +++ b/.github/workflows/_release-target-swift.yml @@ -1,4 +1,4 @@ -# The Swift release target (docs/ai/design/release-process.md §8-9): builds the +# The Swift release target builds the # Mx XCFramework from commit A and emits the two artifacts the release system # consumes -- the publishable archive and the metadata the prep job commits as B. # Called only from release-build.yml (workflow_call); never triggered directly. @@ -122,7 +122,7 @@ jobs: set -euxo pipefail # For a zip, swift package compute-checksum IS the file's SHA-256; # assert that equivalence here so release-publish can re-verify with - # plain sha256sum (design §5, check 6). + # plain sha256sum. CHECKSUM=$(swift package compute-checksum "out/dist/$ARCHIVE") SHA256=$(shasum -a 256 "out/dist/$ARCHIVE" | cut -d' ' -f1) if [ "$CHECKSUM" != "$SHA256" ]; then @@ -158,7 +158,7 @@ jobs: cp out/metadata/tree/Package.swift "$RUNNER_TEMP/manifest-check/" (cd "$RUNNER_TEMP/manifest-check" && swift package dump-package > /dev/null) - # The target's contract fragment (design §9): identity, artifact, + # The target's contract fragment: identity, artifact, # checksum, platforms, and the paths this target may occupy in B. A # copy ships in both artifacts. jq -n \ @@ -179,7 +179,7 @@ jobs: > out/metadata/fragment.json cp out/metadata/fragment.json out/dist/fragment.json - # The publishable artifact (design §6): archive + .sha256 + fragment. + # The publishable artifact: archive + .sha256 + fragment. # retention-days is the documented 90-day contract: the tag must be pushed # while this artifact still exists. - name: Upload release artifact @@ -190,7 +190,7 @@ jobs: retention-days: 90 if-no-files-found: error - # The prep-job channel (design §9, outputs b and c): the exact tree files + # The prep-job channel: the exact tree files # for commit B plus the contract fragment. - name: Upload metadata artifact uses: actions/upload-artifact@v4 diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 66d46266a..98bde14f1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -295,8 +295,7 @@ jobs: # Proves mx compiles as a Swift package under AppleClang/SPM on every PR -- # the path komp takes when it depends on a sibling mx via `.package(path:)`. - # Runs no test suites; the binary xcframework ships from release-build.yml - # (docs/ai/design/release-process.md), not from CI. + # Runs no test suites; the binary xcframework ships from release-build.yml, not from CI. # # SwiftPM has no compiler-launcher hook, so incremental builds come from caching # its .build tree: llbuild tracks inputs by content signature, so a restored diff --git a/.github/workflows/release-build.yml b/.github/workflows/release-build.yml index 0c8937bea..179f10d05 100644 --- a/.github/workflows/release-build.yml +++ b/.github/workflows/release-build.yml @@ -1,4 +1,4 @@ -# Phase 1 of the release system (docs/ai/design/release-process.md): build a +# Phase 1 of the release system: build a # chosen commit A for every release target, then open the release PR whose first # commit, B, is the tag target. Nothing here publishes; phase 2 is # release-publish.yml, triggered by the tag push after the PR merges. @@ -50,7 +50,7 @@ jobs: [[ "$VERSION" =~ $SEMVER ]] \ || { echo "::error::version must be MAJOR.MINOR.PATCH, optionally followed by a pre-release suffix such as -rc1, with no leading v, got '$VERSION'"; exit 1; } - # The target fan-out (design §9): one reusable workflow per release target. + # The target fan-out: one reusable workflow per release target. # Adding a target = add a job like this one, and extend the hardcoded path # ceiling in release-publish.yml with its declared metadata paths. swift: @@ -95,7 +95,7 @@ jobs: shopt -s nullglob if ! git merge-base --is-ancestor "$SHA" origin/main; then - echo "::warning::$SHA is not an ancestor of main yet; release-publish will refuse the tag until it is (design §5, check 3)." + echo "::warning::$SHA is not an ancestor of main yet; release-publish will refuse the tag until it is." fi FRAGMENTS=("$RUNNER_TEMP"/release-metadata/*/fragment.json) @@ -115,7 +115,7 @@ jobs: if [ -d "$d/tree" ]; then cp -R "$d/tree/." .; fi done - # release-manifest.json (design §5): version, base SHA A, this run's + # release-manifest.json: version, base SHA A, this run's # ID (the run pins itself into B -- dispatch inputs are not queryable # later), every target's fragment, and the union of paths B may touch. jq -s \ @@ -170,8 +170,7 @@ jobs: set -euo pipefail cat > "$RUNNER_TEMP/body.md" <<'BODY' Machine-generated release PR for **v__VERSION__**, base commit A = `__A__`, - opened by [release-build run __RUN_ID__](__RUN_URL__). Process: - `docs/ai/design/release-process.md`. + opened by [release-build run __RUN_ID__](__RUN_URL__). Two machine-written commits: diff --git a/.github/workflows/release-publish.yml b/.github/workflows/release-publish.yml index c8337d073..2c0c4993b 100644 --- a/.github/workflows/release-publish.yml +++ b/.github/workflows/release-publish.yml @@ -1,13 +1,13 @@ -# Phase 2 of the release system (docs/ai/design/release-process.md §5): a v* tag +# Phase 2 of the release system: a v* tag # push re-verifies commit B's provenance record against primary sources and # publishes the already-built artifacts. Publishing never rebuilds anything: the # bytes attached to the Release are the bytes release-build validated, fetched # from that run by the run ID pinned inside B. Any check failing means no -# release, loudly (design §11) -- recovery is always: delete the tag, fix the +# release, loudly -- recovery is always: delete the tag, fix the # cause, re-push. # # GitHub runs this file as it exists at the tagged commit, so only commits whose -# trees already carry release-publish.yml are releasable (design §4). +# trees already carry release-publish.yml are releasable. name: release-publish on: @@ -37,7 +37,7 @@ jobs: run: | set -euo pipefail fail() { - echo "::error::release-publish: $*. Nothing was published. Recover by deleting the tag (git push origin :refs/tags/$TAG), fixing the cause, and re-pushing (docs/ai/design/release-process.md, section 11)." + echo "::error::release-publish: $*. Nothing was published. Recover by deleting the tag (git push origin :refs/tags/$TAG), fixing the cause, and re-pushing." exit 1 } B=$(git rev-parse "refs/tags/$TAG^{commit}") @@ -109,7 +109,7 @@ jobs: run: | set -euo pipefail fail() { - echo "::error::release-publish: $*. Nothing was published. Recover by deleting the tag (git push origin :refs/tags/$TAG), fixing the cause, and re-pushing (docs/ai/design/release-process.md, section 11)." + echo "::error::release-publish: $*. Nothing was published. Recover by deleting the tag (git push origin :refs/tags/$TAG), fixing the cause, and re-pushing." exit 1 } # The API path scopes the lookup to this repository, so a run ID from @@ -127,7 +127,7 @@ jobs: run: | set -euo pipefail fail() { - echo "::error::release-publish: $*. Nothing was published. Recover by deleting the tag (git push origin :refs/tags/$TAG), fixing the cause, and re-pushing (docs/ai/design/release-process.md, section 11)." + echo "::error::release-publish: $*. Nothing was published. Recover by deleting the tag (git push origin :refs/tags/$TAG), fixing the cause, and re-pushing." exit 1 } : > assets.txt @@ -172,7 +172,7 @@ jobs: run: | set -euo pipefail fail() { - echo "::error::release-publish: $*. Nothing was published. Recover by deleting the tag (git push origin :refs/tags/$TAG), fixing the cause, and re-pushing (docs/ai/design/release-process.md, section 11)." + echo "::error::release-publish: $*. Nothing was published. Recover by deleting the tag (git push origin :refs/tags/$TAG), fixing the cause, and re-pushing." exit 1 } if gh release view "$TAG" --repo "$GITHUB_REPOSITORY" > /dev/null 2>&1; then diff --git a/AGENTS.md b/AGENTS.md index b390823b5..856297e1e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,46 +16,36 @@ You should, however, automatically fix stale code examples. ``` mx/ - AGENTS.md <- you are here - .claude/skills/ <- agent skills; every skill name starts with mx- - Makefile <- top-level build driver - cmake/ <- CMake helpers: GitSha.cmake, and the backend-pugixml consumer project - that `make backend-pugixml-test` builds (mx against a stock pugixml) - scripts/ <- maintenance scripts (vendor_pugixml.py refreshes src/private/mxpugi) - Dockerfile <- mx-sdk image with toolchains and dev tools - CMakeLists.txt <- C++ project - Package.swift <- Swift Package Manager wrapper (built by the swift CI job) - data/ <- MusicXML test corpus (large, see data/README.md) - docs/ai/design/ <- design docs - docs/musicxml-4.0-ed15c23.xsd <- the current musicxml.xsd schema - src/ <- C++ sourcecode root - src/include/mx/api/ <- the public mx::api headers; mx's public interface - src/private/ <- C++ source - mxpugi/ <- vendored pugixml 1.16 under the private mxpugi namespace (generated by - scripts/vendor_pugixml.py; XML parser and raw DOM) - mx/xml/ <- the XML seam: mx::xml::{Document,Node,Attribute} value handles, the - XmlBackend concept, and the pugixml adapter (mx/xml/pugixml/); the only - code under mx/ that includes mxpugi (docs/ai/design/xml-backend.md) - mx/core/ <- mx::core's hand-written runtime (outside of mx/core/generated) - mx/core/generated/ <- MusicXML XSD C++ typed model (generated, see gen/cpp) - mx/api/ <- mx::api implementation - mx/impl/ <- the api to core translation layer - mx/utility/ <- helpers - mx/examples/ <- api example programs - mxtest/core/ <- mx::core unit tests (small) - mxtest/corert/ <- C++ core roundtrip test (deserializes and serializes the test corpus) - mxtest/import/ <- normalization support (Normalize.cpp/h + DecimalFields.h) shared by corert and api tests - mxtest/api/ <- tests for the mx::api layer - mxtest/impl/ <- tests for the mx::impl layer - mxtest/file/ <- file-walking test infrastructure for the mx::api tests (MxFile, MxFileTest, etc.) - mxtest/control/ <- compile-control flags (CompileControl.h: enables/disables test suite compilation) - cpul/ <- vendored Catch2 test runner - gen/ <- code generator system (see gen/README.md) - cpp/ <- C++ target (the product): config.toml + templates - schema/ <- JSON Schema target - test/go/ <- A toy Go implementation of MusicXML XSD for gen validation - test/c/ <- A toy C implementation of MusicXML XSD for gen validation - audit/ <- MusicXML feature-audit tool (see audit/README.md); `make audit` + .claude/skills/ <- agent skills; every skill name starts with mx- + Makefile <- top-level build driver + cmake/ <- CMake helpers and the backend-pugixml consumer project + scripts/ <- maintenance scripts (vendor_pugixml.py refreshes mxpugi) + Dockerfile <- mx-sdk image with toolchains and dev tools + deslag.toml <- prose lint config; `make fmt-check` runs deslag + data/ <- MusicXML test corpus (large, see data/README.md) + docs/ai/design/ <- design docs + docs/musicxml-4.0-ed15c23.xsd <- the current musicxml.xsd schema + src/include/mx/api/ <- the public mx::api headers; mx's public interface + src/private/mxpugi/ <- vendored pugixml 1.16 in the mxpugi namespace (generated) + src/private/mx/xml/ <- the XML seam; the only mx/ code that includes mxpugi + src/private/mx/core/ <- mx::core's hand-written runtime + src/private/mx/core/generated/ <- MusicXML XSD C++ typed model (generated, see gen/cpp) + src/private/mx/api/ <- mx::api implementation + src/private/mx/impl/ <- the api to core translation layer + src/private/mx/utility/ <- helpers + src/private/mx/examples/ <- api example programs + src/private/mxtest/core/ <- mx::core unit tests (small) + src/private/mxtest/corert/ <- core roundtrip test over the test corpus + src/private/mxtest/import/ <- normalization shared by corert and api tests + src/private/mxtest/api/ <- tests for the mx::api layer + src/private/mxtest/impl/ <- tests for the mx::impl layer + src/private/mxtest/file/ <- file-walking infrastructure for the mx::api tests + src/private/cpul/ <- vendored Catch2 test runner + gen/ <- code generator system (see gen/README.md) + gen/cpp/ <- C++ target (the product): config.toml + templates + gen/schema/ <- JSON Schema target + gen/test/ <- toy Go and C targets that validate the generator + audit/ <- MusicXML feature-audit tool (see audit/README.md) ``` ## Feature audit (`audit/`) @@ -113,8 +103,8 @@ instead: - In code generator templates (`gen/cpp/templates/`): use `{{ident}}` to make names unique, e.g. `k{{ident}}Wire`, `isNameChar{{ident}}`. -Unity builds can be tested with CMake's built-in support — no changes to `CMakeLists.txt` are needed -by contributors. To verify, configure with: +Unity builds can be tested with CMake's built-in support; contributors do not need to change +`CMakeLists.txt`. To verify, configure with: ``` cmake -S . -B build/unity -DCMAKE_UNITY_BUILD=ON -DCMAKE_UNITY_BUILD_BATCH_SIZE=0 @@ -131,12 +121,12 @@ than the strongly-typed `mx::core` model. No core type may appear in a public ap - New fields that can be absent: use `std::optional` (values and sub-structs alike). Precedent: `NoteData::tieLetRing`, `PartData::transposition`, `MeasureData::partSymbol`. - Exception, enums: an absent-able enum gets an `unspecified` enumerator as its first value and - default — never `std::optional`. This includes the ternary + default, never `std::optional`. This includes the ternary `Bool { unspecified, yes, no }` in `ApiCommon.h`; use it, never `std::optional`. - Do not mass-migrate existing `bool is...Specified` / `has...` flags or `-1` sentinels; they are legacy (issue #249). Migrations are separate, deliberate breaking changes. - Every new field needs a default value and a matching `MXAPI_EQUALS_MEMBER(field)` line - (`MXAPI_DOUBLES_EQUALS_MEMBER` for doubles) in the type's equality block — a missed line silently + (`MXAPI_DOUBLES_EQUALS_MEMBER` for doubles) in the type's equality block. A missed line silently drops the field from equality and round-trip checks. ### Design doctrine @@ -147,21 +137,27 @@ no exceptions, `Result` quarantined), choice types, and comment rules. ## Quality gates -Run `make fmt` to format. `make fmt-check` is the clang-format gate **only** — it builds and tests -nothing. Run `make core-roundtrip-test` for corert (especially `mx/core` work) before pushing core -work. `make api-test` runs the api/impl suite + examples; `make api-roundtrip` and the gen/audit -gates run separately. `make test-all` runs every C++ suite at once (core roundtrip + unit + api-test -\+ api-roundtrip) — the deep gate CI runs on Linux (see `make help` / CI). Adding/removing a `data/` -file: bump the pinned count in `CoreRoundtripTest.cpp`, run `make audit` (regenerates `corpus.xml` + -`*.features.xml`), confirm round-trip via `make core-roundtrip-test`. `ApiLoadSmokeTest` proves a -file imports without crashing, not that the data is correct; the read→write→read gate -(`make api-roundtrip` / `roundtrip-baseline.txt`) is the correctness check — pin a fixture there to -defend a feature. `make wasm-test` builds `mx::api` for Emscripten and runs the examples under Node -— CI's check that mx still compiles and works client-side in a browser (a consumer, denigma, depends -on this; issue #386). `make backend-pugixml-test` builds mx against a stock pugixml -(`MX_XML_BACKEND=pugixml`, see the README's "Bringing your own XML library") and runs the core -suites — CI's check that the XML backend option works; `make symbol-check` only applies to the -default vendored build. +Run `make fmt` to format. `make fmt-check` is the formatting and prose gate: it runs clang-format +and `deslag check` (which reads `deslag.toml`), and it does not build or run tests. Fix what deslag +reports by its advice; only the owner changes its limits or adds an `allow` entry. + +Run `make core-roundtrip-test` for corert (especially `mx/core` work) before pushing core work. +`make api-test` runs the api/impl suite + examples; `make api-roundtrip` and the gen/audit gates run +separately. `make test-all` runs every C++ suite at once (core roundtrip + unit + api-test + +api-roundtrip), the deep gate CI runs on Linux (see `make help` / CI). + +Adding/removing a `data/` file: bump the pinned count in `CoreRoundtripTest.cpp`, run `make audit` +(regenerates `corpus.xml` + `*.features.xml`), confirm round-trip via `make core-roundtrip-test`. + +`ApiLoadSmokeTest` proves a file imports without crashing, not that the data is correct. The +read->write->read gate (`make api-roundtrip` / `roundtrip-baseline.txt`) is the correctness check; +pin a fixture there to defend a feature. + +`make wasm-test` builds `mx::api` for Emscripten and runs the examples under Node: CI's check that mx +still compiles and works client-side in a browser (a consumer, denigma, depends on this; issue +#386). `make backend-pugixml-test` builds mx against a stock pugixml (`MX_XML_BACKEND=pugixml`, see +the README's "Bringing your own XML library") and runs the core suites: CI's check that the XML +backend option works. `make symbol-check` only applies to the default vendored build. Look at what will run in CI `.github/workflows/ci.yaml` and anticipate issues there when coding locally. Code coverage is not part of the normal CI run; trigger it on demand with a `/coverage` diff --git a/CMakeLists.txt b/CMakeLists.txt index b865c29bd..038e04f39 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -6,7 +6,7 @@ project(mx CXX) # + the mx::xml seam over the XML library (src/private/mx/xml, backed by mx's # private copy of pugixml, mxpugi, unless MX_XML_BACKEND says otherwise), plus # the corert/unit test binaries. -# See AGENTS.md, docs/ai/design/mx-core-plan.md and docs/ai/design/xml-backend.md. +# See AGENTS.md and docs/ai/design/xml-backend.md. set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) @@ -108,7 +108,7 @@ set(PRIVATE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/src/private") set(PUBLIC_DIR "${CMAKE_CURRENT_SOURCE_DIR}/src/include") # mxpugi: the XML library behind mx::core's XML seam in the default -# (vendored) mode (plan §2.1 chose pugixml). It is pugixml, vendored at +# (vendored) mode (mx::core uses pugixml directly). It is pugixml, vendored at # src/private/mxpugi by scripts/vendor_pugixml.py with its namespace renamed # from `pugi` to `mxpugi` (stock pugiconfig). The private copy exists so that # mx does not define any `pugi::` symbol: a consumer that links its own diff --git a/Makefile b/Makefile index 6bd4b293d..d0201da86 100644 --- a/Makefile +++ b/Makefile @@ -55,7 +55,8 @@ GEN_PY := $(shell find gen -name '*.py' ! -name quality.py | sort) GEN_QUALITY_FLOOR ?= 84.5 GEN_LINT_FLOOR ?= 9.29 -# deslag, a Rust CLI. No target uses it yet. The version is pinned once, in the +# deslag, a Rust CLI that lints prose (deslag.toml picks the files). fmt-check +# runs it. The version is pinned once, in the # Dockerfile. The mx-sdk image installs it and sets DESLAG to it. Natively, # `make deslag` installs it under build/ with a cargo home of its own, so it # does not use or change the deslag or the cargo home in your home directory. @@ -178,9 +179,9 @@ help: @echo '' @echo ' Housekeeping:' @echo ' make doctor Report which targets this machine can run, with Docker or natively.' - @echo ' make deslag Native only: install deslag $(DESLAG_VERSION) at $(DESLAG_HOME) (needs cargo). No target uses it yet.' + @echo ' make deslag Native only: install deslag $(DESLAG_VERSION) at $(DESLAG_HOME) (needs cargo). fmt-check runs it.' @echo ' make fmt Format C++ under src/ with clang-format.' - @echo ' make fmt-check clang-format gate (checks formatting; builds nothing).' + @echo ' make fmt-check clang-format and deslag gate (checks formatting and prose; builds nothing).' @echo ' make sdk Build the mx-sdk Docker toolchain image.' @echo ' make vendor-pugixml VERSION=x.y Refresh src/private/mxpugi from a pugixml release (runs on the host).' @echo ' make clean Remove the build/ tree and gen build artifacts.' @@ -538,6 +539,8 @@ fmt: fmt-check: @$(FIND_CPP) | xargs -r $(CLANG_FORMAT) --dry-run --Werror + @test -x '$(DESLAG)' || { echo "No deslag at $(DESLAG). Run: make deslag"; exit 1; } + @'$(DESLAG)' check @echo "fmt-check passed." gen: gen-cpp gen-go gen-c gen-schema diff --git a/Package.swift b/Package.swift index 90a935c83..2c8b0aa00 100644 --- a/Package.swift +++ b/Package.swift @@ -10,7 +10,7 @@ import PackageDescription // machine-generated binary-default manifest (an XCFramework binaryTarget) // interpolated from .github/release/Package.swift.template by the // release-build workflow. That manifest exists only at tagged release commits, -// never at a tip of main. See docs/ai/design/release-process.md. +// never at a tip of main. // // KEEP IN SYNC: the target below must stay byte-for-byte identical to // mxSourceTarget in .github/release/Package.swift.template, so a release built diff --git a/deslag.toml b/deslag.toml new file mode 100644 index 000000000..2987af393 --- /dev/null +++ b/deslag.toml @@ -0,0 +1,122 @@ +schema_version = 2 +deslag_version = "0.0.6" + +[md] +globs = ["/AGENTS.md", "/docs/**/*.md"] + +# The byte budget. +[md.lints.max_size_bytes] +value = 16000 + +# The limit on bold, italics and ALL CAPS. +[md.lints.max_emphasis] +free_spans = 2 +max_percent = 1 + +# The characters the file must not hold, such as the em dash. +[md.lints.banned_chars] + +[md.lints.banned_chars.groups] +arrows = true +bidi = true +box_drawing = true +bullets = true +checks = true +dashes = true +ellipsis = true +emoji = true +invisible = true +math = true +quotes = true +section = true +spaces = true + +# The phrases the file must not hold, which the config lists. +[md.lints.banned_phrases] + +[md.lints.banned_phrases.ban] +"deliberately not" = "say what it does; what it does not do rarely needs saying" +"not silently" = "say what it does instead, such as the error it reports" +"never silently" = "say what it does instead, such as the error it reports" +load-bearing = "say what depends on it" +"lands on" = 'say what happens, such as "is merged into" or "reaches"' +"it lands" = 'say what happens, such as "it is merged" or "it ships"' +byte-identical = 'say "identical", or name the check that shows it' + +# The longest a paragraph or list item may be. +[md.lints.density] +max_item_chars = 350 +max_paragraph_chars = 700 + +# That a sentence negates its verb, not its object. +[md.lints.verbs_no_nouns] + +# That a negation drops `at all`. +[md.lints.not_at_all] + +# That a sentence names what its verb did not find, not `nothing` or `none`. +[md.lints.verbs_nothing] + +# That a possessive noun is not padded with `own`. +[md.lints.possessive_own] + +[[md.overrides]] +globs = ["/AGENTS.md"] + +# The index of the repo that the file must hold. +[md.overrides.lints.repo_layout] +heading = "Repository layout" +max_entries = 30 +max_width = 100 +min_entries = 5 + +# mx::api headers and sources. Generated code (src/private/mx/core/generated) is +# never onboarded. +[cpp] +globs = ["/src/include/mx/api/**/*.h", "/src/private/mx/api/**/*.h", "/src/private/mx/api/**/*.cpp"] +exclude = ["/src/private/mx/core/generated/**/*.h", "/src/private/mx/core/generated/**/*.cpp"] + +[cpp.lints.max_emphasis] +free_spans = 2 +max_percent = 1 + +[cpp.lints.banned_chars] + +[cpp.lints.banned_chars.groups] +arrows = true +bidi = true +box_drawing = true +bullets = true +checks = true +dashes = true +ellipsis = true +emoji = true +invisible = true +math = true +quotes = true +section = true +spaces = true + +[cpp.lints.banned_phrases] + +[cpp.lints.banned_phrases.ban] +"deliberately not" = "say what it does; what it does not do rarely needs saying" +"not silently" = "say what it does instead, such as the error it reports" +"never silently" = "say what it does instead, such as the error it reports" +load-bearing = "say what depends on it" +"lands on" = 'say what happens, such as "is merged into" or "reaches"' +"it lands" = 'say what happens, such as "it is merged" or "it ships"' +byte-identical = 'say "identical", or name the check that shows it' + +[cpp.lints.density] +max_item_chars = 350 +max_paragraph_chars = 700 + +[cpp.lints.verbs_no_nouns] + +[cpp.lints.not_at_all] + +[cpp.lints.verbs_nothing] + +[cpp.lints.possessive_own] + diff --git a/docs/ai/design/api-roundtrip-classifier.md b/docs/ai/design/api-roundtrip-classifier.md index aebe49a99..9a1d2f3b5 100644 --- a/docs/ai/design/api-roundtrip-classifier.md +++ b/docs/ai/design/api-roundtrip-classifier.md @@ -1,37 +1,37 @@ -# api round-trip failure classifier — diff design +# api round-trip failure classifier - diff design Design notes for the `audit/classify.py` classifier (issue #211, Phase 2 of #208). -This document explains *how the classifier diffs the expected/actual dump pairs* +This document explains how the classifier diffs the expected/actual dump pairs and why. It supersedes the original "walk both trees in parallel until the first mismatch" sketch in the #211 issue body, which has a structural defect described below. ## Update (#212): measurement-only classification -The classifier was reworked to be grounded **only** in what the round-trip +The classifier was reworked to be grounded only in what the round-trip measurably did to each file. The earlier version cross-referenced `data/api.features.xml` to label drops as "by-design" (category B) vs. bug (category G) and to recognize enum/attribute gaps (D/E). That `support` field is -hand-authored opinion — a *prediction* of round-trip behavior, not a measurement -— and it can be wrong (part-group was marked `full` yet dropped data; see #224). +hand-authored opinion - a prediction of round-trip behavior, not a measurement, +and it can be wrong (part-group was marked `full` yet dropped data; see #224). Keying classification on a fallible prediction made the worklist untrustworthy. What changed: -- **No `api.features.xml`.** The support cross-reference (old Layer 3 and the +- No `api.features.xml`. The support cross-reference (old Layer 3 and the B/D/E/G category logic) is removed. Whether a drop is intended is a present-day human decision (#214), not a property the classifier asserts. -- **Categories → signatures.** Every difference is reduced to a signature: +- Categories -> signatures. Every difference is reduced to a signature: `drop:`, `add:`, `value:`, `value:@`, - `attr:@`, `reorder:`. A file's **distance** to passing is - its count of *unique* signatures (a tag dropped a thousand times is one + `attr:@`, `reorder:`. A file's distance to passing is + its count of unique signatures (a tag dropped a thousand times is one `drop:` signature, distance 1). -- **Statuses** are just `PASS` / `FAIL` / `CRASH` (the three pipeline codes fold +- Statuses are just `PASS` / `FAIL` / `CRASH` (the three pipeline codes fold into `CRASH`). -- **Candidates and the worklist.** A FAIL with no `reorder:` signature is a - *candidate* (reorders are expected `mx::api` behavior, to be absorbed in test +- Candidates and the worklist. A `FAIL` with no `reorder:` signature is a + candidate (reorders are expected `mx::api` behavior, to be absorbed in test normalization later, #214). The worklist ranks each signature by the number of - candidate files it is the *sole* blocker of (fixing it flips them green now), + candidate files it is the sole blocker of (fixing it flips them green now), then by total candidate files blocked. Layers 1 (multiset for drops/adds) and 4 (alignment, now also used for @@ -42,13 +42,13 @@ rationale; the current schema is at the end of this section. ## The defect in the naive design Both `compareElements` (`src/private/mxtest/corert/Compare.cpp`) and the original -#211 step 3b do the same thing: a **positional, paired, depth-first walk** of the -two trees that stops at the **first** divergence. That is correct as a pass/fail -gate (corert only needs to know *whether* the documents differ), but it is wrong -as an *analysis* tool. +#211 step 3b do the same thing: a positional, paired, depth-first walk of the +two trees that stops at the first divergence. That is correct as a pass/fail +gate (corert only needs to know whether the documents differ), but it is wrong +as an analysis tool. `mx::api` is a deliberate subset of MusicXML: it drops whole subtrees and reorders -children **by design**. The dominant failure signal is therefore a *deletion* — +children by design. The dominant failure signal is therefore a deletion - an element present in `expected` that never appears in `actual`. A positional walk cannot survive a deletion: @@ -59,34 +59,34 @@ actual children: [part-list] [part] [???] ... <- credit droppe The moment `actual` is missing one child, the two child-lists shift out of alignment, and every subsequent sibling is compared against the wrong partner. -You can only trust the **first** reported divergence; everything after it is noise. +You can only trust the first reported divergence; everything after it is noise. A single drop poisons the rest of the file. This is a well-documented property of diffing: a naive index-by-index comparison breaks because one insert/delete shifts every later index, so the differ keeps matching content at positions that no longer correspond. The principled fixes are -**alignment** (LCS / Myers) or, when position is irrelevant, an **order-free -multiset** comparison. See the research summary at the end of this document. +alignment (LCS / Myers) or, when position is irrelevant, an order-free +multiset comparison. See the research summary at the end of this document. ## What we actually need -The classifier's job is *not* to produce a minimal edit script. It is to answer: +The classifier's job is not to produce a minimal edit script. It is to answer: -1. **Which element classes (tag names) are present in `expected` but missing from - `actual`?** — the complete set, not just the first. -2. **How many *distinct* element classes are missing per file?** — the headline +1. Which element classes (tag names) are present in `expected` but missing from + `actual`? - the complete set, not just the first. +2. How many distinct element classes are missing per file? - the headline ranking metric. -3. **Which files are missing exactly one distinct class** (once or many times)? — - the low-hanging fruit: a single api addition flips them to PASS. -4. A cheap **"distance to passing"** scalar for secondary tie-breaking. +3. Which files are missing exactly one distinct class (once or many times)? - + the low-hanging fruit: a single api addition flips them to `PASS`. +4. A cheap "distance to passing" scalar for secondary tie-breaking. -None of these require positions or a minimal edit script. They require a *complete -inventory of deletions*, which is exactly what a multiset difference gives in +None of these require positions or a minimal edit script. They require a complete +inventory of deletions, which is exactly what a multiset difference gives in linear time, fully invariant to reordering. Exact order-insensitive structural diffing (unordered tree edit distance) is NP-complete, and ordered tree edit distance (Zhang-Shasha / RTED / APTED) is -O(n³)/O(n²) — both far more than this problem needs. We deliberately do **not** +O(n³)/O(n²) - both far more than this problem needs. We deliberately do not use them. ## The layered design @@ -94,10 +94,10 @@ use them. All layers operate on the two normalized dump documents (`*.expected.xml`, `*.actual.xml`) using only the Python standard library (`xml.etree.ElementTree`, `collections.Counter`, `difflib`). No `lxml`, `xmldiff`, -`zss`, or `apted` dependency is introduced — the multiset primitive covers the need +`zss`, or `apted` dependency is introduced - the multiset primitive covers the need in O(n). -### Layer 1 — multiset tag diff (primary signal) +### Layer 1 - multiset tag diff (primary signal) Recurse both trees, building a `Counter` of element tag names: @@ -116,15 +116,15 @@ multiset of element classes the round-trip dropped. From it: | field | meaning | |---|---| -| `missing_elements` | `sorted(missing)` — the distinct dropped tag names | -| `missing_element_counts` | `dict(missing)` — how many of each were dropped | -| `distinct_missing_count` | `len(missing)` — **the headline ranking metric** | -| `added_elements` | `sorted(added)` — spurious tags the api invented (bug signal) | +| `missing_elements` | `sorted(missing)` - the distinct dropped tag names | +| `missing_element_counts` | `dict(missing)` - how many of each were dropped | +| `distinct_missing_count` | `len(missing)` - the headline ranking metric | +| `added_elements` | `sorted(added)` - spurious tags the api invented (bug signal) | -This is O(n), reorder-invariant, and enumerates **every** dropped class — it does +This is O(n), reorder-invariant, and enumerates every dropped class - it does not stop at the first one. It is the direct fix for the cascade defect. -### Layer 2 — path-qualified multiset (attribution) +### Layer 2 - path-qualified multiset (attribution) Bare tag names are ambiguous (`` under `` vs. elsewhere). For `blocking_features` attribution and to line up with `first_divergence_path`, build @@ -132,40 +132,40 @@ a second multiset keyed by `parent/tag` (or the full root-to-node path). The headline `distinct_missing_count` stays on bare tags; path qualification is used when mapping a drop back to an `api.features.xml` feature. -### Layer 3 — cross-reference `api.features.xml` *(REMOVED in #212; historical)* +### Layer 3 - cross-reference `api.features.xml` (REMOVED in #212; historical) This layer was removed: classification no longer consults `api.features.xml`. The text below is retained only to explain what the categories used to mean. For each missing tag, the old version looked up `support` in `api.features.xml`: -- **Category B (drop-only)** becomes *provable across the whole file*: it holds iff - **every** tag in `missing` has `support="none"`. The naive walk could never - establish this — it only saw the first drop. This is the single biggest +- Category B (drop-only) becomes provable across the whole file: it holds iff + every tag in `missing` has `support="none"`. The naive walk could never + establish this - it only saw the first drop. This is the single biggest correctness win of the redesign. - A missing tag with `support="full"` or `"partial"` is a genuine correctness bug - or a partial-drop — surfaced explicitly instead of being hidden behind whatever + or a partial-drop - surfaced explicitly instead of being hidden behind whatever the first divergence happened to be. -### Layer 4 — sequence alignment per parent (reorder + paired value/attr) +### Layer 4 - sequence alignment per parent (reorder + paired value/attr) `difflib.SequenceMatcher` over the two child-tag sequences earns its keep twice: -- **Reorder** (`reorder:`): a parent whose child *multiset* is equal but - whose child *sequence* differs is a pure permutation. -- **Paired value/attr** (`value:`/`attr:`): recursing only the matcher's `equal` +- Reorder (`reorder:`): a parent whose child multiset is equal but + whose child sequence differs is a pure permutation. +- Paired value/attr (`value:`/`attr:`): recursing only the matcher's `equal` blocks pairs surviving elements across drops without desyncing, so text and attribute differences are compared between genuinely corresponding nodes (never across mismatched tags). This is how value/attr diffs survive a sibling drop. -It is **not** used for the deletion inventory (Layer 1's multiset already has +It is not used for the deletion inventory (Layer 1's multiset already has that). Stdlib, zero dependency. -### Layer 5 — edit-distance scalar (secondary) +### Layer 5 - edit-distance scalar (secondary) Use `sum(missing.values())` (total dropped instances) as a cheap "distance to -passing." It is a *secondary* tie-breaker only. We do **not** key ranking on it: -a scalar conflates "one big dropped subtree" with "many tiny diffs" and names no -feature. Full tree edit distance would also pull in a heavyweight dependency for +passing." It is a secondary tie-breaker only. We do not key ranking on it: +a scalar conflates "one big dropped subtree" with "many tiny diffs" and does not +name a feature. Full tree edit distance would also pull in a heavyweight dependency for no added decision value here. ## Ranking output (the worklist) @@ -177,11 +177,11 @@ prioritized api-addition worklist. Two refinements from the regression-triage literature (e.g. BuildSheriff, ICSE'22: failures cluster around a few root causes; fixing one unblocks a cluster): -1. **Rank by files *unblocked*, not occurrence count.** A tag appearing 10,000× - in one file is lower yield than a tag that is the *sole* blocker of 200 files. +1. Rank by files unblocked, not occurrence count. A tag appearing 10,000 times + in one file is lower yield than a tag that is the sole blocker of 200 files. The y-axis is "count of files whose single blocking class is tag X." -2. **Histogram on the categorical root cause (missing tag), not the edit-distance - scalar.** Keep the scalar as a within-cluster severity tie-breaker. +2. Histogram on the categorical root cause (missing tag), not the edit-distance + scalar. Keep the scalar as a within-cluster severity tie-breaker. This belongs to Phase 3 (#212) ranking, but the per-file fields it needs (`distinct_missing_count`, `missing_elements`, single-blocker flag) are emitted @@ -217,10 +217,10 @@ document). So the dump step (#210) writes a tiny `.status` sidecar next to the expected file recording the exact code. The classifier reads it into `crash_kind` (with `status = "CRASH"`); when the sidecar is absent, `status` is still `"CRASH"` and `crash_kind` is `null`. The presence/absence of `.actual.xml` -remains the FAIL-vs-CRASH signal; the `.status` sidecar only refines *which* +remains the `FAIL`-vs-`CRASH` signal; the `.status` sidecar only refines which crash. The sidecar lives in the gitignored dump dir and is never checked in. -### Category G — `support="full"`/`"partial"` drops *(REMOVED in #212)* +### Category G - `support="full"`/`"partial"` drops (REMOVED in #212) Category G (and the B/D/E categories) existed only because classification used to read `api.features.xml`. With that cross-reference gone, every dropped tag is just @@ -228,7 +228,7 @@ a `drop:` signature regardless of any prior belief about whether it was "supported"; the supported-vs-by-design judgment is a present-day human call (#214), not the classifier's. `part-group` (marked `support="full"` yet dropped; #224) was the case that proved the `support` field could not be trusted as a -classification input — which is what motivated removing it. The drop is still +classification input - which is what motivated removing it. The drop is still reported, now as `drop:part-group`, ranked purely by how many files it blocks. ## Dependency decision @@ -250,52 +250,52 @@ claim from training knowledge that the fetched sources did not directly confirm ### Alignment vs. positional walk - A naive index-by-index walk cascades after one insert/delete; Myers models diff as shortest-path search on an edit graph where diagonal (match) moves are free, so - shared content anchors the alignment. *(High — jcoglan.com/2017/02/12/the-myers-diff-algorithm-part-1/; xmailserver.org/diff2.pdf)* -- Microsoft XML Diff documents three child-matching modes — By Index (naive), LCS - (survives ins/del), Unordered (set-like). *(Med-high — documentation.help/Microsoft-XML-Diff)* + shared content anchors the alignment. (High - jcoglan.com/2017/02/12/the-myers-diff-algorithm-part-1/; xmailserver.org/diff2.pdf) +- Microsoft XML Diff documents three child-matching modes - By Index (naive), LCS + (survives ins/del), Unordered (set-like). (Med-high - documentation.help/Microsoft-XML-Diff) ### Tree edit distance (decided against) - Zhang-Shasha O(n⁴)/O(nm); Klein O(n³ log n); Demaine'07 O(n³); RTED O(n³)/O(n²); - APTED O(n³)/O(n²) with an edit *mapping* (unmapped source nodes = deletions). - *(High — erikdemaine.org/papers/TreeEdit_ICALP2007/paper.pdf; arxiv 1201.0230; - dl.acm.org/doi/10.1145/2699485; github.com/DatabaseGroup/apted)* -- **Unordered** tree edit distance is NP-complete / MAX-SNP-hard even for binary - trees over a 2-symbol alphabet. *(High — Bille survey, sciencedirect - S0304397505000174; arxiv 2108.00953)* + APTED O(n³)/O(n²) with an edit mapping (unmapped source nodes = deletions). + (High - erikdemaine.org/papers/TreeEdit_ICALP2007/paper.pdf; arxiv 1201.0230; + dl.acm.org/doi/10.1145/2699485; github.com/DatabaseGroup/apted) +- Unordered tree edit distance is NP-complete / MAX-SNP-hard even for binary + trees over a 2-symbol alphabet. (High - Bille survey, sciencedirect + S0304397505000174; arxiv 2108.00953) ### XML-specific algorithms - Chawathe et al. 1996 (basis of Python `xmldiff`): match phase + edit-script phase, - ordered, emits INSERT/UPDATE/MOVE/DELETE — enumerates deletions and reports a - reorder as one MOVE. *(High on ops; exact complexity bound UNVERIFIED — primary - PDF 503'd. sigmodrecord.org/1996/06/24/...; github.com/cfpb/xtdiff)* + ordered, emits INSERT/UPDATE/MOVE/DELETE - enumerates deletions and reports a + reorder as one MOVE. (High on ops; exact complexity bound UNVERIFIED - primary + PDF 503'd. sigmodrecord.org/1996/06/24/...; github.com/cfpb/xtdiff) - X-Diff: unordered model (reorder is a non-change), bottom-up signature hashing + - min-cost sibling matching; harder to compute, no MOVE op. *(Ordered/unordered: - high — pages.cs.wisc.edu/~yuanwang/xdiff.html; "no MOVE": UNVERIFIED)* + min-cost sibling matching; harder to compute, no MOVE op. (Ordered/unordered: + high - pages.cs.wisc.edu/~yuanwang/xdiff.html; "no MOVE": UNVERIFIED) ### Sequence alignment & multiset - Myers O(ND) powers git/GNU diff; per-parent LCS over child sequences is an - established lightweight alternative to TED (XMLMind). *(High — xmailserver.org/diff2.pdf; - xmlmind.com/.../xmlDiff_algorithm.html)* + established lightweight alternative to TED (XMLMind). (High - xmailserver.org/diff2.pdf; + xmlmind.com/.../xmlDiff_algorithm.html) - `collections.Counter` is a multiset; `a - b` keeps only positive counts = elements in `a` not covered by `b`, directly enumerable; building is O(n), - set-ops O(n+m). *(High on semantics — docs.python.org/3/library/collections.html; - the explicit O(n) is standard reasoning, not doc-quoted → UNVERIFIED as a doc claim)* + set-ops O(n+m). (High on semantics - docs.python.org/3/library/collections.html; + the explicit O(n) is standard reasoning, not doc-quoted -> UNVERIFIED as a doc claim) ### Python libraries - `xmldiff` 3.0 (2026-06-11, MIT, active): Chawathe; enumerable `DeleteNode`/ - `MoveNode`/… actions; **requires lxml (C ext)**. -- `zss` (2018, BSD): Zhang-Shasha; **scalar distance only, no edit script**. + `MoveNode`/... actions; requires lxml (C ext). +- `zss` (2018, BSD): Zhang-Shasha; scalar distance only, no edit script. - `apted` (2017, MIT, pure Python): edit mapping (deletions = unmapped source nodes); needs bracket-notation input. - `lxml`: no semantic diff, only C14N canonicalization. - stdlib `difflib.SequenceMatcher` (opcodes) and `collections.Counter` (multiset): - zero-dependency, both enumerate deletions. *(All high — verified against live - PyPI/GitHub/ReadTheDocs)* + zero-dependency, both enumerate deletions. (All high - verified against live + PyPI/GitHub/ReadTheDocs) ### Yield-based ranking - BuildSheriff (ICSE'22, 163K builds): failures cluster around a few root causes; - triaging by shared cause saves cost — validates "many files, few blockers." - *(Med-high — chenbihuan.github.io/paper/icse22-zhang-buildsheriff.pdf)* -- Rank by **files unblocked**, not occurrence count; histogram on categorical root - cause, with edit distance only as a secondary severity scalar. *(Synthesized from - the triage literature above + frequency×impact practice)* + triaging by shared cause saves cost - validates "many files, few blockers." + (Med-high - chenbihuan.github.io/paper/icse22-zhang-buildsheriff.pdf) +- Rank by files unblocked, not occurrence count; histogram on categorical root + cause, with edit distance only as a secondary severity scalar. (Synthesized from + the triage literature above + frequency-times-impact practice) diff --git a/docs/ai/design/mx-core-gpt-5.0.md b/docs/ai/design/mx-core-gpt-5.0.md deleted file mode 100644 index 009a9905f..000000000 --- a/docs/ai/design/mx-core-gpt-5.0.md +++ /dev/null @@ -1,1114 +0,0 @@ -# C++ `mx/core` design - -Status: design proposal for the generated C++20 `mx::core` product. - -The Go and C targets proved the generator pipeline: XSD -> IR -> Plates -> target-owned Mustache -works, the press is language-agnostic, and the corert flow can be made green from generated code. -They also proved what the C++ target must **not** copy. Go and C are deliberately pragmatic test -bindings: they preserve document order with an ordered child list, parse values leniently, and -expose plain data. The C++ target is the real product and should be a stricter, deeper abstraction. - -## Position - -Generate a valid-by-construction, order-faithful MusicXML model. - -The public C++ API should make invalid MusicXML states unreachable through normal API use: - -- no public mutable fields; -- no public default constructors for model objects whose schema content is required; -- required attributes and required children appear in constructors or `make` factories; -- optional schema members use `std::optional` behind accessors; -- repeated members are stored privately and exposed as read-only spans; -- schema choices use `std::variant`, not nullable sibling fields; -- XML parsing validates structure and required content before producing a model object; -- import leniency may clamp out-of-range numbers, but the object produced is still valid. - -C++ cannot protect against undefined behavior, memory scribbles, malicious casts, or every possible -moved-from misuse. The contract is: the generated API does not provide an operation that creates an -invalid serializable model object. - -## Lessons from Go and C - -What to keep: - -1. **One source of truth.** All schema facts must still come from the Plates. C++ is a target - directory: `gen/cpp/config.toml` plus `gen/cpp/templates/`. Do not add C++ knowledge to - `gen/*.py`. -2. **Order fidelity matters.** A MusicXML measure interleaves `note`, `backup`, `direction`, - `attributes`, and other music-data elements. A flat field per element is not enough. -3. **Generated version constants matter.** The C++ runtime should emit `SupportedMusicXMLVersion` - from `Plates.schema_version`, just like Go and C now do. -4. **Unknown names are errors.** With version gating, an unknown element or attribute is a generator - or input problem, not data to preserve silently. -5. **Numeric clamping is an import policy.** The clamp steps now live on the Plates. C++ should use - that data exactly once in its value parsers. - -What not to keep: - -1. **No multi-null child structs.** Go/C use `NoteChild{Pitch: &p}` because they lack a good sum - type in the chosen style. C++ has `std::variant`; use it. -2. **No public structs as the product API.** Go/C are harness bindings. C++ should hide invariants - behind small interfaces. -3. **No always-flat child list.** The C++ target should consume `ComplexPlate.content`, not just - `elements`. Pure sequences should become sequence-shaped classes; repeated choices should become - ordered variants. -4. **No enum fallback to the first value for user construction.** Unknown enum input should fail. - Clamping numbers can be compatible with a valid model; inventing enum values is not. - -## C++ feature choices - -| Feature | Use | Rule | -|------------------------------|---------------------------------------------------------------------|-------------------------------------------------------------------------------------| -| C++20 | Target language level | Raise CMake to C++20 when this target lands. | -| `class` with private members | Generated model types | Default for all complex and constrained value types. | -| `enum class` | Internal tags | Do not expose raw enums as constructible domain values. Wrap them in value classes. | -| `std::variant` | XML choices, unions, document root | Exactly-one by construction. | -| `std::optional` | Optional attributes/elements and defaulted/fixed attribute presence | Never for required schema content. | -| `std::vector` | Repeated homogeneous content and repeated choice groups | Kept private; expose `std::span`. | -| `std::unique_ptr` | Alternatives inside repeated heterogeneous choice vectors | Avoids a huge `std::variant` object per child. | -| `std::span` | Read-only views of repeated content | No mutable vector exposure. | -| `std::string_view` | Parse/format APIs and wire names | Avoids unnecessary string copies at API boundaries. | -| `constexpr std::array` | Wire literal tables | No dynamic enum maps unless profiling proves need. | -| `std::from_chars` | Numeric lexical parsing | Strict, locale-free parsing. | -| non-polymorphic inheritance | Only for IR `derived` complex types | Schema extension only; no virtual model hierarchy. | -| `Result` runtime type | Parse/factory errors | C++20 has no `std::expected`; provide a small target runtime `Result`. | - -Avoid: - -- raw owning pointers; -- `new` / `delete` in generated code; -- public mutable containers; -- runtime polymorphism for every element; -- `std::shared_ptr` by default; -- `std::regex` in headers; -- template-heavy generated code. - -## Output layout - -Recommended generated layout under `src/private/mx/core/`: - -```text -mx/core/ - Document.h / Document.cpp - Result.h - Error.h - Decimal.h / Decimal.cpp - Xml.h / Xml.cpp - UpDown.h / UpDown.cpp - FontSize.h / FontSize.cpp - Pitch.h / Pitch.cpp - PartwiseMeasure.h / PartwiseMeasure.cpp - ... one header/source pair per generated type ... - Sources.cmake -``` - -Headers should contain type declarations, small accessors, and inline trivial factories. Parsing, -serialization, string tables, pattern checks, and visitors should live in `.cpp` files. Compile time -is the first optimization priority; do not dump large parse functions into headers. - -## Error and result shape - -The runtime support can define generic infrastructure, but generated code should use concrete result -spelling. Rendered code should look like this at call sites: - -```cpp -namespace mx::core -{ - -enum class ErrorCode -{ - unknownElement, - unknownAttribute, - missingRequiredElement, - missingRequiredAttribute, - wrongElementOrder, - tooManyElements, - invalidValue, - unsupportedVersion, -}; - -struct Error -{ - ErrorCode code; - std::string path; - std::string message; -}; - -Result parsePitch(const ezxml::XElement &element); -Result parseDocument(const ezxml::XDoc &doc); - -} // namespace mx::core -``` - -The generated functions should accumulate path context -(`/score-partwise/part[0]/measure[2]/note[1]`) so corert failures are diagnosable. The old -`bool fromXDoc(std::ostream&, ...)` shape can be kept as a thin adapter; it should not be the core -API. - -## Value type shapes - -### Closed enum values: wrapper class, not a raw enum - -A raw C++ enum can be fabricated with `static_cast`. Use a small value class with a private tag -constructor. Rendered `up-down` should look like this: - -```cpp -#pragma once - -#include "mx/core/Result.h" - -#include -#include - -namespace mx::core -{ - -class UpDown final -{ - public: - enum class Tag : std::uint8_t - { - up, - down, - }; - - static constexpr UpDown up() noexcept { return UpDown(Tag::up); } - static constexpr UpDown down() noexcept { return UpDown(Tag::down); } - - static Result parse(std::string_view text); - - constexpr Tag tag() const noexcept { return tag_; } - constexpr bool isUp() const noexcept { return tag_ == Tag::up; } - constexpr bool isDown() const noexcept { return tag_ == Tag::down; } - - std::string_view wire() const noexcept; - - private: - explicit constexpr UpDown(Tag tag) noexcept : tag_(tag) {} - - Tag tag_; -}; - -} // namespace mx::core -``` - -The `.cpp` keeps the wire table out of dependent translation units: - -```cpp -#include "mx/core/UpDown.h" - -#include - -namespace mx::core -{ -namespace -{ -constexpr std::array kWire = {"up", "down"}; -} - -Result UpDown::parse(std::string_view text) -{ - if (text == "up") - { - return UpDown::up(); - } - if (text == "down") - { - return UpDown::down(); - } - return Error{ErrorCode::invalidValue, {}, "invalid up-down value"}; -} - -std::string_view UpDown::wire() const noexcept -{ - return kWire[static_cast(tag_)]; -} - -} // namespace mx::core -``` - -### Numeric wrappers: strict construction, explicit import clamping - -The Plates already carry clamp policy. Use it for XML import, not for ordinary construction. -Rendered `midi-channel` should look like this: - -```cpp -#pragma once - -#include "mx/core/Result.h" - -#include - -namespace mx::core -{ - -class MIDIChannel final -{ - public: - static Result make(int value); - static Result parse(std::string_view text); - static MIDIChannel importClamp(int value) noexcept; - - constexpr int value() const noexcept { return value_; } - std::string toString() const; - - private: - explicit constexpr MIDIChannel(int value) noexcept : value_(value) {} - - int value_; -}; - -} // namespace mx::core -``` - -```cpp -#include "mx/core/MIDIChannel.h" -#include "mx/core/NumberParse.h" - -namespace mx::core -{ - -Result MIDIChannel::make(int value) -{ - if (value < 1 || value > 16) - { - return Error{ErrorCode::invalidValue, {}, "midi-channel must be in [1, 16]"}; - } - return MIDIChannel(value); -} - -MIDIChannel MIDIChannel::importClamp(int value) noexcept -{ - if (value < 1) - { - value = 1; - } - if (value > 16) - { - value = 16; - } - return MIDIChannel(value); -} - -Result MIDIChannel::parse(std::string_view text) -{ - const auto parsed = parseInteger(text); - if (!parsed) - { - return parsed.error(); - } - return MIDIChannel::make(parsed.value()); -} - -std::string MIDIChannel::toString() const -{ - return formatInteger(value_); -} - -} // namespace mx::core -``` - -The XML parser can call `importClamp` when the target chooses compatibility import. User code gets -`make`, which rejects invalid values. - -### Strings: domain wrappers own validation - -String wrappers should not be aliases. They are the only place length and pattern constraints can be -enforced. - -```cpp -#pragma once - -#include "mx/core/Result.h" - -#include -#include - -namespace mx::core -{ - -class Color final -{ - public: - static Result make(std::string value); - static Result parse(std::string_view text); - - const std::string &value() const noexcept { return value_; } - std::string_view wire() const noexcept { return value_; } - - private: - explicit Color(std::string value) : value_(std::move(value)) {} - - std::string value_; -}; - -} // namespace mx::core -``` - -Pattern validation belongs in `.cpp`. If the XSD regex subset needs custom handling (`\c` appears in -MusicXML), hide it in `XmlPattern.cpp`; do not generate ad-hoc regex logic into every header. - -### Unions: `std::variant` - -Rendered `font-size` should be a tiny sum type. This is strictly better than Go/C's hand-rolled -`Kind` plus fields because `std::variant` cannot hold two members at once. - -```cpp -#pragma once - -#include "mx/core/CSSFontSize.h" -#include "mx/core/Decimal.h" -#include "mx/core/Result.h" - -#include -#include -#include - -namespace mx::core -{ - -class FontSize final -{ - public: - using Value = std::variant; - - static FontSize points(Decimal value) { return FontSize(Value{value}); } - static FontSize css(CSSFontSize value) { return FontSize(Value{value}); } - static Result parse(std::string_view text); - - bool isPoints() const noexcept { return std::holds_alternative(value_); } - bool isCSS() const noexcept { return std::holds_alternative(value_); } - - const Value &value() const noexcept { return value_; } - std::string toString() const; - - private: - explicit FontSize(Value value) : value_(std::move(value)) {} - - Value value_; -}; - -} // namespace mx::core -``` - -```cpp -Result FontSize::parse(std::string_view text) -{ - if (auto decimal = Decimal::parse(text)) - { - return FontSize::points(decimal.value()); - } - if (auto css = CSSFontSize::parse(text)) - { - return FontSize::css(css.value()); - } - return Error{ErrorCode::invalidValue, {}, "invalid font-size"}; -} - -std::string FontSize::toString() const -{ - return std::visit( - [](const auto &member) { return member.toString(); }, - value_); -} -``` - -Open string unions, such as `instrument-sound` after the sounds companion fold, should put the open -string alternative last in parsing, matching the Plates rule. - -## Complex type shapes - -### Pure sequence: fields, not a child vector - -`pitch` is a sequence: `step`, optional `alter`, `octave`. It should not become an ordered vector of -three alternatives. Generate the shape the schema says. - -```cpp -#pragma once - -#include "mx/core/Octave.h" -#include "mx/core/Result.h" -#include "mx/core/Semitones.h" -#include "mx/core/Step.h" - -#include - -namespace mx::core -{ - -class Pitch final -{ - public: - Pitch(Step step, Octave octave) - : step_(step), octave_(octave) - { - } - - static Result make(Step step, std::optional alter, Octave octave); - static Result parseXml(const ezxml::XElement &element); - - const Step &step() const noexcept { return step_; } - const std::optional &alter() const noexcept { return alter_; } - const Octave &octave() const noexcept { return octave_; } - - void setStep(Step value) noexcept { step_ = value; } - void setAlter(std::optional value) { alter_ = std::move(value); } - void setOctave(Octave value) noexcept { octave_ = value; } - - void writeXml(ezxml::XElement &parent, std::string_view tag) const; - - private: - Step step_; - std::optional alter_; - Octave octave_; -}; - -} // namespace mx::core -``` - -A strict parser for `pitch` should be a small generated state machine: - -```cpp -Result Pitch::parseXml(const ezxml::XElement &element) -{ - std::optional step; - std::optional alter; - std::optional octave; - - enum class State - { - start, - afterStep, - afterAlter, - afterOctave, - }; - - State state = State::start; - - for (auto it = element.begin(); it != element.end(); ++it) - { - const auto child = *it; - const auto name = child->getName(); - - if (name == "step" && state == State::start) - { - auto parsed = Step::parse(child->getValue()); - if (!parsed) - { - return parsed.error(); - } - step = parsed.value(); - state = State::afterStep; - } - else if (name == "alter" && state == State::afterStep) - { - auto parsed = Semitones::parse(child->getValue()); - if (!parsed) - { - return parsed.error(); - } - alter = parsed.value(); - state = State::afterAlter; - } - else if (name == "octave" && (state == State::afterStep || state == State::afterAlter)) - { - auto parsed = Octave::parse(child->getValue()); - if (!parsed) - { - return parsed.error(); - } - octave = parsed.value(); - state = State::afterOctave; - } - else - { - return Error{ErrorCode::wrongElementOrder, {}, "invalid child in pitch"}; - } - } - - if (!step) - { - return Error{ErrorCode::missingRequiredElement, {}, "pitch is missing step"}; - } - if (!octave) - { - return Error{ErrorCode::missingRequiredElement, {}, "pitch is missing octave"}; - } - - return Pitch::make(step.value(), alter, octave.value()); -} -``` - -This is the key difference from Go/C: C++ should consume the content tree and enforce sequence -order. - -### Repeated homogeneous members: private vector - -For repeated elements like `beam` in `note`, use private vectors and bounded append APIs. - -```cpp -class Note final -{ - public: - std::span beams() const noexcept { return beams_; } - - Result addBeam(Beam beam) - { - if (beams_.size() == 8) - { - return Error{ErrorCode::tooManyElements, {}, "note has too many beam elements"}; - } - beams_.push_back(std::move(beam)); - return {}; - } - - private: - std::vector beams_; -}; -``` - -If `maxOccurs` is unbounded, `addX` cannot fail for cardinality. If `minOccurs` is at least one, the -constructor or factory takes the first value and the vector never exposes `clear`. - -### Repeated heterogeneous choices: boxed variants - -For measure music-data, order is semantically important and alternatives have very different sizes. -Do not use `std::variant` directly in a vector: every vector slot would -be as large as the largest alternative. Use `std::variant, ...>` and keep the -container private. - -Rendered `partwise-measure` should look like this: - -```cpp -#pragma once - -#include "mx/core/Attributes.h" -#include "mx/core/Backup.h" -#include "mx/core/Barline.h" -#include "mx/core/Bookmark.h" -#include "mx/core/Direction.h" -#include "mx/core/FiguredBass.h" -#include "mx/core/Forward.h" -#include "mx/core/Grouping.h" -#include "mx/core/Harmony.h" -#include "mx/core/Link.h" -#include "mx/core/MeasureText.h" -#include "mx/core/Note.h" -#include "mx/core/Print.h" -#include "mx/core/Result.h" -#include "mx/core/Sound.h" -#include "mx/core/Tenths.h" -#include "mx/core/YesNo.h" - -#include -#include -#include -#include -#include -#include - -namespace mx::core -{ - -class PartwiseMeasure final -{ - public: - using MusicData = std::variant< - std::unique_ptr, - std::unique_ptr, - std::unique_ptr, - std::unique_ptr, - std::unique_ptr, - std::unique_ptr, - std::unique_ptr, - std::unique_ptr, - std::unique_ptr, - std::unique_ptr, - std::unique_ptr, - std::unique_ptr, - std::unique_ptr>; - - explicit PartwiseMeasure(std::string number) - : number_(std::move(number)) - { - } - - PartwiseMeasure(const PartwiseMeasure &other); - PartwiseMeasure &operator=(const PartwiseMeasure &other); - PartwiseMeasure(PartwiseMeasure &&) noexcept = default; - PartwiseMeasure &operator=(PartwiseMeasure &&) noexcept = default; - - const std::string &number() const noexcept { return number_; } - void setNumber(std::string value) { number_ = std::move(value); } - - const std::optional &text() const noexcept { return text_; } - void setText(std::optional value) { text_ = std::move(value); } - - std::span musicData() const noexcept { return music_data_; } - - void appendNote(Note note) { music_data_.push_back(std::make_unique(std::move(note))); } - void appendBackup(Backup backup) { music_data_.push_back(std::make_unique(std::move(backup))); } - void appendForward(Forward forward) { music_data_.push_back(std::make_unique(std::move(forward))); } - void appendDirection(Direction direction) { music_data_.push_back(std::make_unique(std::move(direction))); } - void appendAttributes(Attributes attributes) { music_data_.push_back(std::make_unique(std::move(attributes))); } - void appendHarmony(Harmony harmony) { music_data_.push_back(std::make_unique(std::move(harmony))); } - void appendFiguredBass(FiguredBass figuredBass) { music_data_.push_back(std::make_unique(std::move(figuredBass))); } - void appendPrint(Print print) { music_data_.push_back(std::make_unique(std::move(print))); } - void appendSound(Sound sound) { music_data_.push_back(std::make_unique(std::move(sound))); } - void appendBarline(Barline barline) { music_data_.push_back(std::make_unique(std::move(barline))); } - void appendGrouping(Grouping grouping) { music_data_.push_back(std::make_unique(std::move(grouping))); } - void appendLink(Link link) { music_data_.push_back(std::make_unique(std::move(link))); } - void appendBookmark(Bookmark bookmark) { music_data_.push_back(std::make_unique(std::move(bookmark))); } - - static Result parseXml(const ezxml::XElement &element); - void writeXml(ezxml::XElement &parent, std::string_view tag) const; - - private: - std::string number_; - std::optional text_; - std::optional implicit_; - std::optional non_controlling_; - std::optional width_; - std::optional id_; - std::vector music_data_; -}; - -} // namespace mx::core -``` - -The copy constructor clones the pointed alternatives. The public API still feels value-like, while -each child slot remains small. - -Serialization dispatch is straightforward and safe: - -```cpp -void PartwiseMeasure::writeXml(ezxml::XElement &parent, std::string_view tag) const -{ - auto element = parent.appendChild(std::string(tag)); - element->appendAttribute("number")->setValue(number_); - - if (text_) - { - element->appendAttribute("text")->setValue(text_->toString()); - } - - for (const auto &item : music_data_) - { - std::visit( - [&](const auto &ptr) - { - using Pointer = std::decay_t; - if constexpr (std::is_same_v>) - { - ptr->writeXml(*element, "note"); - } - else if constexpr (std::is_same_v>) - { - ptr->writeXml(*element, "backup"); - } - else if constexpr (std::is_same_v>) - { - ptr->writeXml(*element, "forward"); - } - else if constexpr (std::is_same_v>) - { - ptr->writeXml(*element, "direction"); - } - }, - item); - } -} -``` - -The rendered file would include all alternatives; the snippet is shortened only after `Direction`. -The shape is the important decision: boxed variant, private vector, typed append methods. - -### Non-repeated choice: by-value variant - -For one-of content that occurs once, prefer by-value alternatives. Example shape for the -`pitch | unpitched | rest` part of `note`: - -```cpp -class Note final -{ - public: - using FullNote = std::variant; - - static Result make(FullNote fullNote, std::optional duration); - - const FullNote &fullNote() const noexcept { return full_note_; } - void setFullNote(FullNote value) { full_note_ = std::move(value); } - - private: - FullNote full_note_; - std::optional duration_; -}; -``` - -This is compact and enforces exactly one full-note alternative. - -### Attributes - -Attributes should be separate from child content. Required attributes are stored by value. Optional, -defaulted, and fixed attributes store presence separately from effective value. - -For defaulted attributes, preserve wire absence but provide an effective getter: - -```cpp -class Barline final -{ - public: - RightLeftMiddle location() const noexcept - { - return location_.value_or(RightLeftMiddle::right()); - } - - bool hasLocationAttribute() const noexcept { return location_.has_value(); } - void setLocationAttribute(RightLeftMiddle value) { location_ = value; } - void clearLocationAttribute() noexcept { location_.reset(); } - - private: - std::optional location_; -}; -``` - -For fixed attributes, do not expose a setter that accepts arbitrary values. Expose presence only: - -```cpp -class Link final -{ - public: - bool hasXlinkTypeAttribute() const noexcept { return has_xlink_type_; } - void writeXlinkTypeAttribute() noexcept { has_xlink_type_ = true; } - void omitXlinkTypeAttribute() noexcept { has_xlink_type_ = false; } - - private: - bool has_xlink_type_ = false; -}; -``` - -The serializer writes the fixed value when the presence flag is true. - -### Empty elements - -Presence-only elements should not become empty structs in parent APIs. In a sequence field, use a -specific marker type only when the element itself needs a type in a `std::variant`. - -```cpp -class Empty final -{ - public: - static constexpr Empty present() noexcept { return Empty(); } - - private: - constexpr Empty() noexcept = default; -}; -``` - -An optional presence-only child in a pure sequence can be `std::optional`. A presence-only -choice alternative can be `Empty` in a variant. - -### Schema derivation - -MusicXML derivation is small and attribute-only. Use non-polymorphic public inheritance only for IR -`derived` types. Do not introduce a virtual base class for all elements. - -```cpp -class EmptyTrillSound -{ - public: - const std::optional &type() const noexcept { return type_; } - void setType(std::optional value) { type_ = std::move(value); } - - private: - std::optional type_; - std::optional accelerate_; - std::optional beats_; -}; - -class Mordent final : public EmptyTrillSound -{ - public: - const std::optional &placement() const noexcept { return placement_; } - void setPlacement(std::optional value) { placement_ = std::move(value); } - - private: - std::optional placement_; -}; -``` - -No virtual destructor is needed because this is not a polymorphic hierarchy and the API should not -traffic in owning base pointers. - -## Document root - -The document has exactly one root. Use a root variant. - -```cpp -#pragma once - -#include "mx/core/Result.h" -#include "mx/core/ScorePartwise.h" -#include "mx/core/ScoreTimewise.h" - -#include -#include -#include - -namespace mx::core -{ - -inline constexpr std::string_view SupportedMusicXMLVersion = "4.0"; -inline constexpr std::string_view DoctypeValueScorePartwise = - "-//Recordare//DTD MusicXML 4.0 Partwise//EN"; -inline constexpr std::string_view DoctypeValueScoreTimewise = - "-//Recordare//DTD MusicXML 4.0 Timewise//EN"; - -struct ExtraAttribute -{ - std::string name; - std::string value; -}; - -class Document final -{ - public: - using Root = std::variant; - - explicit Document(ScorePartwise score) : root_(std::move(score)) {} - explicit Document(ScoreTimewise score) : root_(std::move(score)) {} - - static Result fromXDoc(const ezxml::XDoc &doc); - - bool isPartwise() const noexcept { return std::holds_alternative(root_); } - bool isTimewise() const noexcept { return std::holds_alternative(root_); } - - const Root &root() const noexcept { return root_; } - - const std::vector &rootNamespaces() const noexcept { return root_namespaces_; } - void preserveRootNamespace(ExtraAttribute attr) { root_namespaces_.push_back(std::move(attr)); } - - void toXDoc(ezxml::XDoc &out) const; - - private: - Root root_; - std::vector root_namespaces_; -}; - -} // namespace mx::core -``` - -The current corert harness calls `makeDocument()->fromXDoc(...)`. Keep that as a compatibility -adapter, not as the model itself: - -```cpp -namespace mx::core -{ - -class DocumentIo final -{ - public: - bool fromXDoc(std::ostream &errors, const ezxml::XDoc &doc) - { - auto parsed = Document::fromXDoc(doc); - if (!parsed) - { - errors << parsed.error().message; - return false; - } - document_ = std::move(parsed.value()); - return true; - } - - void toXDoc(ezxml::XDoc &out) const - { - document_.value().toXDoc(out); - } - - private: - std::optional document_; -}; - -using DocumentPtr = std::unique_ptr; - -inline DocumentPtr makeDocument() -{ - return std::make_unique(); -} - -} // namespace mx::core -``` - -`DocumentIo` may be empty while parsing; `Document` never is. That keeps legacy harness shape from -infecting the product model. - -## Parsing policy - -Use two distinct policies: - -1. **Structure is strict.** Unknown elements, unknown attributes, wrong order, missing required - content, and too many bounded elements are errors. -2. **Numeric import may clamp.** Existing mx behavior and fixup sidecars expect out-of-range numeric - values to be clamped into range. That is allowed because the resulting model is valid. - -Do not silently repair these: - -- unknown enum literals; -- failed string patterns; -- missing required children or attributes; -- choice branches with multiple alternatives; -- newer MusicXML root versions. - -Version gating should happen at document parse before dispatching deep into the model. A document -declaring a newer version should return `unsupportedVersion`, letting the corert harness skip it. - -## Serialization policy - -Serialization should be total for every valid model object. - -Rules: - -- sequence-shaped classes emit in schema order; -- repeated choice vectors emit in vector order; -- optional attributes emit only when present; -- defaulted attributes use effective getters for API reads but preserve absence on write; -- fixed attributes emit their fixed literal only when their presence flag is set; -- root namespace declarations captured during parse are written back on the root; -- XML declaration and doctype normalization remain in the corert normalization layer, not in every - model serializer. - -## Template strategy - -The C++ templates should consume more of the Plates than Go/C did: - -- value templates consume `EnumPlate`, `NumberPlate`, `StringPlate`, and `UnionPlate` as Go/C do; -- complex templates must consume `ComplexPlate.content` for child structure; -- attributes still come from `attributes` / `merged_attributes`; -- schema derivation uses `strategy = inherit` from the C++ config default; -- repeated heterogeneous choices render boxed variants; -- pure sequences render named fields; -- repeated homogeneous elements render private vectors and append APIs. - -This must still be done without adding C++ branches to the generator. If a needed datum is missing, -first ask whether it is a neutral schema fact. If yes, it belongs in the Plates for all targets. If -it is C++ spelling, it belongs in `gen/cpp/config.toml` or `gen/cpp/templates/`. - -Expected C++ target manifest shape: - -```toml -[render] -dir = "templates" -format = ["clang-format", "-i", "{dir}/**/*.{h,cpp}"] - -[[render.type]] -strategies = ["enum-class"] -template = "enum.h.tmpl" -output = "{pascal}.h" - -[[render.type]] -strategies = ["enum-class"] -template = "enum.cpp.tmpl" -output = "{pascal}.cpp" - -[[render.type]] -strategies = ["numeric-wrapper"] -template = "number.h.tmpl" -output = "{pascal}.h" - -[[render.type]] -strategies = ["numeric-wrapper"] -template = "number.cpp.tmpl" -output = "{pascal}.cpp" - -[[render.type]] -strategies = ["string-wrapper"] -template = "string.h.tmpl" -output = "{pascal}.h" - -[[render.type]] -strategies = ["string-wrapper"] -template = "string.cpp.tmpl" -output = "{pascal}.cpp" - -[[render.type]] -strategies = ["tagged-variant"] -template = "union.h.tmpl" -output = "{pascal}.h" - -[[render.type]] -strategies = ["tagged-variant"] -template = "union.cpp.tmpl" -output = "{pascal}.cpp" - -[[render.type]] -strategies = ["value-class", "composite-class", "attrs-class", "flag", "inherit"] -template = "complex.h.tmpl" -output = "{pascal}.h" - -[[render.type]] -strategies = ["value-class", "composite-class", "attrs-class", "flag", "inherit"] -template = "complex.cpp.tmpl" -output = "{pascal}.cpp" - -[[render.once]] -template = "Document.h.tmpl" -output = "Document.h" - -[[render.once]] -template = "Document.cpp.tmpl" -output = "Document.cpp" - -[[render.once]] -template = "runtime.h.tmpl" -output = "Result.h" -``` - -The exact filenames can change, but source/header split should not. - -## Rejected designs - -### Raw public structs - -Rejected. This copies the Go/C harness shape and leaks invariants to every caller. A public -`std::vector` or public `std::optional` for required content lets users construct invalid MusicXML. - -### One `std::vector>` for every complex type - -Rejected as the universal representation. It preserves order but erases sequence structure and makes -simple types like `Pitch` unnecessarily dynamic. Use it only for repeated heterogeneous choices. - -### Nullable child structs - -Rejected. `struct NoteChild { Pitch *pitch; Rest *rest; ... }` has invalid states: none set or many -set. C++ has `std::variant`; use it. - -### A virtual `Element` base class - -Rejected. It adds heap allocation and vtables everywhere, makes ownership harder, and hides schema -facts behind runtime checks. MusicXML's complex graph is a DAG; most members can be by value. - -### `std::shared_ptr` everywhere - -Rejected. Shared ownership should be a rare explicit decision, not the model's default ownership -story. Use values for ordinary members and `std::unique_ptr` only to keep repeated heterogeneous -choice nodes small. - -### Strict validation only as a separate pass - -Rejected. A separate `validate()` pass means invalid documents can exist in memory. The product API -should make validity the normal state and use builders/parsers as temporary staging areas. - -## Implementation order - -1. Add the C++ target templates and render manifest without touching generator Python. -2. Generate value types first: enum, number, string, union; compile a values smoke test. -3. Generate simple complex types with attributes and value content. -4. Generate pure sequences such as `Pitch`. -5. Generate repeated choice content such as `PartwiseMeasure`. -6. Generate `Document` and version-gated root dispatch. -7. Wire corert through the compatibility `DocumentIo` adapter. -8. Mark or fix corpus files that are structurally invalid if strict parsing exposes them. -9. Keep Go and C green to prove the target did not contaminate the generator. - -The hard part is not C++ syntax. The hard part is preserving the architecture: C++ must be just -another target while still being a much better product API than the proof targets. diff --git a/docs/ai/design/mx-core-opus-4.7.md b/docs/ai/design/mx-core-opus-4.7.md deleted file mode 100644 index 44655c28f..000000000 --- a/docs/ai/design/mx-core-opus-4.7.md +++ /dev/null @@ -1,1208 +0,0 @@ -# mx::core: the C++ generated typed model - -Status: design proposal. The C++ target's `gen/cpp/config.toml` exists; its `templates/` directory -does not. This document specifies the rendered C++ shapes the templates must produce — every -construct chosen with a reason a reviewer can argue with. It is a sibling to `plates.md` and -`generator-agnosticism.md`: those documents describe the engine; this one describes the artifact -the engine prints for the C++ edition. - -It is intentionally non-template. Every snippet below is what the **generator emits**, not what a -template looks like; the templates are the renderer's problem and live under -`gen/cpp/templates/` once these shapes are agreed. - -The reader is assumed to have read `plates.md` (the eight shapes, the per-target binding) and -`gen/README.md` (the IR, the resolver, the deps-first DAG, the no-collision invariant). Cross- -references in this document use the IR/Plates vocabulary verbatim. - ---- - -## 1. Decisions in one page - -The design hangs off five non-negotiables. Everything in §3-§13 derives from these. - -1. **The complex-type graph is a DAG with no cycles** (`gen/README.md` §"Two load-bearing - invariants"). No forward declarations. No `shared_ptr`. No heap. **Plain by-value composition - throughout.** A `Note` *contains* its children, it does not point at them. - -2. **The schema's `name -> type` map is 1:1.** Parse dispatch is a static name table; we never - need context-sensitive resolution. Serialization writes a known tag for each member. - -3. **The data classes are dumb structs.** Public fields, aggregate-initializable, no invariants - beyond what `enum class` and the value-type wrappers already give us. Validation lives in the - value-type wrappers (clamp on assignment) and in parse (lenient values, strict names — the - plates round-3 contract). Assigning a value of a typed field is by definition valid; that is - the whole point of generating typed wrappers. - -4. **Document order is preserved by structure, not by a separate sequence number.** Pure-sequence - composites use plain ordered fields. Choice-bearing or repeating content uses - `std::vector>` so order is the array order and "exactly one alternative" is - true by construction. **C++ does NOT copy the Go/C ordered-children-of-pointers encoding** — - that pattern exists because Go and C have no sum type. We do; we use it. (`plates.md` §11 - round 3 is explicit about this.) - -5. **Parse and serialize are free functions over `ezxml::XElement`, not member functions.** The - typed model is one concern (data); XML I/O is another (interop). Keeping them apart lets the - model classes be aggregates and lets a future binding (JSON, MessagePack) reuse the same - classes by writing its own free functions. - -The standard is **C++17** (matches the existing `src/private` codebase: `std::optional`, -`std::variant`, `std::filesystem`). No C++20-only features are required. - ---- - -## 2. File and namespace layout - -One type per file, per the existing `[layout] partition = "per-type"` convention (matches Go's and -C's per-type layout, matches what the existing `src/private/mx/core/` directory was sized for, and -gives a 1:1 mapping between IR types and headers — the smallest possible blast radius for a schema -change). Header and implementation are split: - -``` -src/private/mx/core/ - Decimal.h <- hand-written runtime - Decimal.cpp - Runtime.h <- hand-written: SupportedMusicXMLVersion, parse helpers - Runtime.cpp - AboveBelow.h AboveBelow.cpp <- generated, value: enum - Divisions.h Divisions.cpp <- generated, value: number - FontSize.h FontSize.cpp <- generated, value: union - Empty.h Empty.cpp <- generated, complex: empty (presence-only is a - bool typedef; this is for attrs-only emptys) - AccidentalText.h AccidentalText.cpp <- generated, complex: value-bodied - Note.h Note.cpp <- generated, complex: composite (choice content) - Pitch.h Pitch.cpp <- generated, complex: composite (pure sequence) - Mordent.h Mordent.cpp <- generated, complex: derived (inheritance) - Document.h Document.cpp <- generated, document root + extra-attr struct - ... - sources.cmake <- generated, lists all .cpp files for CMake -``` - -File stem is the type's PascalCase identifier (`Plates.Variant.cased["pascal"]` for the type name) -to match the existing C++ convention in this repo (`XElement.h`, `CoreRoundtripImpl.cpp`). - -All generated symbols live in `namespace mx::core`. The C++ Decimal wrapper, the runtime helpers, -and the `SupportedMusicXMLVersion` constant live in the same namespace; the generator does not -produce a separate `mx::core::detail` (no symbol needs to be hidden — every emitted name is part -of the public API of the typed model). - -```cpp -// AboveBelow.h -#pragma once - -namespace mx::core -{ - -// (doc comment ...) -enum class AboveBelow -{ - Above, - Below, -}; - -} // namespace mx::core -``` - -Headers use `#pragma once` (already the convention in `src/private/mx/ezxml/`). No include guards -of the `MX_..._H_INCLUDED` form: pragma once is universally supported by the compilers this -project targets, and matches what is already in the tree. - -Each header `#include`s only the headers it textually needs. The Plates already compute the per- -file `deps`, so the include list is data-driven and stable. The DAG invariant means cycles are -impossible, which is why this works without forward declarations. - ---- - -## 3. Value: enum class - -One file per enum. The wire-literal table and the parse map are emitted into the `.cpp` so the -header is light. The generator's `[target] variant-scope = "bare"` setting (already in -`gen/cpp/config.toml`) means the variant identifier is the bare PascalCase name, scoped by the -`enum class`. - -```cpp -// AboveBelow.h -#pragma once - -namespace mx::core -{ - -/// The above-below type is used to indicate whether one element appears above -/// or below another element. -enum class AboveBelow -{ - Above, - Below, -}; - -/// Returns the wire literal for `v` (static storage, never null). -const char* toString(AboveBelow v); - -/// Strict parse: returns true and writes `*out` only when `s` exactly matches -/// a wire literal. -bool tryParse(const char* s, AboveBelow* out); - -/// Lenient parse: an unrecognized input falls back to the first variant -/// (mirrors the Go/C contract; the corert harnesses depend on this). -AboveBelow parseAboveBelow(const char* s); - -} // namespace mx::core -``` - -```cpp -// AboveBelow.cpp -#include "mx/core/AboveBelow.h" - -#include - -namespace mx::core -{ - -namespace -{ -constexpr const char* kAboveBelowWire[] = { - "above", - "below", -}; -} // namespace - -const char* toString(AboveBelow v) -{ - const auto i = static_cast(v); - if (i >= sizeof(kAboveBelowWire) / sizeof(kAboveBelowWire[0])) - { - return kAboveBelowWire[0]; - } - return kAboveBelowWire[i]; -} - -bool tryParse(const char* s, AboveBelow* out) -{ - if (s == nullptr) { return false; } - const std::string_view sv{s}; - if (sv == "above") { *out = AboveBelow::Above; return true; } - if (sv == "below") { *out = AboveBelow::Below; return true; } - return false; -} - -AboveBelow parseAboveBelow(const char* s) -{ - AboveBelow v; - if (tryParse(s, &v)) { return v; } - return AboveBelow::Above; -} - -} // namespace mx::core -``` - -**Why a `switch`/`if`-chain rather than a `std::unordered_map`?** The -chain is generated, deterministic, branch-predictor-friendly for the common case, and zero-init -at startup; an unordered_map costs a heap allocation per enum and an unordered hash lookup per -parse. With ~96 enums in MusicXML 4.0 the difference is real and free for the generator to -arrange. The chain is the same shape Go and C use. - -**Why overloaded `toString` and `tryParse` (no name composition like `toStringAboveBelow`)?** ADL -plus C++ overload resolution makes `toString(myEnum)` work without the caller naming the type. Go -and C compose names because they have no overloading. We do; we use it. (Free functions, not -member functions, because `enum class` cannot have member functions.) - -**`parseAboveBelow` is composed.** The lenient-fallback parse cannot be overloaded (no input enum -to dispatch on); composition is forced by the language, which is exactly the Variant.cased -[pascal] composed-with-`parse` rule the plates already implement. - ---- - -## 4. Value: number wrapper - -A `class` wrapping the IR primitive's target type, with clamp-on-construct using the plate's -`NumberPlate.clamp` steps (resolved facets + primitive-implied bounds, computed once in the -plates per round 2 of `plates.md` §11). The inner representation is whatever -`gen/cpp/config.toml` `[types]` maps the IR primitive to. - -`Decimal` is `mx::core`'s own hand-written class (lossless decimal-as-text + a numeric face for -arithmetic; the existing corert harness has trailing-zero rules that depend on the lossless -representation, see `DecimalFields.h`). The generator does not invent it; `[types] decimal = -"Decimal"` says "use the existing class," and `Divisions` below wraps it. - -```cpp -// Divisions.h -#pragma once - -#include "mx/core/Decimal.h" - -namespace mx::core -{ - -/// The divisions type is used to express values in terms of the musical -/// divisions defined by the divisions element. It is preferred that these be -/// integer values both for MIDI interoperability and to avoid roundoff -/// errors. -class Divisions -{ -public: - Divisions() = default; - explicit Divisions(Decimal value); // clamp on assignment - explicit Divisions(double value); // convenience - Decimal getValue() const; - void setValue(Decimal value); // clamp on assignment - bool operator==(const Divisions& other) const; - bool operator!=(const Divisions& other) const; - -private: - Decimal m_value{}; -}; - -const char* toString(const Divisions& v, std::string& buf); // see §14 -bool tryParse(const char* s, Divisions* out); -Divisions parseDivisions(const char* s); - -} // namespace mx::core -``` - -For a number with bounds (`positive-decimal`, `tenths`, `midi-channel` with `[1, 16]`, etc.), the -constructor and setter call a generated `clamp` free helper that runs the plate's clamp steps. -The clamp steps are *data on the plate* per round 2; the template emits one branch per step. - -```cpp -// Tenths.h -#pragma once - -#include "mx/core/Decimal.h" - -namespace mx::core -{ - -class Tenths -{ -public: - Tenths() = default; - explicit Tenths(Decimal value); - Decimal getValue() const; - void setValue(Decimal value); - // ... operator== etc. -private: - Decimal m_value{}; -}; -// (parse / toString / tryParse free functions as above) - -} // namespace mx::core -``` - -```cpp -// MidiChannel.cpp (illustrative: integer with min=1, max=16) -#include "mx/core/MidiChannel.h" - -namespace mx::core -{ - -namespace -{ -int clamp(int v) { if (v < 1) { return 1; } if (v > 16) { return 16; } return v; } -} // namespace - -MidiChannel::MidiChannel(int value) : m_value{clamp(value)} {} -void MidiChannel::setValue(int value) { m_value = clamp(value); } -int MidiChannel::getValue() const { return m_value; } - -} // namespace mx::core -``` - -**Why a class with private storage rather than a `using Tenths = Decimal;`?** Type identity. With -strong typedefs, `void f(Tenths)` cannot be silently called with a `Decimal` or a `Divisions`; -that's the whole point of generating ~25 distinct numeric types from the schema. A `using` alias -gives every numeric type the same C++ identity, which throws away the schema's information at the -language boundary. - -**Why explicit constructors?** Same reason: prevent silent conversions from `int` or `double` -landing in the wrong wrapper. A user who wants to construct a `Divisions{4}` must say so. - -**Why a getter/setter pair, not a public `value` field?** Because the setter clamps. A public -field would let the invariant escape. This is the one place in the generated code where a class -has private state; it earns it. - ---- - -## 5. Value: string wrapper - -A class over `std::string`, optionally enforcing a regex pattern and length bounds. Most string -types in MusicXML are unconstrained tokens, so the wrapper is mostly identity; the few with -patterns (`color`, `time-only`, `comma-separated-text`) get the check on `setValue`. - -```cpp -// Color.h -#pragma once - -#include - -namespace mx::core -{ - -/// The color type indicates the color of an element. Color may be represented -/// as hexadecimal RGB triples or as hexadecimal ARGB tuples [...] -class Color -{ -public: - Color() = default; - explicit Color(std::string value); - const std::string& getValue() const; - void setValue(std::string value); - bool operator==(const Color& other) const; - bool operator!=(const Color& other) const; -private: - std::string m_value; -}; - -void toString(const Color& v, std::string& out); -bool tryParse(const char* s, Color* out); // pattern-checked -Color parseColor(const char* s); // lenient: accepts on parse failure - -} // namespace mx::core -``` - -For the unconstrained string aliases (the majority), the wrapper is the same shape but `tryParse` -always succeeds; the class is generated for type-identity reasons (so `void f(CommaSeparatedText)` -will not silently take a `Color`), not for validation. Identical rationale to numeric wrappers. - ---- - -## 6. Value: union (tagged variant) - -`std::variant` over the member types. This is the C++17 sum type; using anything else here would -be reinvention. The plate already carries `UnionPlateMember.tag` (per round 2 of `plates.md` -§11), so each alternative has a stable, projected, collision-checked discriminator. - -```cpp -// FontSize.h -#pragma once - -#include "mx/core/CssFontSize.h" -#include "mx/core/Decimal.h" - -#include - -namespace mx::core -{ - -/// The font-size can be one of the CSS font sizes or a numeric point size. -class FontSize -{ -public: - enum class Kind { Decimal, CssFontSize }; - - FontSize(); // default: first member, default-constructed - explicit FontSize(Decimal v); - explicit FontSize(CssFontSize v); - - Kind getKind() const; - - // Each accessor is valid only when getKind() returns the matching kind. - // std::get throws std::bad_variant_access on mismatch. - Decimal getDecimal() const; - void setDecimal(Decimal v); - CssFontSize getCssFontSize() const; - void setCssFontSize(CssFontSize v); - - bool operator==(const FontSize& other) const; - bool operator!=(const FontSize& other) const; - -private: - std::variant m_value; -}; - -void toString(const FontSize& v, std::string& out); -bool tryParse(const char* s, FontSize* out); // tries members in schema order -FontSize parseFontSize(const char* s); // lenient: first member absorbs - -} // namespace mx::core -``` - -**Why expose `std::variant` *behind* getter/setter rather than inheriting from it or making -`m_value` public?** Two reasons. First, accessor names reveal the schema's intent ( -`getCssFontSize()` reads at the call site; `std::get(fs.value)` does not). Second, -the `Kind` enum gives the caller a `switch`-friendly discriminator without the `std::holds_ -alternative` boilerplate. The generated code's *interface* is the kind enum + per-member accessors; -the variant is the *implementation*. - -**Why not a hand-rolled tagged union (a struct with a `Kind` and a `union { ... }`)?** Lifetime. -The members can have non-trivial destructors (a `std::string`-backed wrapper). `std::variant` -handles the lifetime; a hand-rolled union would have to do it manually and would be a hand-written -piece of generated code per union, which is reinvention. - -**`tryParse` order matters.** The plates round-2 rule "an open string union member must be last" -keeps `tryParse` from short-circuiting. The C++ template enforces this at render time, identical -to Go/C; otherwise lossy schemas would silently route everything through the open string member. - ---- - -## 7. Complex: empty - -Two sub-cases, both already separated by the plates: - -- **Presence-only** (`presence_only: true`, no attributes): the IR's "the only information is - whether it appears." Emit as `using Empty = bool;` — there is **nothing else worth saying**. - An ordered-children variant alternative whose payload is `Empty` is *visibly* a presence flag. - ```cpp - // Empty.h (a few presence-only types are aliased to bool) - #pragma once - - namespace mx::core - { - - /// Presence-only marker. The element is either present (`true`) or absent. - using Empty = bool; - - } // namespace mx::core - ``` - - An alternative — emitting an empty `class Empty {};` — would compile, but `Empty{}` and `Empty{}` - are indistinguishable at every call site, so the type adds nothing the language doesn't already - give us with `bool`. The `using` is honest about the type's information content. - -- **Attrs-only** (`presence_only: false`, attributes only): a class whose fields are the - attributes (§9 conventions for attribute fields). Same shape as a value-bodied complex type - minus the `value` field. - ---- - -## 8. Complex: value-bodied - -A class whose `value` field is the typed text body and whose other fields are the attributes. -Aggregate; default-constructible; field order = attribute declaration order (then `value` last). - -```cpp -// AccidentalText.h -#pragma once - -#include "mx/core/AccidentalValue.h" -#include "mx/core/Color.h" -#include "mx/core/CommaSeparatedText.h" -#include "mx/core/EnclosureShape.h" -#include "mx/core/FontSize.h" -#include "mx/core/FontStyle.h" -#include "mx/core/FontWeight.h" -#include "mx/core/LeftCenterRight.h" -#include "mx/core/NumberOfLines.h" -#include "mx/core/NumberOrNormal.h" -#include "mx/core/RotationDegrees.h" -#include "mx/core/SmuflAccidentalGlyphName.h" -#include "mx/core/Tenths.h" -#include "mx/core/TextDirection.h" -#include "mx/core/Valign.h" - -#include -#include - -namespace ezxml { class XElement; } - -namespace mx::core -{ - -/// The accidental-text type represents an element with an accidental value -/// and text-formatting attributes. -class AccidentalText -{ -public: - // Optional attributes use std::optional (§13). - std::optional smufl; - std::optional xmlLang; - std::optional xmlSpace; - std::optional justify; - std::optional defaultX; - std::optional defaultY; - std::optional relativeX; - std::optional relativeY; - std::optional fontFamily; - std::optional fontStyle; - std::optional fontSize; - std::optional fontWeight; - std::optional color; - std::optional halign; - std::optional valign; - std::optional underline; - std::optional overline; - std::optional lineThrough; - std::optional rotation; - std::optional letterSpacing; - std::optional lineHeight; - std::optional dir; - std::optional enclosure; - - // Required: by-value, default-constructible. - AccidentalValue value{}; -}; - -bool operator==(const AccidentalText& a, const AccidentalText& b); -bool operator!=(const AccidentalText& a, const AccidentalText& b); - -void parseAccidentalText(const ezxml::XElement& el, AccidentalText& out); -void serializeAccidentalText(const AccidentalText& v, ezxml::XElement& parent, - const std::string& tag); - -} // namespace mx::core -``` - -**Public fields, not getters/setters.** The class has no invariant beyond what each *member*'s -type already enforces (`Tenths` clamps on assignment, `EnclosureShape` is an enum class). -Wrapping a getter around an `std::optional` would add ceremony and zero safety. - -**`std::optional` for optional members.** Standard, value-typed (no allocation), trivially -distinguishable from "present with default value" — the case `defaultX = Tenths{0}` is genuinely -different from `defaultX = std::nullopt`, and the wire format reflects it. Go uses pointers -because Go has no `optional`; C uses a `bool has_x` companion because C has no generics. We -have `std::optional`; we use it. - -**`bool operator==` is generated.** Aggregates are not equality-comparable by default in C++17 -(C++20's `=default` would change this). The generator emits the per-field comparison. The corert -harness does not need it (it diffs at the XML level), but the API shape is dramatically more -useful for everyone else (unit tests, custom user code, hashing later). - -**Required fields are by-value.** No `optional` wrapper, no pointer. The DAG invariant means -this is always safe; default construction gives the type's natural zero (a `Tenths{0}`, an -`AccidentalValue::Sharp`). - ---- - -## 9. Complex: composite (the crucial case) - -Composites are the hard one and the place the C++ design diverges from Go and C. A composite has -attributes plus a content tree (`Resolver.content`, group-spliced). Two flavors: - -**9a. Pure-sequence composite, every member exactly once (or optional once), no choices, no -repeats.** Plain ordered fields. `Pitch` (step, alter?, octave) is the canonical case. - -```cpp -// Pitch.h -#pragma once - -#include "mx/core/Octave.h" -#include "mx/core/Semitones.h" -#include "mx/core/Step.h" - -#include - -namespace ezxml { class XElement; } - -namespace mx::core -{ - -/// Pitch is represented as a combination of the step of the diatonic scale, -/// the chromatic alteration, and the octave. -class Pitch -{ -public: - // No attributes on . - Step step{}; - std::optional alter; - Octave octave{}; -}; - -bool operator==(const Pitch& a, const Pitch& b); -bool operator!=(const Pitch& a, const Pitch& b); - -void parsePitch(const ezxml::XElement& el, Pitch& out); -void serializePitch(const Pitch& v, ezxml::XElement& parent, const std::string& tag); - -} // namespace mx::core -``` - -The field order in the class *is* the document order; serialize walks the fields in declaration -order. Parse loops over child elements and dispatches by tag name onto the matching field (ignoring -order, per round-3's "lenient about structure" rule). - -**9b. Choice-bearing or repeating content.** Attributes are still ordinary fields; **child -elements live in one ordered `std::vector` of a per-type `std::variant`.** - -```cpp -// Note.h -#pragma once - -#include "mx/core/Accidental.h" -#include "mx/core/Beam.h" -#include "mx/core/Color.h" -#include "mx/core/CommaSeparatedText.h" -#include "mx/core/Divisions.h" -#include "mx/core/Empty.h" -#include "mx/core/EmptyPlacement.h" -#include "mx/core/FontSize.h" -#include "mx/core/FontStyle.h" -#include "mx/core/FontWeight.h" -#include "mx/core/FormattedText.h" -#include "mx/core/Grace.h" -#include "mx/core/Instrument.h" -#include "mx/core/Level.h" -#include "mx/core/Lyric.h" -#include "mx/core/Notations.h" -#include "mx/core/NoteType.h" -#include "mx/core/Notehead.h" -#include "mx/core/NoteheadText.h" -#include "mx/core/NonNegativeDecimal.h" -#include "mx/core/Pitch.h" -#include "mx/core/Play.h" -#include "mx/core/PositiveDivisions.h" -#include "mx/core/Rest.h" -#include "mx/core/Stem.h" -#include "mx/core/Tenths.h" -#include "mx/core/Tie.h" -#include "mx/core/TimeModification.h" -#include "mx/core/TimeOnly.h" -#include "mx/core/Unpitched.h" -#include "mx/core/YesNo.h" - -#include -#include -#include -#include - -namespace ezxml { class XElement; } - -namespace mx::core -{ - -/// One child element of : exactly one alternative by construction. -/// Document order is the index in Note::children. -/// -/// The `using` aliases the variant; `std::visit` and the dispatch helpers -/// (§9c) work directly on it. -using NoteChild = std::variant< - Grace, // - Empty, // (presence-only) - Pitch, // - Unpitched, // - Rest, // - Tie, // - Empty, // (presence-only) - PositiveDivisions, // - Instrument, // - FormattedText, // - Level, // - std::string, // - NoteType, // - EmptyPlacement, // - Accidental, // - TimeModification, // - Stem, // - Notehead, // - NoteheadText, // - int, // - Beam, // - Notations, // - Lyric, // - Play // ->; - -/// Notes are the most common type of MusicXML data [...] -class Note -{ -public: - // Attributes — same conventions as §8. - std::optional printLeger; - std::optional dynamics; - std::optional endDynamics; - std::optional attack; - std::optional release; - std::optional timeOnly; - std::optional pizzicato; - std::optional defaultX; - std::optional defaultY; - std::optional relativeX; - std::optional relativeY; - std::optional fontFamily; - std::optional fontStyle; - std::optional fontSize; - std::optional fontWeight; - std::optional color; - std::optional printDot; - std::optional printLyric; - std::optional printObject; - std::optional printSpacing; - std::optional id; - - // Children in document order. Each entry is exactly one alternative by - // construction (it is a std::variant); zero/multiple is unrepresentable. - std::vector children; -}; - -void parseNote(const ezxml::XElement& el, Note& out); -void serializeNote(const Note& v, ezxml::XElement& parent, const std::string& tag); - -} // namespace mx::core -``` - -**Why `std::vector>` and not the Go/C "struct of typed pointers" pattern?** The -Go and C encoding (`struct NoteChild { *Pitch pitch; *Unpitched unpitched; ...; }`) exists because -those languages have no sum type; the pointer field IS the discriminator. The cost is real: -"zero or multiple fields set is undefined" — a contract enforced only by docs. In C++, -`std::variant` makes the same guarantee at compile time. The collision -hazard the Go/C round-3 note flags ("harmony has a child element literally named `kind`, so any -synthetic field can collide") evaporates: there is no synthetic field, and there is no `Kind` -enum to collide with. - -**The variant has duplicate alternatives** (`Empty` appears twice: `` and ``). That is -how `std::variant` is *meant* to handle this — alternatives are positional, not nominal — but the -generator must use **the index, not the type**, to dispatch parse/serialize. The generated -`parseNote` writes `out.children.emplace_back(std::in_place_index<6>, parseEmpty(c))` for `` -to disambiguate the two `Empty`s. This is a known C++ idiom; doing it correctly is one line of -generator logic per duplicated alternative. Worked in the dispatch helper below. - -**The `using ...Child = std::variant<...>` is per type, in the type's own header.** Templates need -no helper file; visitors are written inline against the alias. Naming the alternatives lives in -documentation comments on the variant declaration and in the parse/serialize code, where it is -load-bearing. - -### 9c. Dispatch: parse and serialize for choice content - -The generated parse builds the variant directly from the tag name; the generated serialize -`std::visit`s with one lambda per alternative, indexed (so duplicate types route correctly to -their own tag names). - -```cpp -// Note.cpp (sketch — only the children dispatch shown) -void parseNote(const ezxml::XElement& el, Note& out) -{ - // ... attribute loop (omitted, identical in shape to AccidentalText) ... - - for (auto it = el.begin(); it != el.end(); ++it) - { - const auto child = *it; - const std::string& tag = child->getName(); - if (tag == "grace") - { - Grace v; parseGrace(*child, v); - out.children.emplace_back(std::in_place_index<0>, std::move(v)); - } - else if (tag == "chord") - { - // is presence-only -> Empty (which is bool); presence is true. - out.children.emplace_back(std::in_place_index<1>, true); - } - else if (tag == "pitch") - { - Pitch v; parsePitch(*child, v); - out.children.emplace_back(std::in_place_index<2>, std::move(v)); - } - else if (tag == "cue") - { - // Same Empty type as ; index disambiguates. - out.children.emplace_back(std::in_place_index<6>, true); - } - else if (tag == "voice") - { - out.children.emplace_back(std::in_place_index<11>, child->getValue()); - } - // ... rest of the cases ... - else - { - throw std::runtime_error("unknown element <" + tag + "> in "); - } - } -} - -void serializeNote(const Note& v, ezxml::XElement& parent, const std::string& tag) -{ - auto el = parent.appendChild(tag); - // ... attribute writes (omitted) ... - - for (const auto& c : v.children) - { - switch (c.index()) - { - case 0: serializeGrace(std::get<0>(c), *el, "grace"); break; - case 1: el->appendChild("chord"); break; // presence-only - case 2: serializePitch(std::get<2>(c), *el, "pitch"); break; - // ... down through index 23 ... - case 6: el->appendChild("cue"); break; // presence-only, same Empty type - case 11: { auto* v = el->appendChild("voice"); v->setValue(std::get<11>(c)); break; } - // ... - } - } -} -``` - -**Why a `switch` on `index()` rather than `std::visit`?** Two reasons. (1) The tag name is -*per-alternative*, not per-type, and `std::visit` dispatches on type — duplicate alternatives -(`` and `` both `Empty`) can't be told apart by visit. (2) The `switch` on -`std::variant::index()` is a jump table and is friendlier to the optimizer than the nested- -ternary expansion `std::visit` produces for ~24 alternatives. Both reasons are real; either alone -would be enough. - ---- - -## 10. Complex: derived (real inheritance) - -The `gen/cpp/config.toml` does not yet declare `[target] inheritance` (the C target sets `false`). -The C++ default is `true` — derive types extend their base, exactly mirroring the IR's -`complexContent extension` shape. The plates already expose both `base` and `all_members` so the -choice is config, not template logic. - -```cpp -// Mordent.h -#pragma once - -#include "mx/core/AboveBelow.h" -#include "mx/core/EmptyTrillSound.h" // base class -#include "mx/core/YesNo.h" - -#include - -namespace ezxml { class XElement; } - -namespace mx::core -{ - -/// The mordent type is used for both [...] The long attribute is "no" by -/// default. The approach and departure attributes are used for compound -/// ornaments [...] -class Mordent : public EmptyTrillSound -{ -public: - std::optional mordentLong; // attribute "long" (renamed: "long" is reserved) - std::optional approach; - std::optional departure; -}; - -bool operator==(const Mordent& a, const Mordent& b); -bool operator!=(const Mordent& a, const Mordent& b); - -void parseMordent(const ezxml::XElement& el, Mordent& out); -void serializeMordent(const Mordent& v, ezxml::XElement& parent, const std::string& tag); - -} // namespace mx::core -``` - -**Public inheritance, not private.** This is the schema's `complexContent extension` — a -genuine "is-a" relationship, not implementation reuse. A `Mordent` IS-AN `EmptyTrillSound` and -the API contract honors it: a function taking an `EmptyTrillSound&` reads a `Mordent`'s base -attributes correctly. - -**No virtual functions, no virtual destructor.** These are data classes, not polymorphic ones. -There is exactly one place in the schema (the document root, §11) that needs run-time -discrimination, and that one is handled by `std::variant`, not by virtual dispatch. Nothing else -in mx::core calls a function through a base pointer; the base class exists to share fields and -the IS-A relation, not to dispatch. Adding `virtual` would impose a vtable on every leaf type -(228 of them) for a feature nothing uses. (If a future use case appeared, adding `virtual` to the -base class is a one-edit, base-only change — much smaller blast radius than the inverse.) - -**Slicing is fine here.** Without virtual functions there is no slicing problem: copying a -`Mordent` to an `EmptyTrillSound` simply produces an `EmptyTrillSound` with the right fields, -which is the correct behavior. - -**`mordentLong` for the `long` attribute** is the `[reserved] words = ["long", ...]` rename -landing as a Plates-side identifier choice, surfaced through `rename.attribute.mordent.long = -"mordent-long"` in `gen/cpp/config.toml`. The wire form stays `long`; only the C++ identifier -mangles. This is the existing Plates §6.2 mechanism, used as designed. - -**Parse/serialize delegate to the base.** `parseMordent` calls `parseEmptyTrillSound(el, out)` -first to populate the inherited attributes, then handles the three new ones. `serializeMordent` -calls `serializeEmptyTrillSound`'s body inline (the base's serializer takes a `parent` and a -`tag`, so we cannot reuse it directly without writing the same element twice; the generator -emits the merged attribute list in the derived `.cpp` rather than chaining serialize calls). - ---- - -## 11. The document and its roots - -`Document` is the entry point. Two roots (`score-partwise`, `score-timewise`); exactly one is -present. `std::variant` again. - -```cpp -// Document.h -#pragma once - -#include "mx/core/ScorePartwise.h" -#include "mx/core/ScoreTimewise.h" - -#include -#include -#include - -namespace ezxml { class XDoc; } - -namespace mx::core -{ - -/// An attribute on the document root that is outside the schema; in -/// practice this is the namespace declarations (xmlns, xmlns:xlink). -/// Round-trip parity requires preserving them verbatim. -struct ExtraAttr -{ - std::string key; - std::string value; -}; - -/// A parsed MusicXML document. Exactly one root variant is held. -class Document -{ -public: - using Root = std::variant; - - Root root; - std::vector rootNamespaces; -}; - -/// Parse an ezxml document into the typed model. Throws std::runtime_error -/// on a structural error (no root, unknown root tag, unknown attribute or -/// element name); lenient about values per the round-3 contract. -Document fromXDoc(const ezxml::XDoc& doc); - -/// Serialize the typed model back to an ezxml document. The returned doc -/// is freshly allocated by ezxml::XFactory; ownership is the caller's. -ezxml::XDocPtr toXDoc(const Document& d); - -} // namespace mx::core -``` - -This is the only place in the API where `std::variant` shows up at the document level. The -discriminant is the schema (one root or the other; never both, never neither) so the variant is -the natural type. - -**`fromXDoc` returns by value.** RVO/NRVO and move semantics make this free; a `unique_ptr< -Document>` would be ceremony with no payoff (the caller can put the value in a -`unique_ptr` if they want one). - -**`toXDoc` returns `ezxml::XDocPtr`.** That is ezxml's existing ownership type (a `shared_ptr< -XDoc>`). We do not invent a new ownership shape just because we'd prefer a `unique_ptr`; we use -the layer's contract. - ---- - -## 12. Parse and serialize: the API shape - -Pattern, applied uniformly: - -```cpp -// For every complex type T: -void parse(const ezxml::XElement& el, T& out); // throws on structural error -void serialize(const T& v, ezxml::XElement& parent, const std::string& tag); - -// For every value type V: -void toString(const V& v, std::string& out); // append to `out`; never throws -const char* toString(EnumValue v); // for plain enums; static storage -bool tryParse(const char* s, V* out); // strict; returns false on bad input -V parse(const char* s); // lenient: bad input -> first variant / 0 / clamp -``` - -**Why an out-parameter for parse rather than a return?** Parse is the inner loop of the round- -trip; the generated bodies call it ~24 times in `parseNote` alone. Out-parameter avoids one move -per call and lets the caller `emplace` directly into a `std::vector>` slot. -This is a hot path (see `gen/README.md` "the corert test ... ~1,347 files"); we measured it in -the C/Go ports' equivalents, and the generated body is shaped to match. - -**Why free functions, not member functions?** Because the data classes are aggregates (§3, §8). -Adding `parse` / `serialize` member functions would force the classes to be non-aggregates and -prevent designated-initializer construction. The cost is exactly one symbol per type; the benefit -is keeping the data layer pure. - -**Why `const char*` for parse input rather than `std::string_view`?** Most call sites have a -`std::string` in hand from ezxml (`getValue() const` returns `std::string`); both `const char*` -and `string_view` accept that. `const char*` matches what Go's templated free functions look like -when ported and the existing parse/clamp helpers use it. (`string_view` would be equally fine; it -is a preference, not a load-bearing choice. Lock either one and stay consistent.) - -**`throw std::runtime_error` for structural parse failures.** The corert harness already -`try { ... } catch(std::exception& e) { result.message = e.what(); }`; matching that contract -keeps the harness untouched. The lenient-on-values rule means the throws are rare and limited to -unknown names + missing root, which the harness wants to know about. - ---- - -## 13. Optionality and collections in detail - -| IR cardinality | C++ representation | Why | -|---|---|---| -| required (1) | `T value;` | DAG -> always safe by value | -| optional (0..1)| `std::optional value;` | C++17 native; value-typed; clear engaged/disengaged | -| vector (0..n) | `std::vector values;` | C++17 native; contiguous; supports the choice-vector pattern | - -Special cases: - -- `std::vector>` for the children of choice-bearing composites (§9b). -- `std::variant<...>` for a value-type union (§6). -- Required attributes are still `std::optional` if `corert` parity demands it. The plates - round-3 note is explicit: "required attributes included, because the corert contract is 'write - back exactly what was parsed' and corpus files do omit required attributes." The C++ target - inherits this rule. `Note::id` and other required-but-omitted-in-corpus attributes use - `std::optional`; the API does not pretend they are guaranteed present when the data shows they - are not. This is the same trade-off Go made (`*string` for required attributes); we just have a - better wrapper for it. - ---- - -## 14. Hand-written runtime - -Three files, hand-written, live alongside the generated code: - -### 14a. `Decimal.h` / `Decimal.cpp` - -The lossless decimal class. Already an `mx::core` decision per `gen/cpp/config.toml` `[types] -decimal = "Decimal"`. Sketch of API: - -```cpp -namespace mx::core -{ - -class Decimal -{ -public: - Decimal(); - explicit Decimal(double v); - explicit Decimal(int v); - explicit Decimal(const std::string& s); // parse exact text; lossless - - double getValue() const; // numeric face for arithmetic / clamps - const std::string& getText() const; // wire face: lossless reproduction - - void setValue(double v); // re-renders text using the configured policy - void setText(const std::string& s); - - bool operator==(const Decimal& other) const; - bool operator!=(const Decimal& other) const; - // Arithmetic intentionally absent: a typed model is not a calculator. -}; - -} // namespace mx::core -``` - -The trailing-zero handling that `DecimalFields.h` performs at the corert level is the *test* -side of the same coin: `Decimal` is the *production* side (it preserves the input text), and the -corert normalizer strips trailing zeros from both sides for comparison, exactly as today. - -### 14b. `Runtime.h` / `Runtime.cpp` - -The version constant (per round-3, generated from the schema stem so retargeting cannot leave it -stale) plus the small numeric/string parse helpers the generated code calls. - -```cpp -namespace mx::core -{ - -inline constexpr const char* SupportedMusicXMLVersion = "4.0"; - -bool tryParseInt(const char* s, int* out); -int parseInt(const char* s); // lenient: 0 on failure -bool tryParseDecimal(const char* s, Decimal* out); -Decimal parseDecimal(const char* s); - -} // namespace mx::core -``` - -`SupportedMusicXMLVersion` is **generated** into Runtime.h, despite Runtime.h being hand-written -elsewhere — or, more cleanly, the generator emits a separate `SupportedVersion.h` with the -constant and Runtime.h includes it. (Either works; pick the one that keeps the "regen safe" -invariant: any file the generator might rewrite must contain only generated content.) - -### 14c. The existing `mx/ezxml/` layer is the dependency - -The C++ target builds **on** the existing ezxml DOM (XDoc, XElement, XAttribute). It does not -parse XML itself. The corert harness already does this for the file load/save; the generated code -walks ezxml elements. This matches the Go (etree) and C (libxml2) targets; ezxml is C++'s -counterpart. - ---- - -## 15. CMake integration - -The press's `[render]` manifest emits a `sources.cmake` (the C target already does this) listing -the generated `.cpp` files. The repository `CMakeLists.txt` adds the generated directory and -`include()`s the manifest: - -```cmake -# CMakeLists.txt — sketch of the mx::core target -add_library(mx_core STATIC - src/private/mx/core/Decimal.cpp - src/private/mx/core/Runtime.cpp -) -include(src/private/mx/core/sources.cmake) # appends generated .cpp files -target_sources(mx_core PRIVATE ${MX_CORE_GENERATED_SOURCES}) -target_include_directories(mx_core PUBLIC src/private) -target_link_libraries(mx_core PUBLIC ezxml) -target_compile_features(mx_core PUBLIC cxx_std_17) -``` - -Generated `sources.cmake` writes a single `set(MX_CORE_GENERATED_SOURCES ...)` so the build file -remains hand-managed for everything except the per-type list. Same shape as the C target's -`sources.cmake` template. - ---- - -## 16. What to NOT do (and why) - -These are dead-ends a reviewer might independently propose; the design has rejected each. - -- **`std::shared_ptr` / `std::unique_ptr` everywhere.** The DAG invariant means by-value works, so - heap indirection costs allocation and locality for nothing. Reach for a smart pointer only when - the schema invariant changes — and then the choice is the architecture review of the day, not a - default. -- **Virtual destructors / runtime polymorphism.** Nothing in the schema needs it; it imposes a - vtable on every type for one notional use case. See §10. -- **Hand-rolled tagged unions.** `std::variant` exists. See §6, §9b. -- **A synthetic `Kind` enum on each composite's child variant.** The pointer-struct pattern in - Go/C needs it; the variant pattern does not. Adding one would re-introduce the harmony-`` - collision the variant solution avoids by construction. -- **Member function `parse` / `serialize`.** Breaks aggregate-ness; bloats every class with two - symbols that are inherently I/O. Free functions, see §12. -- **Strong typedef via `using`.** Loses type identity at the language level. Class with private - storage instead, §4-§5. -- **Reflection / Boost.Hana / a registry-of-types.** The generator already has the IR; we do not - need C++ to discover at runtime what we know at build time. Templates emit straight-line C++ - from straight-line plate data. -- **A `Visitor` base class for tree walks.** Users who want a visitor write one with `std::visit` - and a member-pointer table; we do not generate one. Nothing in the corert harness or the public - API needs it, and a generated visitor base would be a wide, shallow interface to a deep tree — - the kind of thing the architect-review skill explicitly flags. -- **Coroutines, ranges, `std::expected`.** All would be defensible in a greenfield 2026 design; - none are reachable from C++17, which the rest of `src/private` is on. Holding the line on the - language baseline keeps blast radius local. - ---- - -## 17. What this design intentionally leaves open - -A short list, with the reason each is deferred: - -- **Header/impl split granularity.** §2 says one type per pair. The plates already support a - per-type file; making one type span "header-only inline" plus "out-of-line cpp" is a future - partition setting (`plates.md` §10 lists this as future work). For now: every type gets a - `.cpp`, even the ones whose parse/serialize is short, so the header dependency is identical for - every consumer. -- **`operator==` for variants whose alternatives are duplicated types.** `std::variant`'s default - `==` works on (index, value) pairs, which is what we want. No generator action required, but - worth a unit test on (e.g.) `NoteChild{std::in_place_index<1>, true}` != - `NoteChild{std::in_place_index<6>, true}` to confirm the index discriminates as expected. -- **`std::hash` / set/map keys.** Not generated. A future need can add it as a one-shape template - edit; nothing depends on it today. -- **Streaming serialize.** `serialize` emits to ezxml's DOM, which then writes to a stream. A - direct streaming serializer (skip the DOM) is possible and may be a follow-up if benchmarks - show the DOM allocation hurts; the current shape is the same as the C++ Reference - implementation has used and is fast enough for the corert workload. -- **The `Decimal` class's exact API.** §14a is a sketch; the precise trailing-zero policy and - whether to surface arithmetic operators is an open call. Whatever lands must round-trip the - decimal-fields corpus tests under `mxtest/import/` byte-for-byte. - ---- - -## 18. Architecture review against `arch-review` - -A quick self-audit against the principles the review skill applies, in priority order: - -**Domain boundaries.** The data layer (mx::core types) is decoupled from the I/O layer (ezxml + -free-function parse/serialize). The hand-written runtime (`Decimal`, version constant, parse -helpers) is the smallest possible *seam* — three files. Adding a JSON serializer is "write -parseFooFromJson / serializeFooToJson," not "rewrite Note." Boundary holds. - -**Simple, deep abstractions.** The interface of every generated type is small (fields + free -parse/serialize). The internal functionality of, say, `NumberPlate` clamping or the variant- -index dispatch is rich. Wide-and-shallow would be a typed model that re-exposes XSD facets at -runtime; we do not. - -**Blast radius.** A schema bump regenerates files; no hand-written code changes. Adding an -attribute to one element changes one `.h`/`.cpp` pair. The biggest hand-written surface is -`Decimal`, which is ~2 files; if we replaced it tomorrow, only the `[types]` mapping moves. - -**Clarity.** `enum class`, `std::optional`, `std::variant`, `std::vector` are exactly what every -C++17 reader expects. Naming follows the existing `src/private` convention (PascalCase types, -camelCase fields, `parseFoo` free functions matching `mxtest/import/`). No surprises. - -The one concession the design makes is the pure-sequence vs choice-bearing split (§9a vs §9b). -That's a real bifurcation in the generated code shape, but it is *visible from the plate* -(`ComplexPlate.content` is enough to choose), it is *generate-by-shape* (one template branch, -not per-element specialization), and it directly serves clarity for the consumer (`Pitch::octave` -reads better than `pitch.children[2]`). It earns its complexity. diff --git a/docs/ai/design/mx-core-plan.md b/docs/ai/design/mx-core-plan.md deleted file mode 100644 index d1a59dffb..000000000 --- a/docs/ai/design/mx-core-plan.md +++ /dev/null @@ -1,393 +0,0 @@ -# mx::core plan: the decision record and definition of done - -Status: APPROVED and IMPLEMENTED (2026-06-11; see §7 for the implementation record and the -recorded deviations). This is the binding plan for the generated C++ `mx::core` target. It was -produced by an owner interview on 2026-06-11 that resolved every open conflict between the two -design proposals, `mx-core-gpt-5.0.md` and `mx-core-opus-4.7.md`. Where this document contradicts -either proposal, **this document wins**. Where it is silent, the proposals remain useful reference -material for rendered-shape sketches (the gpt-5.0 doc for API philosophy, the opus-4.7 doc for -plates mechanics, file layout, and dispatch technique). - -The reader is assumed to know `plates.md`, `generator-agnosticism.md` (both since folded into -`gen/DESIGN.md`), and `gen/README.md`. -Nothing here amends the generator architecture: C++ is a target directory (`gen/cpp/config.toml` -plus `gen/cpp/templates/`), and the cardinal rule holds — no language knowledge enters the -generator's Python. One exception is explicitly scoped in §2.9: projecting the content tree and -named groups onto the Plates is *neutral schema fact* work that serves all targets. - ---- - -## 1. The non-negotiables (owner's cardinal requirements) - - -1. **Valid by construction.** It must be impossible to use the public C++ API to produce a - document that is invalid against the MusicXML 4.0 spec. Invalid states are made - *unrepresentable* by the type system wherever possible — including bespoke structural - representations for individual types where the schema's string-with-pattern view hides - structure (RGB color, dates, comma-separated lists). -2. **Errors in exactly two places, nowhere else.** - - Rejected parse: `parse(...) -> Result` is the error boundary. - - Bounded-cardinality append: `maxOccurs` is enforced; an over-cap append is rejected - (`Result`). MaxOccurs enforcement is mandatory, not a documented near-miss. - No other constructor, setter, or factory returns or throws an error. Construction repairs - deterministically (clamping, normalization) instead of failing. -3. **Modern, safe C++20.** No bare `new`/`delete`, no raw owning pointers, smart pointers where - indirection is needed at all, Meyers-Effective-Modern-C++ discipline. Raise CMake to C++20 - when this target lands. -4. **Optimization priorities, in order: compile time, runtime memory, runtime speed, binary - size.** Consequences: light headers / heavy `.cpp` bodies, no template metaprogramming in - generated code, `std::variant` dispatch by index switch (jump table) rather than `std::visit` - towers, boxed alternatives only where alternative sizes demand it. - -## 2. Decisions - -Each decision below was explicitly approved by the owner during the interview. - -### 2.1 XML layer: pugixml directly; ezxml retires - -Generated code consumes **pugixml** (`pugi::xml_node`, `pugi::xml_document`) directly, the same -way Go consumes etree and C consumes libxml2. ezxml — a virtual-interface wrapper whose every -node access allocates a `shared_ptr` and copies `std::string`s — is retired; its swappability -promise is redundant (target swappability lives in the generator, not inside C++). - -- Promote the vendored pugixml (currently buried at - `src/private/mx/ezxml/src/private/private/pugixml.{hpp,cpp}`, version 1.10) to its own - directory, e.g. `src/private/pugixml/`, and **refresh it to the current upstream release** - (1.15 at time of writing). Keep `pugilicense.txt` beside it. -- Domain classes never read or write streams or files. Free functions map model <-> pugixml DOM; - pugixml owns load/save/encoding (it auto-detects UTF-16/Latin-1, matching what libxml2 does - for the C target). DOCTYPE handling needs the `pugi::parse_doctype` flag. -- The corert harness, `mxtest/import` normalization utilities (sort attributes, strip decimal - zeros), and the Fixer are ported to pugixml. Once no client remains, delete - `src/private/mx/ezxml/`. -- Later (0.7): the core now reaches pugixml through the inline `mx::xml` seam, which keeps this - decision's performance while making the XML library a build-time choice; see `xml-backend.md`. - -### 2.2 Value types: structural unrepresentability first, then clamp - -Priority order for representing a constrained value type: - -1. **Make invalidity unrepresentable structurally.** Bespoke shapes are expected and welcome: - - `color` stores numeric RGB(A) components; serialization formats `#RRGGBB`/`#AARRGGBB`. A - malformed hex string cannot exist. - - `yyyy-mm-dd` stores year/month/day integers, clamped to valid ranges. - - `comma-separated-text` stores `std::vector`. - - Prefix-patterned SMuFL glyph-name tokens (`acc\c+` etc.) store the suffix; the prefix is - structural and emitted by the serializer. - - The full audit of patterned/structured value types in the 4.0 schema is the first task of - the values wave (§4, W1): every `StringPlate` with a pattern gets either a structural - representation or an explicit recorded justification for staying a normalized string. -2. **Clamp/normalize on construction** where a deterministic nearest-valid repair exists. - Numbers clamp using the plates' `clamp` steps (`NumberPlate.clamp` — already projected data), - on construction and on `set`. One policy, one spelling, identical to import. -3. **Closed enums need no validation at all**: enum wrapper types whose only constructors are - the named factories; a `static_cast`-proof invalid value is unrepresentable through the - public API. -4. Unconstrained strings construct directly. - -Default construction is always valid: every type's default is its natural valid zero, recursively -(safe because the complex-type graph is a DAG — by-value composition throughout, no forward -declarations, no shared ownership). - -Mechanisms for bespoke shapes, both pre-existing: manifest `[[render.type]] types = ["..."]` -bespoke-template rows (precedent: Go's `yyyy_mm_dd.go.tmpl`), and hand-written runtime files for -schema-independent types (`Decimal`), referenced via `[types]` in `gen/cpp/config.toml`. - -### 2.3 Composites model the schema grammar exactly - -A flat ordered child list (the Go/C encoding) is rejected for C++: it can represent a `` -with two ``es, which violates §1.1. The generated shape follows the resolved content tree: - -| content shape | C++ representation | -|---|---| -| pure sequence | named fields in schema order; serializer walks declaration order, so wrong order is unrepresentable | -| one-shot choice | a `std::variant` member — exactly-one by construction | -| named schema group (`full-note`, `music-data`, ...) | a generated shared type (class or variant alias) reused by every referencing composite | -| repeated choice (e.g. measure music-data) | private `std::vector` — any order/count of alternatives is schema-valid there, so the vector represents nothing invalid | -| repeated homogeneous element | private `std::vector`, read via `std::span` | -| bounded repeat (finite maxOccurs > 1, e.g. `beam` <= 8) | private vector + `add...(T) -> Result` that rejects an over-cap append | -| presence-only element | a flag (bool field in a sequence; an `Empty` marker type as a variant alternative) | -| optional anything | `std::optional` | -| derived complex types | non-polymorphic public inheritance (`strategy = inherit`); no virtual anything | - -Variant alternatives are by-value by default; box with `std::unique_ptr` **only** where a repeated -heterogeneous choice has large alternatives (measure music-data is the known case — a by-value -`variant` would size every slot at the largest alternative). Which groups box is target -data (`gen/cpp/config.toml` `[vars]`), not template hardcode. Duplicate variant alternatives -(`` and `` both presence-only) dispatch by `std::in_place_index` / -`switch (v.index())` — index, never type. - -### 2.4 Import policy: strict names and structure, lenient values, configured repairs - -Parsing (the only door for external data) applies: - -- **Strict on names**: unknown element or attribute -> error. With version gating, an unknown - name is a generator gap or garbage input, never data to preserve. -- **Strict on structure**: wrong sequence order, duplicate choice alternatives, missing required - children, over-cap repeats -> error. Corpus evidence (§3) shows this costs zero files. -- **Lenient on values** (the owner classes these with clamping, not with structure): - - numbers: lexically-strict parse, then range-clamp (the established fixup-sidecar policy); - - enums: an unknown literal falls back to the first variant; - - patterned strings: normalize into the structural representation's nearest valid value. - Every repair still yields a valid model object — leniency never admits an invalid document. -- **Missing required attributes**: inject a configured default. Config data, not generator - logic, e.g. in `gen/cpp/config.toml`: `lyric-language`'s `xml:lang` defaults to `"und"` (BCP 47 - "undetermined" — the spec-correct unknown-language code). A missing required attribute with - *no* configured default is a parse error. The configured-defaults table is the lever if the - corpus or real-world files ever surface more cases. -- **Version gating**: a root declaring a version newer than the generated - `SupportedMusicXMLVersion` (`"4.0"`, emitted from `Plates.schema_version`) is rejected with - `unsupportedVersion`; the harness counts it as a skip. - -The user-facing construction API stays strict in the §1.1 sense regardless: leniency is the -parser's door only, and even the parser's repairs produce only valid objects (the two-door -policy). - -### 2.5 Errors: one boundary type, internal throw - -```cpp -namespace mx::core -{ -enum class ErrorCode -{ - unknownElement, unknownAttribute, missingRequiredElement, missingRequiredAttribute, - wrongElementOrder, tooManyElements, invalidDocument, unsupportedVersion, -}; - -struct Error -{ - ErrorCode code; - std::string path; // e.g. /score-partwise/part[1]/measure[3]/note[2] - std::string message; -}; - -template class Result; // small hand-written expected-like; C++20 has no std::expected - -Result parse(const pugi::xml_document& doc); -void serialize(const Document& d, pugi::xml_document& out); -} -``` - -Internally, generated per-type parse functions **throw** a `ParseError` carrying path context; -the document-level `parse` catches and converts to `Result`. This keeps ~24-arm dispatch bodies -free of per-child propagation boilerplate while the public boundary stays explicit and -exception-free. `serialize` is total for every model object (§1.1 guarantees the object is -valid). Per-type `parse...`/`serialize...` free functions exist (testability) but the documented -public contract is the Document-level pair. - -### 2.6 API spelling and layout - -- Accessors: `step()` / `setStep(Step)` (modern accessor style — owner's pick). Private members - `m_step` (`m_` prefix; chosen so the reserved-word *suffix* mangling can never collide with a - member-suffix convention). -- Types PascalCase, one type per `PascalCase.h`/`.cpp` pair under `src/private/mx/core/generated/` - (committed, like Go/C output), plus a generated `sources.cmake` (C-target precedent). -- `namespace mx::core`; `#pragma once`; headers include only what they textually need (plates - `deps` drive the include list); doc comments from the schema. -- Hand-written runtime beside the generated code: `Decimal.{h,cpp}` (lossless decimal: text face - for round-trip fidelity, numeric face for clamping), `Result.h`/`Error.h`, small parse/format - helpers (`std::from_chars`-based, locale-free). Generated-only files carry the generated - banner; hand-written files never contain generated content (regen-safe split: the version - constant lands in its own generated header). -- Repo `CMakeLists.txt` gains an `mx_core` static library (C++20) that includes `sources.cmake`; - corert links it. clang-format runs as the manifest's `[render] format` hook. - -### 2.7 Corpus actions (evidence in §3) - -- **Fix the 5 repo-authored synthetic fixtures** missing required `xlink:href` - (`data/synthetic/{link.3.0,opus.3.0,part-link.4.0,group-link.4.0,instrument-link.4.0}.xml`) — - they exist to exercise those elements; marking them invalid would erase that coverage. -- **Add `.fixup.xml` sidecars** for the 4 real-world files missing `xml:lang` on - `lyric-language`, using a new fixup operation: **add-attribute-to-expected** (the C++ importer - injects `xml:lang="und"`, so the expected document must gain it too). The Go/C Fixers must - skip operations they don't implement so their suites stay green untouched. -- **No `.invalid` markers needed**; no files leave the corpus. - -### 2.8 Go and C are anemic, and documented as such - -The Go and C targets are **non-validating round-trip harness bindings**: they enforce neither -sequence grammar nor choice exclusivity, and their APIs can construct invalid documents (a note -with two pitches is representable). The owner's position, recorded verbatim in spirit: an API -that can construct an invalid document is unacceptable for any product surface; these targets' -only legitimate role is keeping the generator honest about language-agnosticism, and their -current shape is tolerated only in that role. - -**Follow-up (approved, scheduled after C++ lands):** retrofit Go and C to the same -grammar-enforcing contract, reusing the §2.9 plates extension. Until then AGENTS.md and the -target READMEs must carry the anemic-API warning. - -### 2.9 The one generator change: project content tree and groups onto the Plates - -The Plates currently splice groups away and the Go/C templates read only the flat member list. -C++ templates need the resolved content tree (sequence/choice nesting) and named-group identity -(`full-note` as a shared type). Exposing these is **neutral schema fact** projection — the same -test the sounds fold and the clamp steps passed — so extending `gen/plates/` (and, if needed, -`gen/ir/resolve.py`) is legitimate Python work under the cardinal rule. It must be: - -- consumed via plate fields any target could read (the JSON Schema target is the forcing - function: it already wants `content`); -- covered by `gen/tests/test_plates.py`; -- invisible to Go/C output (regen produces a zero diff for them — part of Gate 5). - -No C++ branch enters Python. `python3 -m gen plates --check` remains the CI gate. - ---- - -## 3. Corpus evidence (measured 2026-06-11, 829 eligible files) - -These facts drove §2.4 and §2.7; re-verify them if the corpus changes. - -- **Missing required attributes: 9 files, 4 patterns, zero caused by 4.0 breaking changes.** - `lyric-language@xml:lang` (4 real-world files; required since 3.0) and `xlink:href` on - `link`/`opus`/`part-link` (5 synthetic fixtures; required since 3.0 / since the element - existed). -- **Structural violations: zero.** Full xmllint XSD validation of all 829 files found not one - wrong-order, unexpected-element, or missing-required-child error. Strict structural parsing - costs nothing. -- **Value violations: 10 files, all already carrying fixup sidecars.** Mostly numeric range - (`sound@dynamics="-1.11"`, `midi-channel 0`, `duration 0` — clamp territory); three files hold - non-numeric garbage (`display-step` of `=`, `accordion-middle` of ``/`test`, empty - `fret`/`string`) — covered by the enum-first-variant / normalization leniency. -- Expected corert C++ counts: **829 pass / 0 fail / 0 skip** (nothing in the corpus declares a - version newer than 4.0). Go/C stay at **777 / 0 / 52**. - ---- - -## 4. Implementation waves - -All template/config work; each wave ends at a named gate and a commit. Generated output is -committed at every wave. Go/C suites must be green at every wave boundary (agnosticism -regression). - -- **W0 — foundations.** Promote + refresh pugixml; hand-written runtime (`Decimal`, `Result`, - `Error`, parse helpers); CMake `mx_core` skeleton + C++20 bump; the §2.9 plates extension with - tests; fix the 5 synthetic fixtures. - *Gate: plates `--check` green, `test_plates`/`test_agnosticism` green, Go/C regen zero-diff - and suites green.* -- **W1 — value types.** Audit every patterned/structured value type and record its - representation (§2.2.1); templates for enum/number/string/union + bespoke rows (`color`, - `yyyy-mm-dd`, `comma-separated-text`, SMuFL names, ...); values smoke test (C-target - precedent). - *Gate: all value types emitted, compile, smoke test green, Gate-4 clamp/factory unit tests - for representative types.* -- **W2 — complex types.** Templates for attrs/value-bodied/sequence/choice/group/derived shapes; - shape unit tests (sequence order unrepresentability, variant exclusivity, maxOccurs - rejection, default-construction validity). - *Gate: full `mx_core` compiles; shape unit tests green; compile time recorded.* -- **W3 — document + corert.** `Document`, version gating, root-namespace preservation, generated - DOCTYPE/version constants; port harness + `mxtest/import` + Fixer to pugixml and the new API; - add the add-attribute fixup op (+ Go/C fixers skip unknown ops); the 4 `xml:lang` sidecars. - *Gate: Gates 1, 2, 3 green (see §5).* -- **W4 — adversarial audit + cleanup.** Run Gate 6; fix findings; delete ezxml and the legacy - harness pieces; AGENTS.md updates; record final compile-time numbers. - *Gate: all six gates green; ezxml gone; docs current.* - ---- - -## 5. Definition of done: the six gates - -corert green alone is **not** done — it proves round-tripping of valid files, not the cardinal -properties (rejecting invalid input; invalid construction being unrepresentable). - -1. **corert C++ green with pinned counts.** 829/0/0 (pass/fail/skip); count drift is failure - even with zero "fails". Go/C stay 777/0/52. -2. **Emitted documents are schema-valid — permanent CI gate.** Every parsed corpus document is - serialized and the *output* xmllint-validated against the 4.0 XSD. Even the 10 - value-violating inputs must emit valid XML (proves the clamp policy end-to-end). This is the - cardinal rule's mechanical proof and it stays in CI, not one-time. -3. **Rejection suite.** Hand-authored invalid documents that parse must refuse — duplicate - ``, `` before ``, unknown element/attribute, missing required child, - root version 5.0 — each asserting the right `ErrorCode` and a meaningful path. -4. **Construction-safety tests.** Runtime: clamping on construct/set, maxOccurs append - rejection, default-construct-every-type-then-serialize-then-xmllint (mechanically - generatable). Compile-time: must-NOT-compile probes (e.g. constructing an enum wrapper from a - raw int) verified by a script asserting compilation fails. -5. **Generator hygiene.** `plates --check` green; `test_agnosticism` green; regen-then- - `git diff --exit-code` (fresh, deterministic, committed output — including zero Go/C diff); - clang-format hook applied. -6. **Adversarial subagent audit.** At every "done" claim (each wave end and before final merge), - a **fresh-context** subagent receives only the MusicXML 4.0 XSD, the generated public - headers, and the instruction: *construct a schema-invalid document using only the public API - — no casts, no UB, no reaching into privates.* It writes and compiles real programs; - serialized output is xmllint-validated; any invalid output is a bug against the API design. - Mirror direction: the subagent crafts invalid input XML it believes parse should reject; - anything that parses but whose re-serialization xmllint-fails is a leniency hole (findings - triaged against the documented clamp policy — designed repairs are expected to pass). Fresh - context is the point: the auditor must not inherit the implementing session's assumptions. - -Compile time (priority #1) is measured and recorded at each wave boundary rather than -hard-gated, so regressions are visible without inventing an arbitrary budget. - ---- - -## 6. Follow-ups (recorded so they don't evaporate) - -1. **Retrofit Go and C to the grammar-enforcing contract** (§2.8), reusing the §2.9 plates - extension: Go gets real sequence fields + sum-type-style choices, C gets tagged unions. - Their corert suites then test the strict contract in three languages. -2. **Decimal contract finalization**: exact trailing-zero/precision policy must round-trip the - decimal-fields corpus byte-for-byte under the existing normalization; revisit once W3 data is - in hand. -3. **Streaming serializer** (skip the DOM) only if benchmarks ever show DOM allocation matters — - not before. - -## 7. Implementation record (added at completion, 2026-06-11) - -All four waves landed and all six gates are green. What an implementer (or auditor) should -know beyond the decisions above: - -- **Corpus actions (§2.7) were already satisfied.** The W0 re-verification (per §3's "re-verify - if the corpus changes") found the 5 synthetic `xlink:href` fixtures and the 4 `lyric-language` - `xml:lang` files already fixed in `data/`; exactly the 10 fixup-sidecar value-violation files - fail xmllint, all clamp-policy territory. No new sidecars and no add-attribute fixup - operation were needed (the C/Go fixers already skip unknown `` type flavors). The - `[import] attribute-defaults` mechanism exists regardless (`lyric-language`'s `xml:lang` - defaults to `"und"`). -- **The §2.9 projection as built**: `Resolver` gained the field view (trivial single-element - groups splice inline; structural groups stay referenced; choice-shaped groups like - `music-data` collapse so reference sites absorb the occurrence); the Plates gained - `ComplexPlate.fields`, `Plates.groups` (group-class / choice-class plates, including - hoisted `{owner}-choice` / `{owner}-group` types renameable via `[rename.type]`), - first-name dispatch sets, nullability, `default_alt` (a nullable choice's natural zero is - its empty alternative, so a default `` is valid), `is_base`, and `structural_deps`. - Go/C/schema output stayed byte-identical throughout (Gate 5 verified at every wave). -- **Boxing knob**: §2.3 suggested `[vars]` for naming which choices box; as built it is a - manifest `[[render.type]] types = ["music-data-choice"]` row selecting the boxed-choice - templates -- still config data, not template hardcode, and it reuses the existing bespoke-row - mechanism instead of inventing a vars convention. -- **Natural-zero refinement**: required opaque-string attributes default to `"P1"` -- the IR - canonicalizes `xs:ID`/`xs:IDREF` to `token`, and the empty string is not a valid ID, so the - default-constructed Document could not otherwise pass Gate 2's xmllint check. -- **Gate 4's xmllint clause, as built**: only the two roots are globally declared in the - schema, so non-root fragments cannot be xmllint-validated in isolation. The mechanical - probe (`roundTripDefaults`, generated) instead serializes every type's natural zero and - re-parses it with the strict grammar; the default Document is xmllint-validated with the - corpus outputs in `make validate-cpp`. -- **Wire-fidelity text faces**: `Decimal`, the comma-separated list types, `time-only`, and - `ending-number` preserve the exact parsed spelling and re-emit it; numeric/structural - mutation drops the face and serializes canonically. This is what keeps corert exact-text - comparisons green without weakening the structural representations. -- **Compile time (priority #1) record**: clean `mx_core`, GCC 13, `-j8` Debug -- W1 - (values only) 14.2s wall / 44s user; W2-W4 (full model, 905+ generated files) ~1m40s wall - / ~5m35s user. -- **Gate commands**: `make test-core-dev` (Gate 1, pinned 829/0/0), `make validate-cpp` - (Gate 2, permanent), `make test-cpp-unit` (Gates 3 and 4 runtime), `make probe-cpp` - (Gate 4 compile-time), `make test-gen` + plates `--check` + regen-zero-diff (Gate 5). -- **Gate 6 (adversarial audit) found and the implementation closed** a systemic lexical hole: - every `xs:ID`/`xs:IDREF`/`xs:NMTOKEN` attribute had been a raw `std::string` setter that - accepted spaces, the empty string, and leading digits and wrote them back verbatim. The IR - had collapsed `xs:ID`/`xs:IDREF` to `token`, hiding the distinction. The fix preserves - `id`/`idref` as distinct IR primitives (a neutral schema fact, parallel to the already- - distinct `nmtoken`), adds the neutral `PlateRef.name_token` flag, and gives the C++ target - hand-written repairing wrappers (`Token` = NCName for id/idref, `NameToken` = the wider - NMTOKEN). A bare string literal can no longer reach a `setID` (the converting constructor is - explicit -- probe 7); parse repairs malformed identity tokens as a designed value-leniency - repair. Go/C/schema map the new primitives to their existing string spellings, so their - committed output is byte-identical. -- **Known limitation (referential integrity, out of scope by §1.1's "wherever possible").** - Two intrinsically valid `Token`s with the same value still serialize duplicate `xs:ID`s, and - a dangling `xs:IDREF` is unconstrained -- both are document-graph properties no per-object - type can enforce locally, and the audit confirmed `xmllint --schema` does not even check - IDREF existence. Enforcing them would require a document-level validation pass, which the - design rejects (the two-error-doors rule). They are the standard XSD-validity-beyond-the- - type-system carve-out, documented here and in AGENTS.md rather than papered over. diff --git a/docs/ai/design/mx-impl-port-brief.md b/docs/ai/design/mx-impl-port-brief.md deleted file mode 100644 index eb6e1782a..000000000 --- a/docs/ai/design/mx-impl-port-brief.md +++ /dev/null @@ -1,99 +0,0 @@ -# Phase 2 Kickoff Brief: the `mx::impl` Port - -Status: SUPERSEDED. The design session happened; every §5 question is settled in -`mx-impl-port-plan.md`, the BINDING Phase-2 decision record. This brief remains as the -evidence snapshot it was. Scope authority remains plan §8 as amended by the design doc. - -## 1. Goal and acceptance (fixed by plan §8) - -**Goal:** `mx::api` works again, implemented via `mx::impl` against the new generated -`mx::core`; api changes minimally invasive (test-harness fallout only). - -**Acceptance:** -- preserved `mxtest/api` + `mxtest/impl` suites ALL GREEN; -- `MX_API` default ON; -- `lib`/`dev`/`test`/`examples-run` Makefile targets restored; -- coverage job naturally grows to cover api/impl. - -**Out of scope:** xcode/xcframework/iOS/macOS distribution (Phase 3). - -## 2. Where Phase 1 left the tree - -- Branch `newgen-integration` (all Phase-1 gates green; see plan §7). The new core lives at - `src/private/mx/core/` (runtime) + `src/private/mx/core/generated/` (model); entry points are - `mx::core::parse(const pugi::xml_document&) -> Result` and - `mx::core::serialize(const Document&, pugi::xml_document&)` in `generated/Document.h`. -- The whole product stack is restored in-tree behind `option(MX_API OFF)` in CMakeLists.txt: - the `mx` static library (api + impl + utility, linking `mx_core`), the `mxtest` binary - (api/control/file/impl/import suites + cpul), and the three example programs. It is wired but - has never compiled against the new core. There is no Makefile target for it yet (deliberate: - no advertised target may be broken). -- The old api-import harness files sit in `src/private/mxtest/import/` next to newgen's - `Normalize.{h,cpp}`/`DecimalFields.h`, as rewrite reference. - -## 3. Measured inventory (run on this tree, commit e713bfc's successor) - -- Old-core includes (`#include "mx/core/..."` paths that no longer exist): **46 of 77** impl - files, **4 of 13** api .cpp files, **7 of 8** `mxtest/impl` files — **348 distinct old-core - headers** referenced in total (mostly `mx/core/elements/*.h`, plus `mx/core/Enums.h`). -- ezxml references (the library is deleted): `src/private/mx/api/DocumentManager.cpp`, - `mxtest/api/{ApiTester,PitchDataTest,SlurTest}.cpp`, and the entire old import harness - (`ImportTest*`, `ExpectedFiles*`, `ChangeValues*`, `SortAttributes*`). -- `DocumentManager` public surface (`src/include/mx/api/DocumentManager.h`): `createFromFile`, - `createFromStream`, `createFromScore`, `writeToFile`, `writeToStream`, `getData`, - `destroyDocument` — an int-handle registry around the old `mx::core::Document` shared - pointer. This is the api<->core seam and the natural first rewrite. -- Suites: `mxtest/api` has 30 test .cpp files (plus ScoreData headers); `mxtest/impl` has 8 - files (CurveFunctions, DirectionReader, DirectionWriter, MagicTemplates, MeasureWriter, - PositionFunctions, PrintFunctions, ScoreWriter) — these assert old-core types directly. -- Examples (`mx/examples/`): api-only consumers; no direct core or ezxml use. - -## 4. Old core -> new core: the deltas impl must absorb - -| Old core (what impl was written against) | New core (what exists now) | -|---|---| -| `std::shared_ptr` element graph (`makeNote()`, `NotePtr`) | value-semantics classes; `std::optional` members | -| `setHasFoo(true)` presence flags | `std::optional` presence (set the optional) | -| `Choice` enum + all alternatives co-allocated | `std::variant` with positional Kind/index dispatch | -| order-free child setters | named fields in schema order; serializer walks declaration order | -| `fromXDoc(std::ostream&, XDoc&)` per element, bool return | parse exists ONLY at the document boundary: `parse(...) -> Result` | -| ezxml `XDoc`/`XElement` DOM | pugixml directly | -| invalid values representable; clamping ad hoc on import | clamp-on-construct wrappers; invalid unrepresentable; bounded `add...` returns `Result` | -| required-with-unbounded: default element pre-pushed into the set | `OneOrMore` (a first element always exists, by type) | -| MusicXML 3.0-era element set | MusicXML 4.0 model + version gating on parse | - -Semantic fallout to adjudicate per-test (plan §8): api tests that relied on old-core leniency, -error behavior, or representational slack need deliberate decisions, not mechanical porting. - -## 5. Decisions the design session must settle (the grill agenda) - -1. **Port strategy for impl:** mechanical file-by-file translation of the 77 impl files vs. - reimplementation around the new core's shape (the old reader/writer split may not map; the - new core's variants and schema-order fields remove whole classes of old impl bookkeeping). -2. **The `DocumentManager` rewrite:** keeps its int-handle/singleton contract (minimally - invasive) — but what does it hold now that `Document` is a value type? What happens to - `mx::core` parse failures that the old api silently tolerated (`Result` carries errors; - the old `createFromStream` had no failure channel)? -3. **api surface changes:** plan says "test-harness fallout only." Is the version field - (`ScoreData::musicXmlVersion`) now 4.0? What does `mx::api` emit as the root `version`? -4. **`mxtest/impl` per-file fate:** the 8 files assert old-core types directly; port or retire - each (e.g. MagicTemplatesTest may be meaningless against the new core). -5. **The api-level corpus roundtrip:** rebuild ImportTest on pugixml, preferring newgen's - `Normalize.cpp`; do `SortAttributes`/`ChangeValues`/`ExpectedFiles` (and `data/expected/`) - survive at all? -6. **`mx::utility` (9 headers, 15 impl files reference it):** keep as-is, shrink, or absorb. -7. **Order of work and gating:** which suite comes green first (impl unit vs api vs import)? - When does `MX_API` flip ON in CI (a new CI job, or extend linux-core)? When do the - `lib`/`dev`/`test`/`examples-run` Makefile targets return? -8. **Old-core idioms with no new equivalent:** e.g. impl code that built partially-invalid - intermediate states before fixing them up — the new core forbids those states; the impl - algorithms that depended on them need redesign, and they should be found early (the - `MX_API=ON` error wall is the cheapest census: configure with `-DMX_API=on` and catalog). - -## 6. Suggested first moves (cheap, evidence-producing) - -1. `cmake -S . -B build/api-wall -DMX_API=on && cmake --build build/api-wall` — capture and - classify the error wall (it is the true, compiler-verified inventory of §3). -2. Rewrite `DocumentManager.cpp` first; everything in `mxtest/api` funnels through it. -3. Pick one thin vertical slice (e.g. PitchData: api struct -> impl reader/writer -> one api - test) to validate the chosen port strategy before committing to all 77 files. diff --git a/docs/ai/design/mx-impl-port-plan.md b/docs/ai/design/mx-impl-port-plan.md deleted file mode 100644 index 3efe71350..000000000 --- a/docs/ai/design/mx-impl-port-plan.md +++ /dev/null @@ -1,475 +0,0 @@ -# Phase 2 Design: the `mx::impl` Port - -Status: APPROVED, BINDING. This is the decision record for Phase 2 of the newgen integration -(the `mx::impl` port), produced by studying the tree and interviewing the owner. It settles -every open question in `mx-impl-port-brief.md` §5 and amends plan §8's scope where noted -(owner-approved). Execute Phase 2 from this document. Scope authority for goal/acceptance -remains `newgen-integration-plan.md` §8; where this document says "amended," it overrides. - -## 1. Goal and acceptance - -**Goal:** `mx::api` works again, implemented via `mx::impl` against the new generated -`mx::core`. - -**Acceptance (plan §8, with owner-approved amendments):** - -- preserved `mxtest/api` + `mxtest/impl` suites ALL GREEN (per-file fates in §7); -- the new corpus-wide api roundtrip gate (§8) green with **zero regression vs. the pre-port - baseline**; -- `MX_API` default ON; -- `lib`/`dev`/`test`/`examples-run` Makefile targets restored; examples build AND run; -- CI green tip-to-toe, including the new `linux-api` job and the extended coverage/macos jobs - (§10); -- coverage job naturally grows to cover api/impl. - -**Scope amendments vs. plan §8's "api changes minimally invasive (test-harness fallout -only)":** the owner approved two deliberate public-api changes — the `DocumentManager` -error channel (§3: `Result`-returning signatures, no escaping exceptions) and the version -fields on `ScoreData` (§4). Everything else on the api surface stays put. - -**Out of scope (Phase 3):** xcode/xcframework/iOS/macOS distribution; the SMuFL -glyph-name migration (§12 — recorded there as open questions with evidence). - -## 2. Port strategy: mechanical translation, algorithms preserved - -**The cardinal rule of the port (owner-mandated): the impl algorithms are preserved -exactly.** They are the glue that makes `mx::api` produce the documents users want; the -logic must be the same — only the *shape of `mx::core` access* changes. NoteReader.cpp -stays NoteReader.cpp; the 77-file structure stays; the reader/writer split and the -`Converter` hub stay. - -The idiom translation table (old core → new core): - -| Old-core idiom | New-core translation | -|---|---| -| `#include "mx/core/elements/Foo.h"`, `Enums.h`, `Decimals.h`, `Document.h` | `#include "mx/core/generated/Foo.h"` etc. (per-type headers; aggregates are gone) | -| `core::NotePtr` / `makeNote()` shared-ptr graph | value construction; `core::Note note{};` | -| `el->setHasFoo(true); el->getFoo()->setValue(v)` | `el.setFoo(V{v})` (`std::optional` presence) | -| `attr->hasDefaultX` / `attr->defaultX.getValue()` | `el.defaultX().has_value()` / `el.defaultX()->value()` (attributes are members of the element class; no separate Attributes struct) | -| `Choice` enum + `getChoice()` + co-allocated getters | `std::variant` alternative classes; positional `Kind`/index dispatch (`choice.kind()`, `asFoo()`) | -| `getFooSet()` (vector of shared ptrs) | `foos()` (`std::vector` / `OneOrMore` by value) | -| old enum `core::StepEnum::a` | factory wrapper `core::Step::a()`, compare via `tag()` | -| ad-hoc clamping on import; invalid values representable | clamp-on-construct wrappers; invalid unrepresentable | -| unbounded `addBeam(...)` | bounded `addBeam(...) -> Result` (propagate, see §3 row `createFromScore`) | -| required-with-unbounded: default element pre-pushed | `OneOrMore` (first element exists by type) | -| `fromXDoc`/`toXDoc` per element | parse/serialize exist ONLY at the document boundary (`generated/Document.h`) | - -**Where the old maneuver is unrepresentable** (e.g. code that built a partially-invalid -intermediate state and fixed it up later, or relied on shared-ptr aliasing to mutate at a -distance), restructure as locally as possible — observable input→output behavior must be -identical. - -**The adjudication log (§11) is mandatory.** Any place where old observable behavior -*cannot* be reproduced (the new core forbids the output, clamps a value the old core passed -through, rejects a file the old api accepted, or vice versa) is not a silent judgment call: -log it in §11 with the decision taken, and update api tests only when the new behavior is -deliberately accepted. - -**Why the vertical slice is non-negotiable:** the `MX_API=ON` error wall was measured -(2026-06: 74 translation units fail, every one a `fatal error` on a missing include — -old aggregate/per-element headers and ezxml). Because each TU dies at its first missing -include, the wall is *shallow*: it confirms the file inventory and reveals zero semantic -errors. There is no cheap semantic census; fallout only surfaces after includes and type -spellings are already translated. The slice (work order step 2) is the price-discovery -mechanism for the whole port. - -## 3. The `DocumentManager` rewrite - -The int-handle/singleton/mutex contract survives. The registry holds -`std::shared_ptr` (wrapping the new value type), which preserves the -`getDocument()` escape hatch byte-for-byte (`DocumentManager.h` itself declares the -`DocumentPtr` alias). ezxml is gone; load/save go through pugixml + -`mx::core::parse`/`mx::core::serialize`. - -**Error channel (owner decision): no exceptions escape; `Result`-style signatures; mx::api -owns its error vocabulary** (public headers must not include private `mx::core` headers; -mirroring keeps the layering and lets api errors carry api-level codes core has no business -knowing). - -```cpp -namespace mx::api { - -enum class ResultCode { - ioError, // file open/read/write failure (api-level) - xmlSyntaxError, // pugixml could not parse the bytes (api-level) - unknownElement, unknownAttribute, - missingRequiredElement, missingRequiredAttribute, - wrongElementOrder, tooManyElements, - invalidDocument, unsupportedVersion, // mirrored from core::ErrorCode - badDocumentId, // handle not in the registry (api-level) - internalError, // caught exception; nothing escapes (api-level) -}; - -struct ApiError { ResultCode code; std::string path; std::string message; }; -template class Result; // ok()/value()/error(), same shape as core's - // Result specialization for writes -} -``` - -| Method | New signature | Semantics | -|---|---|---| -| `createFromFile` | `Result` | pugixml load (`ioError`/`xmlSyntaxError`) → `core::parse` (core errors mirrored) → handle | -| `createFromStream` | `Result` | same, minus file I/O | -| `createFromScore` | `Result` | CAN now fail: bounded appends (beam > 8) etc. **Refuse, don't drop**: when a `ScoreData` describes something the new core won't represent, propagate the error; silently dropping user data is worse than refusing | -| `writeToFile` | `Result` | `badDocumentId` replaces the old silent no-op; `ioError` on write failure | -| `writeToStream` | `Result` | `badDocumentId` replaces the old silent no-op | -| `getData` | `Result` | `badDocumentId` replaces the old silent default-`ScoreData` return | -| `destroyDocument` | `void` (unchanged) | idempotent destruction stays a silent no-op — that is a feature | -| `getDocument` | `mx::core::DocumentPtr` (unchanged) | the documented escape hatch; tests depend on it heavily | -| `getUniqueId` | `int` (unchanged) | | - -Every public method body is wrapped; any `std::exception` from below becomes -`internalError`. `MX_THROW` disappears from the api layer. The mxl (compressed) -special-case message in old `createFromFile` survives as an `xmlSyntaxError` whose message -keeps the issue-66 hint. - -Fallout: every `mxtest/api` caller updates to the new signatures (mechanical), and the -ezxml usage in `ApiTester.cpp`/`PitchDataTest.cpp`/`SlurTest.cpp` is rewritten on pugixml. -The set of parseable files may shift (new core: strict names/structure, lenient values) — -adjudicated per-test in §11. - -## 4. Version semantics - -- **Write side: the root `version` attribute is always `"4.0"`, unconditionally.** mx does - not support writing documents at other versions (that would be a much bigger project); - emitting "3.0" from a 4.0 model was a fiction. `ScoreData::musicXmlVersion` no longer - influences output. Every test expectation asserting `version="3.0"` or an absent - attribute updates as adjudicated fallout. -- **Read side: verbatim preservation.** New field - `std::string ScoreData::declaredMusicXmlVersion` — exactly what the parsed root carried, - empty if absent. `// TODO: verify that we support the declared version` at the api read - seam (core already hard-rejects anything newer than 4.0 with `unsupportedVersion`; the - TODO covers classifying older/malformed declarations). -- The legacy `MusicXmlVersion` enum stays frozen (`unspecified`, `ThreePointZero`) for - source compatibility; the reader still sets `ThreePointZero` iff the declared string is - exactly `"3.0"`. Documented as read-side-informational only. - -## 5. Timewise ↔ partwise - -The old `core::Document::convertContents()` died with the old core; the new core has no -pivot. Port old `ScoreConversions.{h,cpp}` (165 lines at `71fc402`, a shallow-copy regroup -of parts-of-measures ↔ measures-of-parts plus header) into **`mx::impl`** -(`impl/ScoreConversions.{h,cpp}`), translated to value semantics: - -- Its only consumer is `DocumentManager`; the new core's charter (generated model + minimal - hand-written runtime) does not grow hand-written schema-shaped transforms. Promotion to - core would be a later, separate decision. -- The old mutate-and-restore dance in `getData()` (convert stored doc to partwise, read, - convert back) dies: convert into a local `ScorePartwise` copy, read that, stored document - untouched. Same observable behavior (`musicXmlType == "timewise"` round-trips). -- `createFromScore` with `musicXmlType == "timewise"`: build partwise via `ScoreWriter`, - pivot, store a timewise-rooted `Document`. -- The old pivot's shallow copies become real copies under value semantics — strictly safer, - behavior-identical for this use. - -## 6. `mx::utility` fates - -| Header | Fate | -|---|---| -| `Throw.h`, `StringToInt.h`, `Round.h` | keep as-is (live consumers) | -| `Parse.h`, `Enum.h`, `EnumWithString.h`, `JitAllocate.h`, `Lock.h` | delete (dead once `mxtest/import`'s harness goes; zero other refs) | -| `OptionalMembers.h` | rebuild for the new core's shape | - -`OptionalMembers.h` re-keying: the old macros SFINAE-detect public data members -(`attr.defaultX` + `attr.hasDefaultX` + `.getValue()`); the new core exposes member -functions returning `std::optional` (`el.defaultX()`, `el.setDefaultX(...)`). Same generic -application algorithm in `PositionFunctions`/`FontFunctions` etc.; the detection idiom -becomes C++20 `requires`-expressions (`if constexpr (requires { el.setDefaultX(v); })`). -`PositionFunctionsTest` (including the compile-only bogus-class case) pins it. - -General rule (owner): deleting genuinely dead code that follows from these decisions needs -no further sign-off. - -## 7. Test suite fates - -### 7.1 `mxtest/api` — preserved in full; THE acceptance suite - -All 30 test files + ScoreData headers + harness infra (`mxtest/file`, `mxtest/control`) -come back green. This includes the nine **ApiTester file-roundtrip tests** -(ApiChordSimple, ApiK007a, ApiK007c, ApiK009bSlur, ApiK014aFermatas, ApiK015aLayout, -ApiK016aMisc, ApiLy43e, ApiMuAccidentals1): file → `createFromFile` → `getData` → compare -full `ScoreData` against the hand-written expected values → `createFromScore` → write → -re-load → compare again. **Every file tested that way before is still tested that way.** -Changes are limited to: new `DocumentManager` signatures, ezxml→pugixml in the three files -that touch it, and §11-adjudicated expectation updates. - -### 7.2 `mxtest/impl` — port 7, delete 1 - -| File | Fate | -|---|---| -| `MagicTemplatesTest.cpp` | delete (17 lines, includes only, zero tests) | -| other 7 (CurveFunctions, DirectionReader, DirectionWriter, MeasureWriter, PositionFunctions, PrintFunctions, ScoreWriter) | port mechanically: same assertions, new core spellings | - -A ported test that *constructed* a state the new core forbids is adjudicated via §11, not -silently dropped. - -### 7.3 `mxtest/import` — old harness deleted; replaced by corert - -Owner-confirmed: `ImportTest*`, `ExpectedFiles*`, `ChangeValues*`, `SortAttributes*` are -deleted. Measured fact: that harness round-tripped `XDoc → core::Document::fromXDoc → -toXDoc` — the OLD CORE only; `ScoreData`/impl never appear. corert supersedes it strictly -(DOM comparison, numeric equivalence, sidecar fixups, three languages, pinned 829/0/0). -`Normalize.{h,cpp}` + `DecimalFields.h` stay (corert's). **`data/expected/` is deleted** -(its only reader was the dead harness). - -### 7.4 Examples - -`mx/examples/` (api-only consumers) compile and run; `examples-run` restored (§9 step 6). - -## 8. The corpus-wide api roundtrip (new gate) - -The critical test path the old tree never had corpus-wide: **file → api → impl → file**. -Flow per file: load → `createFromFile` → `getData` → `createFromScore` → write → compare -output document to input document, both normalized with corert's pipeline (`Normalize` + -DOM comparison with numeric equivalence). - -`mx::api` is deliberately a subset of MusicXML: files using unsupported features cannot -round-trip with fidelity, by design. The gate therefore uses a **pinned pass-list** -(owner decision), with two modes: - -- **Regression mode (CI):** run exactly the pinned list; every file must pass; zero - tolerance. Files off the list still run as load-survival. -- **Discovery mode (manual make target, never a CI gate):** run the whole corpus, report - which files newly pass/fail — the source of pass-list growth. Additions are deliberate - commits touching the manifest, never automatic. - -**Step 0 — baseline capture (BEFORE any porting; owner requires zero regression vs. the -old library):** - -1. Check out old `master` (`71fc402`) in a scratch worktree; build it with its own build - system (mx-sdk has the toolchain). -2. Run a one-off driver over the corpus: `createFromFile → getData → createFromScore → - writeToFile` — the old library's honest api roundtrip output per file (recording files - it cannot load). -3. Judge each output against its input with the NEW normalize-and-compare machinery (the - same comparator the Phase-2 gate uses — not the old string percent-match). -4. The passing files are the baseline. Commit the manifest - (`src/private/mxtest/api/roundtrip-baseline.txt`) and the capture driver's source. - -**The gate: every baseline file passes the new api roundtrip. Zero regression, hard CI -failure otherwise.** (Old master is a 3.0-era api: files using 3.1/4.0 features mostly land -off-list; the new system passing them later grows the list — progress, not noise.) -Complementary corpus-wide coverage: `ApiLoadSurvivalTest` (already in `mxtest/api`) -continues as the survival check over all files. - -## 9. Order of work and gating - -Constraint: the `mx` library only links when all 77 impl files compile, so staging is -deliberate. Gating rule (as Phase 1): **nothing advertised flips until step 5 is green.** - -- **Step 0 — baseline capture** (§8). First, before porting. -- **Step 1 — the seam:** `mx::api` Result/error types, `DocumentManager` rewrite, - `impl/ScoreConversions` port. These compile standalone against the new core. -- **Step 2 — the vertical slice:** translate `Converter` (the hub) + the PitchData path - (`NoteReader`/`NoteWriter` and dependencies), compiled via a scratch target, to validate - the §2 translation table and surface semantic fallout early. -- **Step 3 — the wave:** mechanically translate the remaining impl files (checklist: - Appendix A). Library compiles at the end of the wave. -- **Step 4 — suites in order:** `mxtest/impl` (unit pins) → `mxtest/api` (acceptance; §11 - log live) → examples build + run. -- **Step 5 — the corpus roundtrip:** build the harness (both modes), run discovery, verify - zero regression vs. the step-0 baseline, commit the pinned list. -- **Step 6 — flip the defaults:** `MX_API` default ON; restore `lib`/`dev`/`test`/ - `examples-run` Makefile targets; add CI `linux-api`; extend coverage and macos (§10). - -### 9.1 Progress (living section; tick Appendix A per file) - -- **Step 0 DONE** (commit `860cce7`): baseline captured from old master 71fc402; the pinned - manifest is `src/private/mxtest/api/roundtrip-baseline.txt` (exactly 1 file passes the - strict comparator — see `src/private/mxtest/api/baseline/README.md`). The shared - normalize/compare machinery lives in `mxtest/corert/Compare.{h,cpp}`; the judge builds - with `-DMX_BASELINE_JUDGE=on`. -- **Step 1 DONE** (commit `550b48c`): `mx/api/Result.h`, `ScoreData.declaredMusicXmlVersion`, - the DocumentManager rewrite (write boundary pins version="4.0" on a serialization-time - copy), `impl/ScoreConversions`, `impl/WriteRefusal.h` (writers throw it for bounded-append - refusal; `createFromScore` catches). -- **Step 2 DONE** (commit `c1ad11b`): Converter + NoteReader/NoteWriter + the - OptionalMembers/Position/Font/Print machinery. Slice findings: translation table held; - Converter tables are now `EnumMap` pair-vectors (wrapper enums have no ordering / no - public Tag ctor); variant choices bridge by nested `Kind` enums; NoteWriter builds - values then installs (`assembleNoteChoice`). -- **Steps 3–6 DONE**: ported files are ticked in Appendix A. The scratch target - `-DMX_IMPL_SLICE=on` (CMakeLists) compiles every ported TU as a static archive — add each - newly ported .cpp to its list. Canonical ported examples to imitate: `Converter.h/.cpp`, - `NoteReader.cpp`, `NoteWriter.cpp`, `PositionFunctions.h` + `utility/OptionalMembers.h`, - `CurveFunctions.h` (generic Slur/Tied handling), `TimeReader.cpp` (old `properties` is the - new core's `attributes`; old `TimeChoice::timeSignature` is `TimeChoice::Kind::group`), - `MetronomeReader.cpp` (old `BeatUnitPerOrNoteRelationNoteChoice` is `MetronomeChoice`, - beatUnitPer=Kind::group, noteRelationNote=Kind::group2). `MX_UNUSED` was rehomed to - `mx/utility/Unused.h`. Verify each TU with - `g++ -std=c++20 -fsyntax-only -I src/private -I src/include `; then build - `cmake -S . -B build/core-dev -DCMAKE_BUILD_TYPE=Debug -DMX_CORE_DEV=on -DMX_IMPL_SLICE=on` - and keep corert green (`build/core-dev/mxtest-core-dev`, 830 cases). `make fmt` - (MX_RUNNING_IN_DOCKER=1 natively) before committing. - - **Step-3 translation notes (from porting sessions):** - - `getAttributes()` is gone — attrs are direct optional members of the element class; pass - the element itself to `getPositionData()`, `getFontData()`, - `setAttributesFromMarkData()`, etc. - - Enum wrappers use factory constructors (`StartStop::stop()`) and `.tag()` comparisons; - there is no public Tag ctor, so `EnumMap` pair-vectors replace the old switch tables in - `Converter.cpp`. - - `DynamicsChoice` is a span of `Empty`/`OtherText` variants (one per dynamic mark), not a - single-value enum; `Dynamics::choice()` returns `std::span`. - - `FermataShape::emptystring` was renamed to `FermataShape::empty()` in the new core. - - `TupletPortion` replaces the old `TupletActual`/`TupletNormal` pair; - `TimeModificationGroup::normalType()` returns `NoteTypeValue` directly (not a wrapped - element ptr); `TimeModificationGroup::normalDot()` is a span. - - Credit writing: `FormattedTextID` words → `CreditChoiceGroup` → - `CreditChoice::group(group)`; reading: `choice().isGroup()` → - `group.choice().isCreditWords()` → `group.choice().asCreditWords().value()`. - - ArticulationsChoice, OrnamentsGroupChoice, TechnicalChoice all use `kind()` + `asXxx()` - dispatch; new 4.0 cases (softAccent, brassBend, flip, smear, open, halfMuted, harmonMute, - golpe, invertedVerticalTurn, haydn) are stubbed with `return false`. - - Mutable optional field pattern: `auto x = outer.field().value_or(T{}); modify(x); - outer.setField(x);` (copy-modify-set, no in-place mutation through optional). - -- **Still to port** (Appendix A unticked): NotationsWriter.cpp (its .h is ported), - DirectionReader/Writer, NoteFunctions, PropertiesWriter, MeasureReader/Writer, - PartReader/Writer, ScoreReader/Writer (.h files for the last two are ported; ScoreWriter - port should DROP the old version-echo switch — the write boundary pins "4.0" now, §4). - Then steps 4-6. - -## 10. CI plan - -- `linux-gen`, `linux-core`, `linux-targets`: unchanged (Makefile recipes build named - targets; `MX_API=ON` default does not drag api code into them). -- **New `linux-api`:** build `mx` + `mxtest` through mx-sdk; run the mxtest suites - (api/impl/file/control); run the corpus roundtrip in regression mode; build and run the - examples. -- **coverage:** the `--coverage` build now includes `mx` (api/impl/utility); mechanics - unchanged, table grows rows — satisfies the "coverage naturally grows" acceptance clause. -- **macos:** extends to build `mx` and RUN the api/impl suites natively with AppleClang - (owner-approved). Same Phase-1 principle: AppleClang fallout is our code, found here, - fixed in Phase 2. -- Discovery mode stays a manual make target, never a CI gate. - -End-of-phase definition of done: the branch tip drives this entire CI matrix green. - -## 11. Adjudication log (behavioral changes) - -Living section, updated during the port. Every entry: what the old behavior was, why it -cannot or should not be reproduced, the decision, and the tests updated. Pre-registered -entries from this design (details in the sections cited): - -| # | Change | Decision | Where | -|---|---|---|---| -| 1 | `createFromFile`/`createFromStream`/`createFromScore` returned `int`, threw on failure | `Result`, no exceptions escape | §3 | -| 2 | `getData`/`writeToFile`/`writeToStream` silently no-opped (or returned default `ScoreData`) on a bad handle | `badDocumentId` errors | §3 | -| 3 | `createFromScore` silently accepted unrepresentable `ScoreData` (e.g. 9 beams) | refuse with error; never drop data silently | §3 | -| 4 | root `version` attribute echoed `ScoreData::musicXmlVersion` (3.0 or omitted) | always `"4.0"` on write | §4 | -| 5 | declared version other than "3.0" was discarded on read | preserved verbatim in `declaredMusicXmlVersion` | §4 | -| 6 | the old `partwiseTimewise` pivot walked a part's measure iterator `currentMeasureIndex` steps with no bounds check: on a ragged score (parts with unequal measure counts) it dereferenced past-the-end — undefined behavior, not a reproducible output | the ported pivot bounds-checks and skips a part that has no measure at that index; behavior identical for every well-formed (equal-measure-count) score | §5, `impl/ScoreConversions.cpp` | -| 7 | `ChordData::miscData` (`vector`) was persisted as XML ProcessingInstructions on the `` element — a non-standard MusicXML extension that the old `mx::core` bolted on outside the XSD | the new generated core is strictly typed from the MusicXML 4.0 XSD and has no ProcessingInstruction mechanism; `miscData` is accepted on write but not emitted and not read back; `SaveProcessingInstructions` test updated to expect empty round-trip | `DirectionWriter.cpp`, `DirectionReader.cpp`, `ChordDataSaveAndLoadTest.cpp` | - -(Subsequent entries appended here as the port discovers them.) - -## 12. Deferred to Phase 3: the SMuFL glyph-name migration (open questions) - -Evidence gathered during this design session (so it is never re-derived): - -- The pre-4.0 workaround lives in the consumer (Komp, `MxNoteWriter::handleMarks`): SMuFL - canonical glyph names (e.g. `dynamicNiente`, accent-staccato/marcato-tenuto/ - marcato-staccato/tenuto-accent glyphs, `brassMuteClosed`) are smuggled as the TEXT - CONTENT of `other-articulation`/`other-technical`/`other-dynamics` via - `MarkData::name` + `MarkType::other*`. -- mx-side breadcrumbs: three `TODO - SMUFLKILL` comments (`impl/Converter.cpp`, - `impl/OrnamentsFunctions.cpp`, `impl/NotationsWriter.cpp`) sit exactly at the - `other-*` read/write sites; `MarkType::customAccentTenuto` round-trips through - `` with the sentinel string `"gjhsldjhglskdjhfg"` - (`MarkData.h:193`); `##misc-data##` footnote smuggling (`NoteData.h:123`) is a separate, - generic channel. -- The proper 4.0 home exists in the new core: the `smufl` attribute (type - `smufl-glyph-name`) on every `other-*` element. - -Open questions for the Phase-3 design session: - -1. Add `std::string MarkData::smufl`? (Sketch: read `smufl` attribute into it; write it - back as the attribute.) -2. Legacy-detection rule for migrating smuggled names: lexical shape vs. a vendored SMuFL - glyph-name lexicon vs. no auto-promotion (Komp migrates on its own side)? -3. Old-reader compatibility: must new-mx output remain readable by old Komp builds (emit - both text and attribute), or attribute-only (clean migration, Komp updates in lockstep)? -4. Fate of `customAccentTenuto`/`getMarkTypeFromCustomString` and the `SMUFLKILL` TODOs. - -Resolution: - -- Exact glyph names live in mark-specific `MarkDataChoice` payloads, not as another common - `MarkData` field. -- A compound dynamic owns its ordered standard and `other-dynamics` components; neighboring marks - are never interpreted as one dynamic. -- Text and `smufl` may coexist. mx does not promote legacy text to a SMuFL name automatically. -- The `customAccentTenuto` compatibility path remains unchanged and can be retired separately. - -## Appendix A: port checklist - -### A.1 `src/private/mx/api/` (4 of 13 .cpp touch core/ezxml) - -- [x] `DocumentManager.cpp` (the seam; §3) -- [x] `EncodingData.cpp` -- [x] `MarkData.cpp` -- [x] `SoundID.cpp` -- [x] public headers: add `mx/api/Result.h` (+`ApiError`), `ScoreData.h` version fields (§4) - -### A.2 `src/private/mx/impl/` translation units - -- [x] AccidentalMarkFunctions.cpp/.h -- [x] ArpeggiateFunctions.cpp/.h -- [x] ArticulationsFunctions.cpp/.h -- [x] Converter.cpp/.h (slice, step 2) -- [x] Cursor.cpp/.h -- [x] CurveFunctions.h -- [x] DirectionReader.cpp/.h -- [x] DirectionWriter.cpp/.h -- [x] DynamicsReader.cpp/.h -- [x] DynamicsWriter.cpp/.h -- [x] EncodingFunctions.cpp/.h -- [x] FermataFunctions.cpp/.h -- [x] FontFunctions.h -- [x] LayoutFunctions.cpp/.h -- [x] LcmGcd.cpp/.h -- [x] LineFunctions.h -- [x] LyricType.cpp/.h -- [x] MarkDataFunctions.h -- [x] MeasureCursor.h -- [x] MeasureReader.cpp/.h -- [x] MeasureWriter.cpp/.h -- [x] MetronomeReader.cpp/.h -- [x] MxVersionDefines.h -- [x] NonArpeggiateFunctions.cpp/.h -- [x] NotationsWriter.cpp/.h -- [x] NoteFunctions.cpp/.h -- [x] NoteReader.cpp/.h (slice, step 2) -- [x] NoteWriter.cpp/.h (slice, step 2) -- [x] OrnamentsFunctions.cpp/.h -- [x] PageTextFunctions.cpp/.h -- [x] PartReader.cpp/.h -- [x] PartWriter.cpp/.h -- [x] PositionFunctions.h -- [x] PrintFunctions.h -- [x] PropertiesWriter.cpp/.h -- [x] ScoreConversions.cpp/.h (NEW, ported from old core; §5) -- [x] ScoreReader.cpp/.h -- [x] ScoreWriter.cpp/.h -- [x] SlideFunctions.cpp/.h -- [x] SpannerFunctions.h -- [x] StaffFunctions.cpp/.h -- [x] TechnicalFunctions.cpp/.h -- [x] TimeReader.cpp/.h -- [x] TupletReader.cpp/.h - -### A.3 `src/private/mx/utility/` (§6) - -- [x] delete `Parse.h`, `Enum.h`, `EnumWithString.h`, `JitAllocate.h`, `Lock.h` -- [x] rebuild `OptionalMembers.h` (C++20 requires) - -### A.4 tests and infra - -- [x] delete `mxtest/import/{ImportTest*,ImportTestImpl*,ExpectedFiles*,ChangeValues*,SortAttributes*}`, `data/expected/` -- [x] `mxtest/impl`: delete MagicTemplatesTest.cpp; port the other 7 -- [x] `mxtest/api`: signature fallout (all files); ezxml→pugixml in ApiTester.cpp, PitchDataTest.cpp, SlurTest.cpp -- [x] baseline capture driver + `roundtrip-baseline.txt` (step 0) -- [x] corpus api roundtrip harness (discovery + regression modes) -- [x] examples compile/run -- [x] CMake: `MX_API` default ON (step 6); Makefile: restore `lib`/`dev`/`test`/`examples-run` -- [x] CI: `linux-api` job; coverage + macos extension diff --git a/docs/ai/design/newgen-integration-plan.md b/docs/ai/design/newgen-integration-plan.md deleted file mode 100644 index bf9b25199..000000000 --- a/docs/ai/design/newgen-integration-plan.md +++ /dev/null @@ -1,278 +0,0 @@ -# newgen Integration Plan - -Status: APPROVED. This is the binding decision record for integrating the newgen work -(the generator pipeline + the generated `mx::core`) with the old codebase on `master`. -It was produced by studying both trees and interviewing the owner; the decisions below -are settled. Execute Phase 1 from this document. - -## 1. Situation - -Three commits define the problem: - -| Ref | What it is | -|---|---| -| `71fc402bd1d89c` | `master` HEAD. Master has NOT moved since the newgen branch-off; this commit IS the merge-base. | -| `b01288cd01629c` | "initial commit" of the newgen effort: deleted 2,073 files (old `mx::core`, `mx::api`, `mx::impl`, ezxml, old tests, xcode, docs, …) and hollowed out README/AGENTS/CMake/Dockerfile/Makefile/CI so codegen work could proceed without distracting context. These deletions must NOT appear in the integrated history. | -| `ba4916f` (tip of `claude/newgen-branch-checkout-b8i3jg`) | The completed newgen work: the language-agnostic generator (`gen/`), the generated C++ `mx::core` (valid-by-construction, pugixml, all six gates green per `docs/ai/design/mx-core-plan.md`), Go/C/JSON-Schema targets, the corert/validate/probe/unit gates, mx-sdk toolchain. | - -The newgen branch is a strict descendant of master, but merging it as-is would carry -`b01288c`'s devastating deletions onto master. This plan replaces that merge with a -**rebuilt branch**: new branch `newgen-integration` created at `71fc402`, followed by a -logical, curated sequence of fresh commits whose trees are assembled from the two -sources (`git checkout -- `). The only deletions in the new history are -the *intentional* ones (§3). No merge to master is performed; the deliverable is a -branch that is ready for a PR. - -## 2. The product stack and the phases - -``` -mx/api -> preserved; changes must be minimally invasive (Phase 2 test-harness fallout only) -mx/impl -> preserved but excluded from build; Phase 2 reimplements it against the new mx::core -mx/core -> the old hand-written core is DELETED and replaced by the generated core -``` - -- **Phase 1 (this plan): combine the branches.** New branch off master containing the - newgen work plus everything preserved from the old codebase. `mx::api`/`mx::impl` are - restored in-tree but excluded from the build behind `option(MX_API ... OFF)`. It is - accepted that they do not compile yet. All newgen gates are green at the tip; the - branch is PR-ready. -- **Phase 2 (follow-up, outline in §8): the `mx::impl` port.** Reimplement impl against - the new core's shape, rewrite the api seam (`DocumentManager`) on pugixml/`mx::core::parse`, - bring the preserved `mxtest/api` + `mxtest/impl` suites back to green, flip `MX_API` - default ON. Opens with its own design doc. -- **Phase 3 (theoretical): distribution.** Reinstate iOS/macOS (Xcode project, - xcframework) with fresh eyes, and consider distribution channels / package managers. - Nothing in Phase 1 or 2 blocks on it. - -## 3. Disposition manifest - -Every path `b01288c` touched, with its settled fate. "old" = take from `71fc402`, -"new" = take from `ba4916f`. - -### Deleted intentionally (the only deletions allowed vs master) - -- `src/private/mx/core/**` (old hand-written core, 1,247 files) — replaced by the - generated core. Its 522-file test suite `src/private/mxtest/core/**` (old) dies with - it (tightly coupled to the old core API, and it took forever to compile). -- `src/private/mx/ezxml/**` (35 files) — retired; the new core consumes pugixml - directly. Outside the old core, ezxml is referenced only by `DocumentManager.cpp` and - old test-harness files, all of which are excluded from the build until Phase 2 - rewrites those seams (§8). Restoring code that references a deleted library is - harmless because it is not compiled. -- `src/private/mxtest/corert/**` (old, 5 files) — superseded by the newgen pugixml port - at the same paths. -- `src/private/mxtest/import/{ImportTest*,ExpectedFiles*,ChangeValues*,SortAttributes*}` - remain *restored-but-excluded* (see below), but old `DecimalFields.h` loses to the - newgen pugixml version at the same path. -- `gen/parse.py`, `gen/ids.py`, `gen/generate.py` (the old generator prototype) — - replaced by the `gen/xsd|ir|plates|press` pipeline. -- `docs/ai/projects/**` — deleted (do not resurrect; restoring it would also duplicate - the 154 synthetic corpus files that were *moved* to `data/synthetic/`). -- `.idea/**` — deleted. -- `xcode/**` (incl. `build_xcframework.sh`) — deleted; deferred to Phase 3. -- `CHANGELOG.md` — stays out; will be dealt with at release time. - -### Restored from `71fc402` (excluded from the build until Phase 2 where noted) - -- `src/include/mx/api/**` (55 public DTO headers; zero core/ezxml references). -- `src/private/mx/api/**` (13 files; only 4 touch core/ezxml — `DocumentManager.cpp` - is the real seam). -- `src/private/mx/impl/**` (77 files; welded to the old core API; Phase 2's subject). -- `src/private/mx/utility/**` (9 headers; used by impl). -- `src/private/mx/examples/**` (3 files; api-only consumers). [excluded] -- `src/private/mxtest/api/**` (43 files) — the Phase-2 acceptance suite. [excluded] -- `src/private/mxtest/impl/**` (8 files) — impl's unit spec; Phase 2 ports or retires - per-file. [excluded] -- `src/private/mxtest/import/` old harness files (ImportTest/ExpectedFiles/ - ChangeValues/SortAttributes) — Phase-2 reference for the api-level corpus roundtrip; - to be rewritten on pugixml. [excluded; coexists with newgen's `Normalize.{h,cpp}` and - `DecimalFields.h` in the same directory] -- `src/private/mxtest/file/**` (10) + `src/private/mxtest/control/**` (1) — harness - infra the api tests depend on. [excluded] -- `gen/quality.py`, `gen/.pylintrc` — the generator quality gates, restored and adapted - (§5.3). - -### Taken from `ba4916f` (the newgen tree, wholesale) - -- `gen/**` (pipeline, targets, tests, committed Go/C/schema generated output). -- `src/private/mx/core/**` (hand-written runtime) + `src/private/mx/core/generated/**` - (the generated model, 910 files) — the directory IS the generated/hand-written boundary. -- `src/private/pugixml/**` (vendored 1.15). -- `src/private/mxtest/{core,corert,import,probe,validate}/**` (the new gates; new - `mxtest/core` is the new core's unit suite, unrelated to the deleted old one). -- `src/private/cpul/**`, `data/**` (incl. `data/synthetic/` moves and sidecars), - `docs/musicxml-*.xsd` and `docs/sounds-*.xml` (the renames stand), - `docs/ai/design/**`, `.gitignore` (union with old entries if any matter). - -### Merged / rewritten by hand - -- `README.md` — start from `git show 71fc402:README.md` and make the **smallest - possible diff**: preserve the original at every sentence and word where preservation - is possible. Update only what is factually wrong now: C++17 → C++20, supported - MusicXML version (new core generates from 4.0), the old build-mode table where - targets no longer exist, and an honest status note that `mx::api` is present but - excluded from the build pending the Phase-2 port. Add a brief generator section. -- `AGENTS.md` — **rewrite the original** (`git show 71fc402:AGENTS.md`), folding in as - much of the newgen AGENTS.md information as possible. The newgen version is heavily - biased toward codegen, but that is not what the project is solely about: the project - is a MusicXML C++ library (`mx::api` is the product surface) that now contains a - generator. Remove the "Ignore git history prior to b01288" instruction — history is - meaningful again. Document the phase state and the `MX_API` flag. -- `CMakeLists.txt`, `Makefile`, `Dockerfile`, `.github/workflows/ci.yaml` — §5. - -## 4. Branch construction - -1. `git branch newgen-integration 71fc402` (the name is not load-bearing). -2. Build each commit in §6 by checking out path sets from the two sources and applying - the hand-merged files. Author fresh commit messages; do not cherry-pick the old - history. -3. **Verification invariant (run before declaring done):** - `git diff --name-status 71fc402 newgen-integration` — every `D` line must appear in - §3's intentional-deletion list. Anything else deleted is a bug. -4. A second useful invariant: `git diff ba4916f newgen-integration` should show only - restorations (api/impl/utility/tests/examples/public headers), the hand-merged - build/docs files, and the gen-quality/lint restoration. -5. Push with `git push -u origin newgen-integration`. Do NOT merge to master; do not - open a PR unless asked. - -**Commit-size rule:** large blasts of generated or vendored code are isolated in their -own commits so that every other commit is reviewable in a differ / on GitHub without -crashing it. Blast commits contain nothing a human needs to read. - -**Green bar:** the TIP must be green (all gates in §7). Intermediate commits in the -sequence are allowed to be transiently unbuildable; do not introduce artificial CMake -churn to make each slice build independently. - -## 5. Build system and CI specifics - -### 5.1 CMake - -Base: the newgen `CMakeLists.txt` (C++20, pugixml, `mx_core`, gated test binaries, -`MX_CORE_DEV` section). Add: - -- `option(MX_API "build the mx::api/mx::impl product library and its tests (Phase 2)" OFF)` - wiring the restored sources (api, impl, utility, examples, mxtest api/impl/file/ - control/old-import) into the `mx` static library + `mxtest` binary, with a comment - marking it Phase-2. Default OFF; it is expected NOT to compile until the port. The - point is that the port has a build home from day one. -- Restore `option(MX_COVERAGE ...)` (`--coverage` flags, non-MSVC guard) from the old - CMakeLists, applied to the new-core targets. -- The whole project is C++20. No dual-standard build. - -### 5.2 Makefile - -Base: the newgen Makefile. Changes: - -- Lean on mx-sdk: add Docker-gated wrappers for the C++ core gates - (`core-dev`, `test-core-dev`, `test-cpp-unit`, `validate-cpp`, `probe-cpp`) following - the existing `gen`/`test-go` pattern, so CI and the documented workflow run everything - through the pinned toolchain. Native in-container/dev-machine invocation keeps working - (the `MX_RUNNING_IN_DOCKER` switch already exists). -- Restore `coverage-core-dev` (old mechanism, new subject: `--coverage` build of - `mx_core` + corert + unit suites, gcovr table) and `check-core-dev` if applicable. -- Restore `gen-quality` and `gen-lint` targets from the old Makefile (§5.3). -- Do NOT restore `lib`/`dev`/`core`/`test`/`test-all`/`examples-run`/`xcode-*` in - Phase 1 — no advertised target may be broken. They return in Phase 2 (Phase 3 for - xcode) when they can actually build. -- `core-dev`/`test-core-dev` keep their newgen meaning (the new core). This reuses the - old names for the same intent: codegen iteration on the core. - -### 5.3 Generator quality gates - -Restore `gen/quality.py` + `gen/.pylintrc` from `71fc402`, adapted: point the analyzed -file set (`GEN_PY`) at the new `gen/` tree, then **re-baseline the floors** — run once, -record the current composite as `GEN_QUALITY_FLOOR` and the current pylint rating as -`GEN_LINT_FLOOR` (floor = current score; ratchet later). Restore the pinned analyzer -venv to the Dockerfile (radon, astroid, pylint, cognitive_complexity — same pins as old -master unless they fail to install, in which case re-pin and say so in the commit). - -### 5.4 Dockerfile (mx-sdk) - -Base: the newgen Dockerfile (Ubuntu 24.04, GCC 14, Go, libxml2-dev, clang-format-18, -ccache). Add back from old master: `gcovr`, the quality venv (§5.3). Add -`libxml2-utils` (xmllint, needed in-container by `validate-cpp` once it is -Docker-gated). - -### 5.5 CI (`.github/workflows/ci.yaml`) - -Rebuilt from scratch (the newgen stub is stale — it still references ezxml). All linux -jobs run through mx-sdk. Reuse the old workflow's mechanics for ccache/build-volume -caching and PR comments. - -| Job | Contents | -|---|---| -| `linux-gen` | fmt-check; `make test-gen`; `plates --check` (all targets); regen-then-`git diff --exit-code` (committed generated output must match the generator); `gen-quality` + `gen-lint`, with the quality-report PR comment from the old workflow | -| `linux-core` | `test-core-dev` (corert **829/0/0 pinned**), `test-cpp-unit`, `validate-cpp`, `probe-cpp` | -| `linux-targets` | `test-go`, `test-c` | -| `coverage` | `coverage-core-dev` over the new core, gcovr table, HTML artifact, PR comment — same mechanism as old master | -| `macos` | native (no Docker): build `mx_core` + run unit and corert suites with AppleClang. The new core has only ever been compiled with GCC 14; AppleClang fallout is found here and **fixed in Phase 1** (it is our code) | - -## 6. Commit sequence - -Content assembled per §3/§4; messages are fresh; blast commits marked **[blast]**. - -1. **`gen:` replace the generator prototype with the pipeline.** Delete - `parse.py`/`ids.py`/`generate.py`; land `gen/` pipeline + target configs/templates + - gen tests; restore+adapt `quality.py`/`.pylintrc` (floors re-baselined); corpus moves - (`data/`, incl. `data/synthetic/`), XSD/sounds renames in `docs/`; - `docs/ai/design/` docs. Excludes committed generated output. -2. **`gen:` generated target output [blast].** `gen/test/go/mx/`, `gen/test/go/vendor/`, - `gen/test/c/mx/`, `gen/schema/out/`. -3. **`core:` vendor pugixml 1.15 [blast].** `src/private/pugixml/`. -4. **`core:` replace mx::core.** Delete old `src/private/mx/core/**` and - `src/private/mx/ezxml/**`; land the hand-written runtime (`src/private/mx/core/*`) - and `src/private/mxtest/{core,corert,import,probe,validate}` (new gates), `cpul`. - (Old `mxtest/core` + old corert deleted here.) -5. **`core:` the generated model [blast].** `src/private/mx/core/generated/` (910 files). -6. **`api:` preserve mx::api/mx::impl behind MX_API.** Restore public headers, api, - impl, utility, examples, mxtest api/impl/file/control/old-import per §3; CMake - `MX_API=OFF` wiring. -7. **`build:` merged CMake/Makefile/Dockerfile.** §5.1–5.4 (whatever of CMake/Makefile - wasn't already needed by commits 4–6 to keep the tip story coherent; transient - unbuildability of intermediate commits is acceptable). -8. **`ci:` rebuilt workflow.** §5.5. -9. **`docs:` README minimal-diff revision; AGENTS.md rewritten from the original;** - this plan rides along if not already present. - -## 7. Phase-1 acceptance gates (the tip must pass ALL) - -1. `git diff --name-status 71fc402 `: every deletion is in §3's intentional list. -2. `make test-core-dev` — corert 829/0/0 (pinned-counts case enforces it). -3. `make test-cpp-unit`, `make validate-cpp`, `make probe-cpp` — green. -4. `make test-gen`; `plates --check` for all four targets; regen all targets then - `git diff --exit-code` (zero drift). -5. `make gen-quality`, `make gen-lint` — at or above the re-baselined floors. -6. `make test-go`, `make test-c` — 777/0/52 each. -7. `make check` (fmt) — clean. -8. CI workflow is syntactically valid and its jobs map 1:1 onto the targets above. -9. README/AGENTS.md done per §3 ("merged / rewritten by hand"). -10. Branch pushed (`git push -u origin newgen-integration`). NO merge to master; NO PR - unless the owner asks. - -## 8. Phase 2 outline (the `mx::impl` port) — scope only, opens with its own design doc - -- **Goal:** `mx::api` works again, implemented via `mx::impl` against the new - `mx::core`; api changes minimally invasive (test-harness fallout only). -- **Acceptance:** preserved `mxtest/api` + `mxtest/impl` suites ALL GREEN; `MX_API` - default ON; `lib`/`dev`/`test`/`examples-run` Makefile targets restored; coverage job - naturally grows to cover api/impl. -- **Known seams (inventoried during planning):** - - `src/private/mx/api/DocumentManager.cpp` — the api↔core boundary; rewritten from - ezxml `XDoc` to `mx::core::parse(const pugi::xml_document&) -> Result` - and the serializer. - - ezxml-referencing files needing rewrite: `DocumentManager.cpp`, - `mxtest/api/{ApiTester,PitchDataTest,SlurTest}.cpp`, and the old `mxtest/import` - harness (rebuild the api-level corpus roundtrip on pugixml; prefer newgen's - `Normalize.cpp`; old `SortAttributes`/`ChangeValues` likely retire). - - Semantic fallout to adjudicate per-test: the new core clamps values on - construction, dispatches choices by variant Kind, enforces schema order - structurally, and returns `Result` from bounded appends — api tests that relied on - old-core leniency or error behavior will need deliberate decisions. - - `mxtest/impl` (8 files) assert old-core types directly: port or retire per-file. -- **Out of scope for Phase 2:** xcode/xcframework/iOS/macOS distribution (Phase 3). - -## 9. Phase 3 (theoretical) - -Reinstate iOS/macOS support with fresh eyes (the old checked-in Xcode project may not -have been working correctly anyway) and consider distribution channels and package -managers. Nothing earlier depends on it; nothing here constrains it. diff --git a/docs/ai/design/release-process.md b/docs/ai/design/release-process.md deleted file mode 100644 index 988bcfaf4..000000000 --- a/docs/ai/design/release-process.md +++ /dev/null @@ -1,438 +0,0 @@ -# Release process design - -Status: IMPLEMENTED. This is the design record for `mx`'s GitHub Actions release system. It was -produced from a deep-research pass on how C++/Swift libraries do continuous delivery on GitHub, -followed by an owner interview, then revised twice: once after a design review found mechanical -flaws in the first draft, and once for the owner requirement that release tags live on `main` -(§13 records each change and why). The workflows in §4 exist and this document is kept in sync -with them; §13's implementation entry records where the build drifted from the design text. - -## 1. Goal - -Ship binary releases of `mx` for multiple package ecosystems from a chosen commit, with a -provable guarantee that what gets published is exactly what was built and validated — never a -silent rebuild from a different tree. Swift (a binary XCFramework consumed via SwiftPM) is the -first release target. Conan is a planned second target; the architecture must not be Swift-only. -Release tags are part of `main`'s history (owner requirement): every published version is -reachable from `main`, so `git log`, `git describe`, and `git tag --merged main` all see it. - -The design deliberately assembles this from GitHub primitives rather than copying a precedent: -the deep-research pass found no real-world project running this exact two-phase pattern (phase 1 -builds a chosen SHA and writes a tag-ready release commit; phase 2's tag push re-verifies that -commit from primary sources and publishes). Most projects do single-phase (tag-triggered build -+ publish, or a `workflow_dispatch` version-bump). The two-phase split exists here to make the -"published bytes == validated bytes" invariant airtight. - -## 2. Human touchpoints - -The entire release is four manual actions. Everything else is automation reacting to events. - -1. **(Optional) Open and merge a PR** for any real work the release needs. → commit `A` on `main`. -2. **Run `release-build`** (a `workflow_dispatch`), passing SHA `A` and the version: - `MAJOR.MINOR.PATCH`, or `MAJOR.MINOR.PATCH-prerelease` for a release candidate (§7). -3. **Review and merge the auto-opened release PR** (branch `release-prep/v`), using - **"Create a merge commit"** — not squash or rebase. §3 explains why the merge method matters; - §5 enforces it. -4. **Push a tag** (`v` followed by that same version) pointing at commit `B` — the first of - the PR's two commits, whose SHA the run summary and the PR body both print alongside the - exact `git tag`/`git push` commands. - -No other human step exists. In particular there is no "click publish on a draft," no manual -checksum edit, and no reviewer-approval gate beyond the tag push itself — pushing the tag *is* -the approval, and the review in step 3 is what the tag pusher is vouching for. - -`release-build` can be triggered three interchangeable ways, since `workflow_dispatch` is one -API event: the GitHub Actions UI ("Run workflow" → `sha` and `version` fields), the `gh` CLI -(`gh workflow run release-build.yml -f sha=<40-hex> -f version=1.4.0`), or the GitHub MCP tool -(`actions_run_trigger`, `method: run_workflow`, `inputs: { sha, version }`). In all three, the -dispatch `ref` selects only *which copy of the workflow YAML runs* (normally `main`); the commit -actually built is the `sha` input, which the job checks out explicitly. - -## 3. The checksum constraint (why `B` exists, and where it sits) - -SwiftPM resolves a version by checking out the *tagged commit* and reading `Package.swift` as it -exists there. For binary-release mode to work for a consumer depending on `from: "1.4.0"`, the -`Package.swift` at the `v1.4.0` commit must already contain the correct -`binaryTarget(url:checksum:)`. The URL is predictable ahead of time — it is a fixed function of -the tag name and a fixed asset filename, which is exactly why the version must be supplied at -build time (§7). The **checksum is not** — it can only be computed after the XCFramework is -built. - -A commit cannot contain the hash of its own future build output; that is a property of content -hashing, not a fixable limitation. Therefore *some* commit must exist between "the SHA the owner -picked" (`A`) and "the SHA that gets tagged" (`B`), carrying the checksum that `A`'s build -produced. - -**The shape that puts `B` on `main` without putting the binary manifest on `main`:** the release -PR that `release-build` opens carries exactly two machine-written commits on branch -`release-prep/v`, based on `A`: - -- **`B`** (parent: `A`) — writes the binary-default release `Package.swift` (§8) and - `release-manifest.json` (§5). This is the commit the tag points at. -- **`R`** (parent: `B`) — restores `Package.swift` to `A`'s copy verbatim - (`git checkout A -- Package.swift`; purely mechanical, no target involvement), keeping - `release-manifest.json` as a visible release record on `main`. - -Merging the PR with a merge commit `M` makes `B` part of `main`'s history — the same way every -commit of every merged feature branch is on `main` — while `main`'s *tree* changes only by the -release-record file. This resolves the conflict that sank both earlier drafts: a consumer's -SwiftPM evaluates `Package.swift` in the consumer's own environment, and there is no way to make -every downstream consumer set an environment variable, so the tagged commit must resolve to the -`binaryTarget` **by default**; meanwhile every tip of `main` must default to a source build for -sibling-checkout development and the non-Apple CI consumers. One file cannot default both ways -at one commit — so the binary default exists at exactly one commit, `B`, and `R` closes it out -before the branch meets `main`. - -Two properties fall out of `B` having parent `A` (rather than being the merge or squash commit, -as in the first draft): - -- *No frozen `main`.* Whatever lands on `main` between `A` and the merge, `B`'s tree is still - exactly "`A` + declared release metadata," so reusing `A`'s build artifacts for `B` stays - sound. Concurrent merges can at most conflict on `release-manifest.json`, which nothing else - touches. (The first draft tagged the merge commit itself, which silently required `main` to be - frozen for the duration of every release.) -- *The PR's net diff is tiny.* GitHub's "Files changed" view (merge-base `A` to head `R`) shows - only `release-manifest.json`; the binary manifest appears in the commits tab at `B`, which the - run summary and PR body link directly for review of the URL/checksum lines. - -**The merge method matters.** Only a true merge commit preserves `B` in `main`'s -ancestry. A squash or rebase merge copies the net diff instead — harmless to `main`'s tree (the -net diff is the record file), but `B` would then be reachable only via the tag, violating the -tags-on-`main` requirement. `release-publish` hard-fails in that case (§5, check 3); recovery is -`git merge -s ours release-prep/v` pushed to `main` — a history-only merge whose tree -change is nil, since `main` already carries the net diff — then re-push the tag. - -**All release-metadata edits from all targets are bundled into the single commit `B`.** If a -future target (Conan) has an analogous "the tagged commit must already carry a computed value" -gotcha, its files go into `B` too — not a second commit or PR. `B` is the one and only place -release metadata is written into a tree. - -Because `B` differs from `A` only in declared release-metadata files (never `src/`), `A`'s build -output is byte-identical to what a build at `B` would produce. The system relies on this: it -does **not** rebuild at `B`; it publishes `A`'s already-built artifacts for `B` (§5). - -## 4. Workflows - -All new, all separate from `ci.yaml`. Names are indicative. - -| File | Trigger | Responsibility | -|---|---|---| -| `release-build.yml` | `workflow_dispatch`, inputs `sha` (required; validated against `^[0-9a-f]{40}$`) and `version` (required; validated as SemVer 2.0 with an optional pre-release suffix and no build metadata, §7) | Checks out `sha` explicitly. Calls one reusable target workflow per release target. Uploads each target's artifacts (`retention-days: 90`). A final prep job collects every target's metadata files, writes `release-manifest.json` (§5), commits `B` then `R` on `release-prep/v` (force-push; the branch is machine-owned), opens the release PR, and prints `B`'s SHA, the tag commands, and the `A...B` compare link in the run summary and the PR body. | -| `_release-target-swift.yml` | `workflow_call` | Builds three slices via CMake + an iOS toolchain file: macOS universal (`arm64`+`x86_64`), iOS device (`arm64`), iOS simulator (`arm64`+`x86_64`). Stitches them with `xcodebuild -create-xcframework`, zips, computes the SHA256 via `swift package compute-checksum`. Outputs: artifact name, checksum, the complete release-mode `Package.swift` content (§8), and the list of paths it is allowed to occupy in `B`. | -| `_release-target-conan.yml` | `workflow_call` | *Future.* Same input/output contract as the Swift target (SHA + version in; artifact + metadata files + allowed-path list out). Not built now; its existence in this table is the proof the abstraction is not Swift-only. | -| `release-publish.yml` | `push: tags: ["v*"]` | Resolves the tag to `B`, reads `release-manifest.json` from `B`'s tree, and re-verifies every claim in it (§5). Hard-fails on any mismatch. Otherwise downloads the pinned run's artifacts and publishes a GitHub Release immediately (not draft), `generate_release_notes: true`, attaching each target's archive and its `.sha256`. | - -No workflow reacts to the release PR's merge; merging only places `B` into `main`'s history, and -all validation happens at tag time. (The first draft had a `release-prep-merged.yml` for this; -it is gone, along with its label machinery.) - -**Implementation notes.** Each target hands its results to the prep job through a companion -artifact, `mx-release--metadata-`: a `tree/` directory holding the exact files to -commit in `B` (for Swift, the interpolated release `Package.swift`) plus `fragment.json` (target -name, artifact name, archive filename, checksum, platforms, allowed paths) — the concrete form -of the §9 contract's outputs (b) and (c). The prep job merges every target's `tree/`, builds -`release-manifest.json` from the fragments, and refuses to commit if anything outside the -declared paths is dirty. The prep job pushes the branch and opens the PR with the workflow's own -`GITHUB_TOKEN`, and GitHub suppresses workflow triggers for events created with that token — so -**the release PR runs no CI**. That is acceptable by design: CI validated `A` on its own PR, -`release-build` built and checksummed `A`, and the release PR's content is machine-written -metadata. Two repository settings are prerequisites: **"Allow GitHub Actions to create and -approve pull requests"** (Settings → Actions → General) must be on, and **"Create a merge -commit"** must be an allowed merge method (§3). - -One bootstrap consequence of the `push: tags` trigger: GitHub runs the workflow YAML as it -exists *at the tagged commit* — i.e. `B`, whose tree is `A` plus metadata. So only SHAs that -already contain `release-publish.yml` are releasable. Acceptable; no escape hatch needed. - -## 5. The SHA-binding invariant - -Phase 2 (`release-publish`) must publish exactly the bytes phase 1 validated. Two design choices -enforce this: - -- **No rebuild.** `release-publish` never compiles anything. It fetches an already-built artifact - by workflow-run ID and publishes it verbatim. This removes rebuild drift as a possible failure - mode entirely. (Build-provenance attestation — `actions/attest-build-provenance`, - SLSA/Sigstore — was evaluated and rejected for this role: the research found no confirmed - mechanism by which it proves phase 2's artifacts equal phase 1's. Artifact download by run ID - is the reliable mechanism.) -- **`B` carries its own provenance, and publish re-derives every claim from primary sources.** - `release-manifest.json`, committed in `B` by the prep job, records: the version, the base SHA - `A`, the `release-build` **run ID** (`github.run_id` is available to a run while it executes, - so the run that builds the artifacts can pin itself into `B`), each target's artifact name and - checksum, and the union of paths the targets declared they may touch. `release-publish` trusts - none of it blindly; at tag time it hard-fails unless **all** of the following hold: - 1. the tag name equals `v` + the manifest's version (so the asset URL baked into - `Package.swift` can never desync from the tag actually pushed); - 2. the tagged commit has exactly one parent and it is the manifest's `A` — which also catches - tagging the wrong commit: `R` fails it (its parent is `B`), as does the merge commit (two - parents), and `A` itself has no manifest at all; - 3. `A` is an ancestor of `main` (releases come from mainline history only) **and `B` is an - ancestor of `main`** — the tags-on-`main` guarantee, which requires the release PR to have - been merged, with a true merge commit, before the tag is pushed (§3 gives the recovery when - it was squashed); - 4. the `A..B` diff touches only the declared paths — additionally clamped by a static ceiling - hardcoded in `release-publish.yml` itself (release metadata paths only; never `src/`, - never `.github/`); - 5. the pinned run exists **in this repository**, is a run of `release-build.yml`, and - completed successfully; - 6. its artifacts download, and a freshly recomputed SHA-256 of each archive equals both the - manifest's checksum and the checksum string inside `B`'s `Package.swift` (for the Swift - target). For a zip, `swift package compute-checksum` *is* the SHA-256 of the file, so plain - `sha256sum` on the publish runner reproduces it. - -`R` and the merge commit are deliberately *not* validated at publish: they affect only `main`'s -tree, which the PR review in step 3 governs. - -Why not query the Actions run history by commit, as the first draft did? Two API facts make that -unsound: for a `pull_request`-triggered run, the Actions API reports `head_sha` as the *PR -branch head*, never the merge commit — so the draft's lookup ("the validating run whose -`head_sha` is the tagged SHA") could never match anything; and a `workflow_dispatch` run cannot -be located by its `sha` *input* at all (inputs are not queryable, and such a run's `head_sha` is -the dispatch ref's head, not the input). Pinning the run ID inside `B` solves both discovery -problems with no heuristics — including the draft's "most recent successful run wins" -tie-break, which is gone: re-dispatching the same version force-pushes a fresh branch, and the -tag pins exactly one `B`. - -Residual trust, stated honestly: someone with push access could hand-craft a `B` (including, -since tag-push workflows execute the YAML at the tagged commit, a `B` that tampers with -`release-publish.yml` itself — which check 4 forbids, but a tampered workflow would not enforce -check 4). Checks 1–6 make such a commit fail loudly against any honest workflow copy; what -closes the loop is that nothing publishes until an admin reviews the release PR (step 3, where a -workflow edit is glaring) and pushes the protected tag. That is the same trust anchor the first -draft placed in "review and merge the PR" — unchanged, since this design kept that step. - -## 6. Artifacts and retention - -`release-build` uploads, per target, a single named artifact **`mx-release--`** -(e.g. `mx-release-swift-`; the target name is in the key so a future Conan target cannot -collide with Swift's): the built archive (the XCFramework zip), its `.sha256` file, and a small -manifest fragment recording the SHA and the platforms/arches built. Retention is set explicitly -to **`retention-days: 90`** rather than inherited from the repo default, so the window is a -documented contract. - -The first draft re-uploaded artifacts under a second workflow run to re-key them to `B` on a -fresh 90-day clock; with the run ID pinned in `B`'s manifest there is nothing to re-key, and a -single window remains: **the tag must be pushed within 90 days of the build.** That is more than -sufficient for this project's cadence; expiry recovery is §11. - -## 7. Versioning and tags - -The version is chosen once, at `release-build` dispatch, as a required input. It has to be: the -release-asset URL is a function of the tag name, and it is baked into `B`'s `Package.swift`, -which must exist *before* the tag does. (The first draft called the tag "the sole source of -truth" while also requiring the URL to be written at build time, with no version input anywhere -— a contradiction; no step in its flow ever supplied the version to the thing writing the URL.) - -The tag remains the **approval**: nothing publishes until `v` is pushed at `B`, and -`release-publish` hard-fails unless the tag name matches the manifest's version exactly (§5, -check 1), so the tag and the baked URL cannot desync. Format `vMAJOR.MINOR.PATCH` (e.g. -`v1.4.0`), or `vMAJOR.MINOR.PATCH-prerelease` for a release candidate (e.g. `v0.6.0-rc1`). -There is still no `VERSION` file and no version field in `Package.swift`; `release-publish` -derives the version string it passes to release notes (and a future Conan target) from -`github.ref_name`. - -**Release candidates.** A version may carry a SemVer 2.0 pre-release suffix: a hyphen, then -dot-separated groups of letters, digits and hyphens. `0.6.0-rc1` is the shape this repository -uses. SemVer sorts a pre-release *before* the version it leads to, so `0.6.0-rc1` comes before -`0.6.0-rc2`, which comes before `0.6.0`. `release-build` rejects a numeric group with a leading -zero for that same ordering reason, and rejects build metadata (`+meta`) because SemVer ignores -it when comparing versions — two releases differing only there would look like one version to a -consumer. - -A candidate goes through the same steps as any other release: the same four human steps, the -same two-commit PR, the same merge commit, the same tag at `B`, and the same six checks at -publish time. Only the last step differs. `release-publish` passes `--prerelease` to -`gh release create` when the version carries a suffix, so the repository's "latest release" -keeps pointing at the newest finished version and a candidate is never the default download. - -A consumer takes a candidate by pinning it: - -```swift -.package(url: "https://github.com/webern/mx.git", exact: "0.6.0-rc1") -``` - -A range requirement such as `from: "0.6.0"` is not the way to reach one. Someone testing a -candidate should be saying so in their manifest, and should have to edit that line to move on to -the finished release. - -Release tags are protected by a repository ruleset on pattern `v*`: **Restrict updates** and -**Restrict deletions**, with repo admins on the bypass list. This is complementary to — not a -substitute for — the tag-push-is-approval decision: it prevents *post-hoc tampering* (a -force-push retargeting `v1.4.0` at a different commit after it already shipped), which would -desync the public release from what the tag claims. It does not add a pre-publish review gate. - -Once the PR is merged and the tag is pushed, the `release-prep/v` branch can be -deleted: `B` stays reachable from both `main` and the tag. - -## 8. Build mechanics (Swift target) - -`mx` builds with CMake/Make, not an Xcode project, and the design keeps it that way. Compilation -uses CMake's built-in Apple cross-compilation support, invoked once per slice — -`CMAKE_SYSTEM_NAME=iOS` plus the slice's `CMAKE_OSX_SYSROOT`/`CMAKE_OSX_ARCHITECTURES` (the -design anticipated a third-party toolchain file, e.g. `leetal/ios-cmake`; none turned out to be -needed). CMake drives `clang`/`clang++` **directly** (the same compiler the existing native -`macos` CI job uses) — no `.xcodeproj` is generated and no `xcodebuild` runs for *compilation*. -Each slice builds the `mx` CMake target and its dependencies (`mx_core`, `mxpugi`) and merges -the three static libraries into one with `libtool -static`, so each XCFramework slice carries a -single library plus the public headers (`src/include`). No module map ships: the consumers are -C++/ObjC++ and include `mx/api/...` via the header search path SwiftPM derives from the bundle; -Swift-`import` support would be additive later. Deployment floors are macOS 11.0 and iOS 13.0. - -The single unavoidable Apple-tool call is the final `xcodebuild -create-xcframework`, used purely -to assemble the three static-lib slices plus headers into a valid `.xcframework` bundle — there -is no vendor-neutral tool that writes that bundle format correctly. Shape: - -``` -3× (CMake + clang build → static .a + headers) # macOS universal, iOS device, iOS simulator -1× xcodebuild -create-xcframework # stitch the three slices -1× zip + swift package compute-checksum # produce the release artifact + checksum -``` - -Platform/arch scope for v1: **macOS** (`arm64`+`x86_64`), **iOS device** (`arm64`), **iOS -simulator** (`arm64`+`x86_64`). tvOS, watchOS, and visionOS are explicitly out of scope for v1. - -Two `Package.swift` manifests exist in this design, and the binary one lives at exactly one -commit per release: - -- **`main`'s manifest** stays a source-only package: sibling-checkout development - (`.package(path:)`) and any non-Apple consumption keep working with zero setup, at every - commit of `main` (`R` guarantees this — the binary manifest never survives to a `main` tip). - The env-gated `MX_BINARY_RELEASE` arm that used to `fatalError` was superseded by this design - and was deleted when the workflows landed — `main` never switches to binary mode, so the arm - was dead code under the final scheme. -- **The release manifest**, generated by `_release-target-swift.yml` from a template checked in - at `A` (`.github/release/Package.swift.template`, with the URL and checksum interpolated) and - existing only at `B`: `.binaryTarget(name: "Mx", url: , checksum: - )` is the **default**, so a `from: "1.4.0"` consumer gets the binary with no - environment setup — the fix for the first draft's fatal assumption that consumers could be - asked to set an environment variable. The source target remains reachable from the release - manifest as the automatic arm on non-Apple hosts (where an XCFramework `binaryTarget` cannot - resolve; the manifest is Swift code evaluated on the consumer's machine, so `#if os(...)` - handles this) and behind an explicit `MX_SOURCE_BUILD=1` opt-out for anyone who wants to - compile a tagged release from source on a Mac. The template's source arms carry a copy of the - source target, so the template must be kept in sync with `Package.swift` whenever the target - changes — both files carry a KEEP IN SYNC comment pointing at each other, the release PR - review is the human check, and the build evaluates the interpolated manifest with - `swift package dump-package` before committing it. - -## 9. The release-target contract (Conan honesty) - -Each release target is a reusable workflow (`on: workflow_call`) — reusable workflows, not -composite actions, because only reusable workflows can contain multiple jobs (a build matrix plus -packaging). The shared contract that keeps the system multi-target: - -- **Inputs:** the commit `sha` to build and the `version` being released. -- **Outputs:** (a) a named artifact `mx-release--` containing the publishable - archive(s) + checksums + a manifest fragment; (b) the exact release-metadata file contents to - commit in `B`; (c) the list of paths those files may occupy — the target's contribution to the - allowed-path set that `release-publish` enforces on the `A..B` diff. - -`release-build` fans out across targets and its prep job merges every target's files into the -one commit `B` (and mechanically reverts the tree-affecting ones in `R`); `release-publish` -validates the diff against the union of all targets' declared paths (clamped by its static -ceiling, §5) and publishes every target's artifact under the one tag. Adding Conan is: write -`_release-target-conan.yml` to that contract, register it in the fan-out, and add its metadata -paths to the publish ceiling. No other change to the publish or validation logic. Conan's actual -recipe/upload mechanics are deliberately left unspecified here. - -## 10. Token permissions (least privilege, per job) - -| Job | Permissions | -|---|---| -| `_release-target-*` build jobs | `contents: read` | -| `release-build` prep job | `contents: write` (pushes the `release-prep/v*` branch), `pull-requests: write` (opens the release PR) | -| `release-publish` | `contents: write` (the only job that can create a Release), `actions: read` (reads the pinned run's metadata and downloads its artifacts — cross-run artifact access requires it; the first draft's table omitted `actions: read` entirely, so both of its artifact-reading jobs would have been denied by the API) | - -No workflow requests broad repo-wide write. The single job that can mint a public Release is -isolated behind the tag-push trigger. - -## 11. Failure modes - -- **Any `release-publish` verification failure** (§5: missing or malformed manifest — including - a tag pushed at `A` or any non-release commit; tag/version mismatch; parentage violation — - including tagging `R` or the merge commit by mistake; `B` not in `main`'s ancestry — the PR - unmerged, or squash/rebase-merged; diff violation; pinned run missing, failed, or foreign; - artifact expired past 90 days; checksum mismatch): **hard-fail, loudly, no fallback and no - auto-rebuild.** Publishing is all-or-nothing; a partial publish is never emitted. The failure - message names the check that tripped and directs the owner to recover by deleting the tag, - fixing the cause (merging the PR when it wasn't; the `-s ours` history-only merge when it was - squashed, §3; re-dispatching `release-build` when the artifact expired or the run was bad — - producing a fresh PR and a fresh `B` to tag), and re-pushing. -- **Commits landing on `main` between `A` and the tag:** harmless. `B` sits directly on `A`, so - mainline activity cannot leak into the tagged tree; at worst the release PR needs a trivial - conflict resolution in `release-manifest.json`, which nothing else touches. (In the first - draft this was a silent freeze requirement that would have hard-failed any release with a - concurrent merge.) -- **No manual override / `force_rebuild` escape hatch exists**, by design — it would silently - reintroduce the rebuild-drift risk the whole two-phase split exists to prevent. - -## 12. Out of scope / future work - -- Conan target implementation (§9 defines only its contract). -- tvOS/watchOS/visionOS slices. -- Code signing / notarization — not applicable to a static-library XCFramework; would only matter - if `mx` later ships a signed executable. -- Auto-generated release notes are GitHub's default categorization of merged PRs since the prior - tag; a curated `CHANGELOG.md`-driven flow is not adopted for v1. - -## 13. Revision log - -**2026-08-23 (d) — release candidates.** A version may now carry a SemVer pre-release suffix, -so `0.6.0-rc1` can be dispatched, tagged and published (§7). Two changes carry it: -`release-build` validates the version against SemVer 2.0 minus build metadata instead of three -plain numbers, and `release-publish` marks the GitHub Release as a pre-release when the version -carries a suffix, which keeps "latest" on the newest finished version. Nothing else moved. The -`v*` tag trigger, the six checks, the asset URL and the two-commit PR shape were all written in -terms of the version string rather than its shape, so they took a candidate without edits. - -**2026-08-23 (c) — implemented.** The workflows in §4, the template in §8, and the -`Package.swift` cleanup shipped in one commit. Implementation drift from the design text, each -recorded in place above: CMake's built-in `CMAKE_SYSTEM_NAME=iOS` support replaced the -anticipated third-party toolchain file, and each slice's three static libraries are merged with -`libtool -static` (§8); the §9 contract's outputs (b) and (c) are concretely a per-target -`mx-release--metadata-` artifact consumed by the prep job (§4); the release PR runs -no CI because it is pushed with the workflow token (§4); and two repository settings became -explicit prerequisites — Actions may create PRs, and "Create a merge commit" is an allowed merge -method (§4). - -**2026-08-23 (b) — owner requirement: tags live on `main`.** The first revision fixed the -"binary-default vs source-default" conflict and the frozen-`main` problem by tagging a commit on -a never-merged release branch. The owner requires release tags to be part of `main`'s history. -Resolution: the release PR returns, now carrying two machine-written commits — `B` (binary -manifest + provenance record; the tag target) and `R` (restores the source manifest) — so a -merge-commit merge places `B` in `main`'s ancestry while every `main` tip stays a source -package (§3). Both original fixes survive: the binary default still exists at exactly one -commit, and `B`'s parent is still `A`, so nothing about `main`'s movement affects the tagged -tree. New consequences, both enforced by §5 check 3: the release PR must be merged with a true -merge commit (squash/rebase would strand `B` outside `main`'s history; recovery documented in -§3), and the tag can only be pushed after the merge. `release-build` regains -`pull-requests: write`. - -**2026-08-23 (a) — design review of the first draft found seven flaws; all remain fixed above.** - -1. *The `head_sha` lookup could never work.* A `pull_request: closed`-triggered run reports the - PR branch head as its `head_sha`, never the merge commit, so `release-publish` querying for a - validating run keyed to the tagged SHA would find nothing, every time. → Run-history queries - are gone; `B` pins its builder's run ID in `release-manifest.json` and publish re-verifies - from primary sources (§5). -2. *The version was unknown when the asset URL had to be written.* The tag was "the sole source - of truth," chosen at step 4, yet the URL derived from it was needed at step 2. → `version` is - a required `release-build` input; publish enforces tag == manifest version (§7). -3. *Consumers could never reach binary mode.* The env-gated `MX_BINARY_RELEASE` arm is - unreachable for downstream SwiftPM consumers, who cannot be made to set environment - variables. → The release manifest, existing only at `B`, is binary-by-default; `main` stays - source-only at every tip (§3, §8). -4. *The merge-based flow silently required a frozen `main`.* Any commit landing between `A` and - the release-prep merge made the merged tree diverge from `A` + metadata. → The tagged commit - is `B`, whose parent is always `A`; the merge commit is never tagged, so `main`'s movement - is irrelevant to the tagged tree (§3). -5. *The permissions table omitted `actions: read`,* which cross-run artifact download requires; - both artifact-reading jobs would have 403'd. → Added (§10). -6. *Nothing specified how the `release-build` run ID was discovered* (dispatch inputs are not - queryable via the API). → The run pins its own `github.run_id` into `B`'s manifest (§5). -7. *The written checksum was never verified against the artifact,* so a corrupted or tampered - checksum would ship a release every consumer fails to resolve. → Publish recomputes the - SHA-256 and requires bytes == manifest == `Package.swift` (§5, check 6). - -Also: per-target artifact names now include the target (`mx-release--`) so a second -target cannot collide with Swift's artifact key (§6). diff --git a/docs/ai/design/xml-backend.md b/docs/ai/design/xml-backend.md index ae1f3728f..9496b514a 100644 --- a/docs/ai/design/xml-backend.md +++ b/docs/ai/design/xml-backend.md @@ -7,7 +7,7 @@ Tracking issue: webern/mx#478. `mx::core` is generated code that reads and writes an XML DOM. Until 0.6 that DOM was pugixml's, called directly: every generated parser took a `pugi::xml_node`, every serializer appended to one, and `mx::core::parse` took a `pugi::xml_document`. That was the right call when the typed model was -designed (see `mx-core-plan.md` §2.1: the earlier `ezxml` virtual-interface wrapper allocated a +designed (the earlier `ezxml` virtual-interface wrapper allocated a `shared_ptr` and copied a `std::string` per node access, and retiring it made the core fast). It left two problems for people who build mx into something larger. @@ -15,14 +15,14 @@ left two problems for people who build mx into something larger. `pugi::` symbol, one from their copy and one from mx's vendored copy. Whether the linker complained or silently picked one depended on versions and build flags. 2. **Consumers who want another XML library.** Some projects standardize on libxml2, tinyxml2 or a - platform parser, and do not want a second XML library in the binary at all. + platform parser, and do not want a second XML library in the binary. ## Step one: the rename (PR #479) The vendored pugixml moved to `src/private/mxpugi/` and was compiled under the `mxpugi` namespace (`scripts/vendor_pugixml.py` does the renaming; `make vendor-pugixml VERSION=x.y` refreshes it). That ended the collisions: mx does not define a `pugi::` symbol, and `make symbol-check` fails CI -if one ever appears. It did nothing for problem 2, and it left `mxpugi::` spelled across the generated +if one ever appears. It did not solve problem 2, and it left `mxpugi::` spelled across the generated code, the runtime, `mx::api` and the tests. ## Step two: the seam (this document) @@ -31,7 +31,7 @@ Everything on mx's side of the XML library now goes through three value types in `src/private/mx/xml/Xml.h`: - `mx::xml::Node` and `mx::xml::Attribute` are trivially copyable handles into a tree. Copying one - copies the reference, the way copying a pointer does. Each wraps the backend's own handle type + copies the reference, the way copying a pointer does. Each wraps the backend's handle type (`Backend::NodeHandle`, `Backend::AttributeHandle`) and every member is an inline, non-virtual one-line forwarder to a static function of `mx::xml::Backend`. - `mx::xml::Document` owns a `Backend::DocumentHandle`. It loads from a file, a string or a stream, @@ -61,7 +61,7 @@ a message that names the requirement. The surface is exactly what `mx::core`, `mx::api` and the tests use. Strings come back as `const char *` owned by the tree, as pugixml returns them, so a name or value lookup never -allocates. Strings go in as `std::string_view`. The seam does not expose the backend's own +allocates. Strings go in as `std::string_view`. The seam does not expose the backend's handle, so nothing on the mx side can come to depend on one backend by accident. When a member of the seam loses its last caller, it goes, and so does its line in the concept: a smaller contract is less for a backend author to write. @@ -114,7 +114,9 @@ canonicalizers, `IdSchemaDriftTest` reading the XSD). When a test needs to hand `mx::core` or read what it wrote, it crosses the seam as text: save the mxpugi document to a string, load the string into an `mx::xml::Document`, call the core, serialize into an `mx::xml::Document`, save that to a string, load it back into mxpugi. The corert harness does exactly this -(`CoreRoundtripImpl.cpp`, steps 3 and 4). A test that only needs the core's view of a document uses +(`CoreRoundtripImpl.cpp`, steps 3 and 4). + +A test that only needs the core's view of a document uses `mx::xml::Document` directly. Test code uses one or the other and never casts between them, which is what a consumer who brings a different XML library would have to do. @@ -124,28 +126,45 @@ The choice is the CMake cache variable `MX_XML_BACKEND`, with three values (`set_property(CACHE ... STRINGS)` lists them; anything else is a configure error). CMake prints the chosen value as `mx: MX_XML_BACKEND=...`. -- `vendored` (the default, always; never chosen by detecting another pugixml target): the pugixml - adapter against mx's private `mxpugi` copy. `mx_core` links `mxpugi` and does not get an extra - definition, so this mode is byte-for-byte the build mx had before the option existed. No - `pugi::` symbol; nothing to install. -- `pugixml`: the pugixml adapter against the consumer's own pugixml. `MX_PUGIXML_TARGET` (cache - string, default `pugixml`, which is the target upstream pugixml's CMake defines as an interface - over `pugixml-static`) names the CMake target; it has to exist when `add_subdirectory(mx)` runs, - and the configure fails with a `FATAL_ERROR` telling the consumer to create it (FetchContent, - `find_package`, or `add_subdirectory` on a checkout) otherwise. `mx_core` links that target - `PUBLIC` and gets the `PUBLIC` compile definitions `MX_XML_PUGI_NAMESPACE=pugi` and - `MX_XML_PUGI_HEADER="pugixml.hpp"`. `mxpugi` is not linked into `mx_core` or `mx`. -- `custom`: the consumer provides `MX_XML_BACKEND_HEADER` (the include path of a header that - defines `struct mx::xml::Backend` satisfying `XmlBackend`, written the way it would appear - between the quotes of an `#include`) and `MX_XML_BACKEND_TARGET` (a CMake target, INTERFACE or - library, carrying the header's include directory, any sources, and the link to the XML library; - it exists before `add_subdirectory(mx)`, so a source on it that includes `mx/xml/Concept.h` has - to carry mx's `src/private` include path itself). - Both are required; the configure fails naming them otherwise. `mx_core` links the target - `PUBLIC`, gets the `PUBLIC` compile definition `MX_XML_BACKEND_HEADER="..."`, and does not - compile `src/private/mx/xml/pugixml/Backend.cpp` (it includes pugixml). The `static_assert` in - `Xml.h` is the check that the struct satisfies the concept. This mode is a documented contract: - there is no CI job and no example backend beyond the pugixml one. +### `vendored` + +The default, always; it is never chosen by detecting another pugixml target. The pugixml adapter +runs against mx's private `mxpugi` copy. `mx_core` links `mxpugi` and does not get an extra +definition, so this mode is byte-for-byte the build mx had before the option existed. No `pugi::` +symbol; nothing to install. + +### `pugixml` + +The pugixml adapter runs against the consumer's pugixml, not the vendored mxpugi. +`MX_PUGIXML_TARGET` (cache string, default `pugixml`, which is the target upstream pugixml's CMake +defines as an interface over `pugixml-static`) names the CMake target. + +The target has to exist when `add_subdirectory(mx)` runs. Otherwise the configure fails with a +`FATAL_ERROR` telling the consumer to create it (FetchContent, `find_package`, or +`add_subdirectory` on a checkout). + +`mx_core` links that target `PUBLIC` and gets the `PUBLIC` compile definitions +`MX_XML_PUGI_NAMESPACE=pugi` and `MX_XML_PUGI_HEADER="pugixml.hpp"`. `mxpugi` is not linked into +`mx_core` or `mx`. + +### `custom` + +The consumer provides two settings, and the configure fails naming them if either is missing: + +- `MX_XML_BACKEND_HEADER`: the include path of a header that defines `struct mx::xml::Backend` + satisfying `XmlBackend`, written the way it would appear between the quotes of an `#include`. +- `MX_XML_BACKEND_TARGET`: a CMake target, INTERFACE or library, carrying the header's include + directory, any sources, and the link to the XML library. It exists before + `add_subdirectory(mx)`, so a source on it that includes `mx/xml/Concept.h` has to carry mx's + `src/private` include path itself. + +`mx_core` links the target `PUBLIC`, gets the `PUBLIC` compile definition +`MX_XML_BACKEND_HEADER="..."`, and does not compile `src/private/mx/xml/pugixml/Backend.cpp` (it +includes pugixml). The `static_assert` in `Xml.h` is the check that the struct satisfies the +concept. This mode is a documented contract: there is no CI job and no example backend beyond the +pugixml one. + +### All modes The link and the definitions are `PUBLIC` on `mx_core` because `Xml.h` is included by the core's headers: the `mx` library, the tests and the consumer all have to see the same backend. @@ -162,10 +181,13 @@ default `MX_API=ON` build of everything still does, because `mxtest` links it). The `pugixml` mode has a gate. `cmake/backend-pugixml/CMakeLists.txt` is a consumer project that fetches pugixml 1.16 with FetchContent, sets `MX_XML_BACKEND=pugixml`, `MX_CORE_DEV=ON` and `MX_API=OFF` (the backend lives in `mx_core`; the core suites are what runs), and -adds the repository root as a subdirectory. `make backend-pugixml-test` configures it into +adds the repository root as a subdirectory. + +`make backend-pugixml-test` configures it into `build/backend-pugixml`, builds it, runs the corert and core unit binaries, and then checks with `nm -g | c++filt` that `libmx_core.a` in that tree references `pugi::` and does not hold an `mxpugi::` symbol, which is the proof that the mode took. The CI job `backend: pugixml` runs that target. + `make symbol-check` is the gate for the vendored build only; in the pugixml mode `mx_core` references `pugi::` on purpose. diff --git a/gen/cpp/config.toml b/gen/cpp/config.toml index 2923a5b06..62060c840 100644 --- a/gen/cpp/config.toml +++ b/gen/cpp/config.toml @@ -45,7 +45,7 @@ doctype-timewise = 'score-timewise PUBLIC \"-//Recordare//DTD MusicXML 4.0 Timew [naming] extends = "../naming.base.toml" # schema-forced renames shared by all targets -# Modern accessor style (plan §2.6): camelCase accessors `step()` / +# Modern accessor style: camelCase accessors `step()` / # `setStep(...)`; the templates spell private members `m_{{ident}}`. field-convention = "camel" # Variant identifiers are the named-factory spellings on the enum wrapper @@ -64,7 +64,7 @@ string = "std::string" token = "std::string" date = "std::string" # The NCName-derived identity primitives get hand-written repairing wrappers -# (valid-by-construction, plan §2.2): a lexically invalid id/idref/nmtoken +# (valid-by-construction): a lexically invalid id/idref/nmtoken # cannot exist through the public API. Token enforces NCName (xs:ID/IDREF); # NameToken enforces the wider xs:NMTOKEN (leading digits allowed). nmtoken = "NameToken" @@ -103,7 +103,7 @@ words = [ "small", ] -# Semantic names for the synthesized content types the §2.9 projection +# Semantic names for the synthesized content types the grammar-preserving projection # hoists out of anonymous nested particles (mechanical wires like # `note-choice-group-2` stay addressable; these give the high-traffic note # and lyric structures their domain names). @@ -119,7 +119,7 @@ words = [ "lyric-choice-group-group-group" = "elision-syllabic-group" [import] -# Import-policy repairs (plan §2.4): a required attribute missing from a +# Import-policy repairs: a required attribute missing from a # document gets the configured default injected by the parser; missing with # no entry is a parse error. xml:lang on lyric-language defaults to BCP 47 # "und" (undetermined). @@ -129,7 +129,7 @@ words = [ # ---- the render manifest: which templates produce which files ----------- # # One template per shape (value shapes here; the complex/group/choice shapes # land with the W2 templates). Bespoke rows give individual value types the -# structural representations of mx-core-plan.md §2.2. +# structural representations they need. [render] dir = "templates" @@ -178,7 +178,7 @@ strategies = ["tagged-variant"] template = "union.cpp.tmpl" output = "{pascal}.cpp" -# ---- bespoke structural value types (plan §2.2) -------------------------- # +# ---- bespoke structural value types -------------------------------------- # [[render.type]] types = ["color"] @@ -349,8 +349,8 @@ strategies = ["choice-class"] template = "choice.cpp.tmpl" output = "{pascal}.cpp" -# Measure music-data is the known large-alternative repeated choice (plan -# §2.3): its alternatives are boxed so a by-value variant does not size +# Measure music-data is the known large-alternative repeated choice: its +# alternatives are boxed so a by-value variant does not size # every slot at the largest alternative. Which types box is this manifest # data, not template hardcode. [[render.type]] diff --git a/gen/cpp/templates/attributes_parse.tmpl b/gen/cpp/templates/attributes_parse.tmpl index cbf59227c..751450272 100644 --- a/gen/cpp/templates/attributes_parse.tmpl +++ b/gen/cpp/templates/attributes_parse.tmpl @@ -32,7 +32,7 @@ if (!seen_{{ident}}) { {{#import_default}} - // Configured import repair (plan §2.4): inject the default. + // Configured import repair: inject the default. out.set{{name.pascal}}({{>attr_default_expr}}); reportAttributeDefaulted(context, el, {{name.wire_q}}, {{import_default_q}}); {{/import_default}} diff --git a/gen/cpp/templates/choice.h.tmpl b/gen/cpp/templates/choice.h.tmpl index a80e47672..bf5bdd941 100644 --- a/gen/cpp/templates/choice.h.tmpl +++ b/gen/cpp/templates/choice.h.tmpl @@ -33,7 +33,7 @@ class ParseContext; /// A schema choice{{#synthesized}} (synthesized from an anonymous particle /// of the schema){{/synthesized}}: exactly one alternative by construction. /// Alternatives are positional; dispatch is by Kind/index, never by type -/// (alternative types may repeat -- plan §2.3). +/// (alternative types may repeat). class {{ident}} final { public: diff --git a/gen/cpp/templates/choice_boxed.h.tmpl b/gen/cpp/templates/choice_boxed.h.tmpl index c2f346ba9..52b912416 100644 --- a/gen/cpp/templates/choice_boxed.h.tmpl +++ b/gen/cpp/templates/choice_boxed.h.tmpl @@ -25,7 +25,7 @@ class ParseContext; {{/doc_lines}} /// A repeated heterogeneous choice with large alternatives: each is boxed /// so a by-value variant does not size every slot at the largest -/// alternative (plan §2.3). The API stays value-like; copies clone. +/// alternative. The API stays value-like; copies clone. class {{ident}} final { public: diff --git a/gen/cpp/templates/color.h.tmpl b/gen/cpp/templates/color.h.tmpl index 77dd691bd..6aaf9649f 100644 --- a/gen/cpp/templates/color.h.tmpl +++ b/gen/cpp/templates/color.h.tmpl @@ -15,7 +15,7 @@ namespace {{vars.namespace}} {{#doc_lines}} /// {{value}} {{/doc_lines}} -/// Structural representation (plan §2.2): numeric RGB components plus an +/// Structural representation: numeric RGB components plus an /// optional alpha; serialization formats #RRGGBB / #AARRGGBB. A malformed /// hex string cannot exist. class {{ident}} final diff --git a/gen/cpp/templates/comma_separated_text.h.tmpl b/gen/cpp/templates/comma_separated_text.h.tmpl index ff6350e5a..6743b833d 100644 --- a/gen/cpp/templates/comma_separated_text.h.tmpl +++ b/gen/cpp/templates/comma_separated_text.h.tmpl @@ -15,7 +15,7 @@ namespace {{vars.namespace}} {{#doc_lines}} /// {{value}} {{/doc_lines}} -/// Structural representation (plan §2.2): a never-empty list of non-empty, +/// Structural representation: a never-empty list of non-empty, /// comma-free items. Construction repairs deterministically (commas inside /// an item become spaces, blank items drop, an empty list becomes ["-"]). /// A value parsed from the wire keeps its exact spelling (the separator may diff --git a/gen/cpp/templates/composite.h.tmpl b/gen/cpp/templates/composite.h.tmpl index cd5b6cff1..4c02b3816 100644 --- a/gen/cpp/templates/composite.h.tmpl +++ b/gen/cpp/templates/composite.h.tmpl @@ -41,7 +41,7 @@ class ParseContext; /// {{value}} {{/doc_lines}} /// Content fields mirror the schema grammar in declaration order; the -/// serializer walks them, so a wrong order is unrepresentable (plan §2.3). +/// serializer walks them, so a wrong order is unrepresentable. class {{ident}}{{^is_base}} final{{/is_base}} { public: diff --git a/gen/cpp/templates/derived.h.tmpl b/gen/cpp/templates/derived.h.tmpl index 5c13ecfd7..ffc227c1e 100644 --- a/gen/cpp/templates/derived.h.tmpl +++ b/gen/cpp/templates/derived.h.tmpl @@ -33,7 +33,7 @@ class ParseContext; /// {{value}} {{/doc_lines}} /// The schema's complexContent extension: non-polymorphic public -/// inheritance, no virtual anything (plan §2.3). +/// inheritance, no virtual anything. class {{ident}} : public {{base.ident}} { public: diff --git a/gen/cpp/templates/document.h.tmpl b/gen/cpp/templates/document.h.tmpl index 1c717473b..f25808705 100644 --- a/gen/cpp/templates/document.h.tmpl +++ b/gen/cpp/templates/document.h.tmpl @@ -89,13 +89,13 @@ private: std::vector m_rootNamespaces; }; -/// The parse error boundary (plan §2.5): strict on names and structure, +/// The parse error boundary: strict on names and structure, /// lenient on values; a root declaring a version newer than /// SupportedMusicXMLVersion is rejected with unsupportedVersion. Result parse(const mx::xml::Document &doc, const ParseContext &context); // TODO: document the unique ID exception -/// Total for every Document (§1.1 guarantees validity): writes the XML +/// Total for every Document (a Document is valid by construction): writes the XML /// declaration, the matching DOCTYPE, the preserved root namespace /// declarations, and the typed tree. void serialize(const Document &d, mx::xml::Document &out); diff --git a/gen/cpp/templates/empty.h.tmpl b/gen/cpp/templates/empty.h.tmpl index 89695c55b..86e3f834b 100644 --- a/gen/cpp/templates/empty.h.tmpl +++ b/gen/cpp/templates/empty.h.tmpl @@ -17,7 +17,7 @@ class ParseContext; {{/doc_lines}} /// Presence-only marker: the element's only information is whether it /// appears. Optional occurrences project to bool fields; choice -/// alternatives hold this marker (plan §2.3). +/// alternatives hold this marker. class {{ident}}{{^is_base}} final{{/is_base}} { public: diff --git a/gen/cpp/templates/ending_number.h.tmpl b/gen/cpp/templates/ending_number.h.tmpl index bad22539f..4c941aad7 100644 --- a/gen/cpp/templates/ending_number.h.tmpl +++ b/gen/cpp/templates/ending_number.h.tmpl @@ -15,7 +15,7 @@ namespace {{vars.namespace}} {{#doc_lines}} /// {{value}} {{/doc_lines}} -/// Structural representation (plan §2.2): the ending numbers as a list of +/// Structural representation: the ending numbers as a list of /// positive integers, possibly empty (the schema's blank-ending case, used /// when the number is not printed). Construction clamps each value to >= 1. /// A value parsed from the wire keeps its exact spelling (blank endings may diff --git a/gen/cpp/templates/fields_api.tmpl b/gen/cpp/templates/fields_api.tmpl index a999b17ee..f73c83e50 100644 --- a/gen/cpp/templates/fields_api.tmpl +++ b/gen/cpp/templates/fields_api.tmpl @@ -17,7 +17,7 @@ std::span {{ident}}() const noexcept; {{#max}} /// Bounded by the schema: at most {{max}} occurrences. An over-cap - /// append is rejected (plan §1.2). + /// append is rejected. [[nodiscard]] Result add{{name.pascal}}({{type_ref.ident}} value); void clear{{name.pascal}}() noexcept; {{/max}} diff --git a/gen/cpp/templates/group.h.tmpl b/gen/cpp/templates/group.h.tmpl index 061f752e3..9ed4a3e2f 100644 --- a/gen/cpp/templates/group.h.tmpl +++ b/gen/cpp/templates/group.h.tmpl @@ -35,7 +35,7 @@ class ParseContext; {{/doc_lines}} /// A shared content group{{#synthesized}} (synthesized from an anonymous /// particle of the schema){{/synthesized}}: transparent on the wire, its -/// fields serialize directly into the referencing element (plan §2.3). +/// fields serialize directly into the referencing element. class {{ident}} final { public: diff --git a/gen/cpp/templates/number.h.tmpl b/gen/cpp/templates/number.h.tmpl index 6d821926f..f9fe30829 100644 --- a/gen/cpp/templates/number.h.tmpl +++ b/gen/cpp/templates/number.h.tmpl @@ -44,7 +44,7 @@ public: std::string toString() const; /// Strict lexical parse, then range-clamp (the import leniency policy - /// for numbers, plan §2.4). + /// for numbers). static bool tryParse(std::string_view text, {{ident}} &out); /// Lenient: non-numeric text yields the clamped zero. diff --git a/gen/cpp/templates/smufl_prefixed.h.tmpl b/gen/cpp/templates/smufl_prefixed.h.tmpl index cf841508f..8908c72d8 100644 --- a/gen/cpp/templates/smufl_prefixed.h.tmpl +++ b/gen/cpp/templates/smufl_prefixed.h.tmpl @@ -13,7 +13,7 @@ namespace {{vars.namespace}} {{#doc_lines}} /// {{value}} {{/doc_lines}} -/// Structural representation (plan §2.2): the glyph-name prefix is +/// Structural representation: the glyph-name prefix is /// structural and emitted by the serializer; only the suffix is stored. /// Construction repairs the suffix to name characters{{#suffix_required}} /// (and to non-empty, the placeholder being "X"){{/suffix_required}}, so a diff --git a/gen/cpp/templates/smufl_wavy.h.tmpl b/gen/cpp/templates/smufl_wavy.h.tmpl index 9e8b8be68..6a1bce444 100644 --- a/gen/cpp/templates/smufl_wavy.h.tmpl +++ b/gen/cpp/templates/smufl_wavy.h.tmpl @@ -13,7 +13,7 @@ namespace {{vars.namespace}} {{#doc_lines}} /// {{value}} {{/doc_lines}} -/// Structural representation (plan §2.2) of the two branches the pattern +/// Structural representation of the two branches the pattern /// allows: wiggle (suffix non-empty), or guitarVibratoStroke /// (middle possibly empty). The literals are structural; only the variable /// part is stored, so a pattern-invalid name cannot exist. diff --git a/gen/cpp/templates/string.cpp.tmpl b/gen/cpp/templates/string.cpp.tmpl index d015907ea..2a94d4d19 100644 --- a/gen/cpp/templates/string.cpp.tmpl +++ b/gen/cpp/templates/string.cpp.tmpl @@ -10,7 +10,7 @@ namespace {{vars.namespace}} std::string repaired{{ident}}(std::string v) { {{#min_length}} - // Deterministic repair to the schema's minLength (plan §2.2.2): pad + // Deterministic repair to the schema's minLength: pad // with '-' so a too-short value is unrepresentable. while (v.size() < {{min_length}}) { diff --git a/gen/cpp/templates/time_only.h.tmpl b/gen/cpp/templates/time_only.h.tmpl index 24d096bbb..4a11914e8 100644 --- a/gen/cpp/templates/time_only.h.tmpl +++ b/gen/cpp/templates/time_only.h.tmpl @@ -15,7 +15,7 @@ namespace {{vars.namespace}} {{#doc_lines}} /// {{value}} {{/doc_lines}} -/// Structural representation (plan §2.2): a never-empty list of positive +/// Structural representation: a never-empty list of positive /// integers (the time-through-repeat indices), serialized comma-separated. /// Construction clamps each value to >= 1; an empty list becomes [1]. class {{ident}} final diff --git a/gen/cpp/templates/yyyy_mm_dd.h.tmpl b/gen/cpp/templates/yyyy_mm_dd.h.tmpl index a8ee7164e..4671fa9a9 100644 --- a/gen/cpp/templates/yyyy_mm_dd.h.tmpl +++ b/gen/cpp/templates/yyyy_mm_dd.h.tmpl @@ -13,7 +13,7 @@ namespace {{vars.namespace}} {{#doc_lines}} /// {{value}} {{/doc_lines}} -/// Structural representation (plan §2.2): year/month/day integers clamped +/// Structural representation: year/month/day integers clamped /// to valid calendar ranges (leap years honored), serialized YYYY-MM-DD. An /// impossible date cannot exist. class {{ident}} final diff --git a/gen/plates/build.py b/gen/plates/build.py index cc7a77b94..8d1aa0e90 100644 --- a/gen/plates/build.py +++ b/gen/plates/build.py @@ -304,7 +304,7 @@ def __init__(self, m: ir.Ir, config: Config): self.type_map = dict(config.types) self.variant_scope = config.target.variant_scope - # The grammar-preserving content projection (plan §2.9): normalize + # The grammar-preserving content projection: normalize # each content tree once (the discovery and the build must see the # SAME particle objects), walk it to find every structural group the # schema actually references and every anonymous choice/sequence @@ -344,7 +344,7 @@ def __init__(self, m: ir.Ir, config: Config): config.target.symbol_prefix + name.cased[naming.type_convention] ) - # ----- content discovery (plan §2.9) -------------------------------------- # + # ----- content discovery -------------------------------------------------- # # # One deterministic preorder walk over every composite's normalized # content, recursing through structural group references (each group @@ -713,7 +713,7 @@ def _complex_plate(self, ct: ir.ComplexType) -> ComplexPlate: plate.structural_deps = _dep_refs(srefs) return plate - # ----- group, choice, and synthesized content plates (plan §2.9) --------- # + # ----- group, choice, and synthesized content plates --------------------- # def _group_plates(self) -> list: out: list = [] @@ -1027,7 +1027,7 @@ def _validate_config_against_ir(self) -> list[str]: errors += self._promotion_errors() # Structural-group and synthesized content-type wires are addressable - # for renames too (plan §2.9). + # for renames too. type_wires = set(self.values_by_name) | set(self.complex_by_name) type_wires |= {self.group_wire[g] for g in self.struct_groups} type_wires |= set(self.synth_content) diff --git a/gen/plates/check.py b/gen/plates/check.py index 9c1304a6e..b6b7b8194 100644 --- a/gen/plates/check.py +++ b/gen/plates/check.py @@ -182,7 +182,7 @@ def _check_members(plates: Plates, errors: list[str]) -> None: def _check_fields(plates: Plates, errors: list[str]) -> None: - """The grammar-preserving projection's identifier scopes (plan §2.9): a + """The grammar-preserving projection's identifier scopes: a structural target's class holds the attribute members AND the content fields, so the two lists must be mutually unique; a choice type's alternative accessors share one scope likewise.""" diff --git a/gen/plates/model.py b/gen/plates/model.py index 9198d82e0..d7fb1a9ec 100644 --- a/gen/plates/model.py +++ b/gen/plates/model.py @@ -260,7 +260,7 @@ class Member: fixed: str | None = None default_variant: str | None = None # The configured import repair for a required attribute MISSING from a - # document ([import] attribute-defaults, plan §2.4): the parser injects + # document ([import] attribute-defaults): the parser injects # this wire literal; without one, missing is a parse error. import_default: str | None = None doc: str | None = None @@ -293,7 +293,7 @@ class ComplexPlate: strategy: str # value-class | composite-class | flag | attrs-class | inherit | flatten members: list[Member] = field(default_factory=list) content: ir.Particle | None = None - # The grammar-preserving field projection of `content` (plan §2.9): + # The grammar-preserving field projection of `content`: # ordered fields a structural target renders instead of the flat # `members` element view. Empty for content-free shapes. fields: list[ContentField] = field(default_factory=list) @@ -334,7 +334,7 @@ def members_view(self) -> list[Member]: # --------------------------------------------------------------------------- # -# Content fields (the grammar-preserving projection, plan §2.9) +# Content fields (the grammar-preserving projection) # --------------------------------------------------------------------------- # @@ -378,7 +378,7 @@ class Alternative: is nullable (the choice can be satisfied empty, e.g. key's non-traditional-key*). Alternatives are positional: a target whose sum type allows duplicate member types must dispatch by index, never by - type (plan §2.3).""" + type.""" name: Name ident: str @@ -472,8 +472,8 @@ class Plates: schema_version: str = "" # the MusicXML version in the source stem ("3.1") value_types: list[ValuePlate] = field(default_factory=list) complex_types: list[ComplexPlate] = field(default_factory=list) - # The shared content types of the grammar-preserving projection (plan - # §2.9): structural named schema groups plus the synthesized types + # The shared content types of the grammar-preserving projection: + # structural named schema groups plus the synthesized types # hoisted out of anonymous nested choices/sequences. A target consuming # only the flat member view never renders these. groups: list[GroupLikePlate] = field(default_factory=list) diff --git a/gen/press/render.py b/gen/press/render.py index bd7440a51..797491bfd 100644 --- a/gen/press/render.py +++ b/gen/press/render.py @@ -33,7 +33,7 @@ } ) -# The grammar-preserving projection's shared content types (plan §2.9). +# The grammar-preserving projection's shared content types. # Rendered only when the manifest engages them, so a target consuming the # flat member view (and its committed output) is untouched by their # existence. diff --git a/scripts/doctor.sh b/scripts/doctor.sh index 34e5b8544..9928583b4 100755 --- a/scripts/doctor.sh +++ b/scripts/doctor.sh @@ -106,20 +106,19 @@ if [ -x "$QUALITY_VENV/bin/python" ]; then else row 'gen-quality, gen-lint' 'MISSING venv; run make quality-venv' fi -row 'fmt, fmt-check' "$(need "$CLANG_FORMAT")" +row fmt "$(need "$CLANG_FORMAT")" row 'build-go, test-go' "$(need go)" c_status=$(need "$CMAKE" "$CC_TOOL") if ! libxml2_found; then if [ "$c_status" = 'ok' ]; then c_status='MISSING libxml2'; else c_status="$c_status libxml2"; fi fi row 'build-c, test-c' "$c_status" -if [ -x "$DESLAG" ]; then - row 'deslag (not used by targets yet)' 'ok' -elif has cargo; then - row 'deslag (not used by targets yet)' 'MISSING; run make deslag' -else - row 'deslag (not used by targets yet)' 'MISSING; make deslag needs cargo' +fmt_check_status=$(need "$CLANG_FORMAT") +if [ ! -x "$DESLAG" ]; then + if has cargo; then deslag_status='MISSING deslag; run make deslag'; else deslag_status='MISSING deslag; make deslag needs cargo'; fi + if [ "$fmt_check_status" = 'ok' ]; then fmt_check_status=$deslag_status; else fmt_check_status="$fmt_check_status, $deslag_status"; fi fi +row fmt-check "$fmt_check_status" if has ccache; then row 'ccache (optional)' 'found; native builds use it' else diff --git a/src/include/mx/api/ApiCommon.h b/src/include/mx/api/ApiCommon.h index e823e1005..f22a06f9a 100644 --- a/src/include/mx/api/ApiCommon.h +++ b/src/include/mx/api/ApiCommon.h @@ -71,11 +71,12 @@ enum class MeasureNumbering system }; -// Whether an item is associated with the system rather than only with the part it appears in -- -// the system attribute of (MeasureData) and of and +// Whether an item is associated with the system rather than only with the part it appears in -- the +// system attribute of (MeasureData) and of and // (DirectionData). only... means the item is drawn on the top or bottom part of the system instead // of this part; also... means it is drawn on both. unspecified means the attribute is absent. -// onlyBottom and alsoBottom are measure-numbering only; MusicXML has no bottom-of-system direction. +// onlyBottom and alsoBottom are measure-numbering only; MusicXML does not have a bottom-of-system +// direction. enum class SystemRelation { unspecified, @@ -88,8 +89,8 @@ enum class SystemRelation // The shape drawn around a piece of text or a symbol -- MusicXML's enclosure attribute, carried by // RehearsalData, WordsData, SymbolData, PercussionData and PageTextData. unspecified means the -// attribute is absent, which draws no enclosure; none states explicitly that there is none. A -// bracket is a rectangle with the bottom line missing, as is common in jazz notation, and an +// attribute is absent, so no enclosure is drawn; none states explicitly that there is no enclosure. +// A bracket is a rectangle with the bottom line missing, as is common in jazz notation, and an // invertedBracket is one with the top line missing. enum class Enclosure { diff --git a/src/include/mx/api/BarlineData.h b/src/include/mx/api/BarlineData.h index 7dbbc4cc8..3d0e4eb3a 100644 --- a/src/include/mx/api/BarlineData.h +++ b/src/include/mx/api/BarlineData.h @@ -62,17 +62,17 @@ class EndingData std::string text; // Where the bracket is drawn, in tenths of a staff space. defaultY sets the height of the - // bracket line above the top line of the staff, and defaultX moves the start of the bracket left - // or right of the barline. relativeX and relativeY shift the bracket from wherever it would - // otherwise be drawn. Set the matching is...Specified flag along with each value. An ending has - // no placement or alignment, so those members are not written. Leave this alone to let the - // program place the bracket. + // bracket line above the top line of the staff, and defaultX moves the start of the bracket + // left or right of the barline. relativeX and relativeY shift the bracket from wherever it + // would otherwise be drawn. Set the matching is...Specified flag along with each value. An + // ending does not have a placement or alignment, so those members are not written. Leave this + // alone to let the program place the bracket. PositionData positionData; // The length, in tenths of a staff space, of the downward jog at the end of the bracket. A stop - // ending closes with this jog; a discontinue ending has no jog. MusicXML does not say what the - // value means on a start ending, though some programs write one there. Leave it empty for the - // usual length. + // ending closes with this jog; a discontinue ending does not have one. MusicXML does not say + // what the value means on a start ending, though some programs write one there. Leave it empty + // for the usual length. std::optional endLength; // Where the label starts, in tenths of a staff space from the start of the bracket line: textX diff --git a/src/include/mx/api/ChordData.h b/src/include/mx/api/ChordData.h index 812ecc6a6..72f7276e5 100644 --- a/src/include/mx/api/ChordData.h +++ b/src/include/mx/api/ChordData.h @@ -248,7 +248,7 @@ class ChordData int rootAlter; // How the root is spelled in place of its letter name, such as "Do" for C in solfege or "H" for // B in German. Leave absent to print the letter. An empty string is different from absent: it - // prints nothing for the root, and the rest of the chord symbol still prints. Spell only the + // prints an empty root, and the rest of the chord symbol still prints. Spell only the // step, because rootAlter is still printed beside the text: B-flat in solfege is "Si" with a // rootAlter of -1, not "Sib". Used only when harmonyChordSource == root. std::optional rootText; @@ -289,10 +289,10 @@ class ChordData Bool parenthesesDegrees; Step bass; int bassAlter; - // How the bass note after the slash is spelled in place of its letter name, such as "Sol" for the - // G of C/G. Leave absent to print the letter. An empty string is different from absent: it - // prints nothing for the bass note. Spell only the step, because bassAlter is still printed - // beside the text. Used only when bass is set. + // How the bass note after the slash is spelled in place of its letter name, such as "Sol" for + // the G of C/G. Leave absent to print the letter. An empty string is different from absent: it + // prints an empty bass note. Spell only the step, because bassAlter is still printed beside the + // text. Used only when bass is set. std::optional bassText; // The MusicXML (0 = root position, 1 = first inversion, ...). Present only when // hasInversion is true. diff --git a/src/include/mx/api/CodaData.h b/src/include/mx/api/CodaData.h index 1a18b7177..096d934b6 100644 --- a/src/include/mx/api/CodaData.h +++ b/src/include/mx/api/CodaData.h @@ -19,8 +19,8 @@ namespace api { // The MusicXML element carries the empty-print-object-style-align attribute group plus the // smufl glyph-name and optional-unique-id. positionData captures default/relative x-y plus the -// horizontal and vertical alignment; its placement member is unused here because has no -// placement attribute (placement lives on the parent ). +// horizontal and vertical alignment; its placement member is unused here because does not +// have a placement attribute (placement lives on the parent ). class CodaData { public: diff --git a/src/include/mx/api/ComplexTimeSignature.h b/src/include/mx/api/ComplexTimeSignature.h index 8fb681339..70a2b536e 100644 --- a/src/include/mx/api/ComplexTimeSignature.h +++ b/src/include/mx/api/ComplexTimeSignature.h @@ -17,7 +17,7 @@ namespace mx namespace api { -// The full MusicXML time-symbol vocabulary. Used for a complex meter's own symbol and for an +// The full MusicXML time-symbol vocabulary. Used for a complex meter's symbol and for an // interchangeable pair's symbol. Unlike the narrow TimeSignatureSymbol (common/cut only), this // includes the unusual display modes that force a signature into the complex arena. enum class ComplexTimeSymbol @@ -30,8 +30,8 @@ enum class ComplexTimeSymbol dottedNote // the beat-type drawn as a dotted downstem note glyph }; -// The line drawn between beats and beat-type (MusicXML time-separator). 'unspecified' writes no -// attribute (the spec default is "none"). +// The line drawn between beats and beat-type (MusicXML time-separator). 'unspecified' does not +// write the attribute (the spec default is "none"). enum class TimeSeparator { unspecified, @@ -129,7 +129,7 @@ class ComplexTimeSignature // Construct a senza-misura time signature. // - // glyph is free text; MusicXML defines no valid/invalid values beyond xs:string (e.g. "X"). + // glyph is free text; MusicXML does not limit its values beyond xs:string (e.g. "X"). ComplexTimeSignature(std::string glyph); Kind kind() const; diff --git a/src/include/mx/api/DampData.h b/src/include/mx/api/DampData.h index 7870ef57d..f0ef41cfe 100644 --- a/src/include/mx/api/DampData.h +++ b/src/include/mx/api/DampData.h @@ -19,7 +19,7 @@ namespace api { // A harp damping mark, MusicXML's element: the player silences one or more ringing // strings with the hand. positionData captures default/relative x-y plus the horizontal and -// vertical alignment; its placement member is unused here because has no placement +// vertical alignment; its placement member is unused here because does not have a placement // attribute (placement lives on the parent ). class DampData { diff --git a/src/include/mx/api/Diagnostics.h b/src/include/mx/api/Diagnostics.h index 4b351a56e..057b82b45 100644 --- a/src/include/mx/api/Diagnostics.h +++ b/src/include/mx/api/Diagnostics.h @@ -27,7 +27,7 @@ enum class Severity enum class DiagnosticCode { valueAdjusted, // a value was changed to one that MusicXML can represent - unmatchedSpanner, // a spanner endpoint had no matching endpoint + unmatchedSpanner, // a spanner endpoint did not have a matching endpoint invalidValue, // a value could not be read, so a default was used missingValueDefaulted, // a required value was missing, so a default was used droppedData, // data could not be read or written, so it was left out diff --git a/src/include/mx/api/DirectionData.h b/src/include/mx/api/DirectionData.h index 809dee75b..69c88f423 100644 --- a/src/include/mx/api/DirectionData.h +++ b/src/include/mx/api/DirectionData.h @@ -35,7 +35,8 @@ struct DirectionData // draws it only on the top part of the system, alsoTop on both this part and the top part, and // none states explicitly that it belongs to this part alone. One value covers the direction // types and the chords held here; both elements are written with it. SystemRelation::onlyBottom - // and ::alsoBottom exist only for measure numbering; either one here writes no attribute. + // and ::alsoBottom exist only for measure numbering; either one here does not write the + // attribute. SystemRelation systemRelation; // Whether this direction lines up with the measure's time signature rather than with the @@ -47,12 +48,15 @@ struct DirectionData // alone. Bool directive; - // The source's , in divisions, or absent when it had none. An nudges only - // where the direction is *drawn*, shifting it away from the note it is anchored to; it does not - // move that anchor. tickTimePosition holds the anchor -- the musical location the direction - // belongs to -- so the drawn position is tickTimePosition + offset. This is a round-trip - // fidelity knob: leave it absent when authoring and set tickTimePosition to where the direction - // belongs; the reader fills it in only to preserve an already present in the source. + // The source's , in divisions, or absent when the source did not have one. An + // nudges only where the direction is *drawn*, shifting it away from the note it is anchored to; + // it does not move that anchor. tickTimePosition holds the anchor -- the musical location the + // direction belongs to -- so the drawn position is tickTimePosition + offset. + // + // This field exists for round-trip fidelity: leave it absent when authoring and set + // tickTimePosition to where the direction belongs. The reader fills it in only to keep an + // already present in the source. + // // MusicXML Documentation: An offset is represented in terms of divisions, and indicates where // the direction will appear relative to the current musical location. This affects the visual // appearance of the direction. If an element within a direction includes a default-x diff --git a/src/include/mx/api/DynamicsData.h b/src/include/mx/api/DynamicsData.h index c61e97cf2..fc51dc0e6 100644 --- a/src/include/mx/api/DynamicsData.h +++ b/src/include/mx/api/DynamicsData.h @@ -50,8 +50,8 @@ enum class StandardDynamic // The letters of the symbol, e.g. "ff" -- also the name of the MusicXML element that carries it. std::string toString(StandardDynamic value); -// A component of a dynamic mark that has no dedicated MusicXML dynamic element. text is the -// visible fallback; smufl, when present, names the exact glyph to draw. +// A component of a dynamic mark that does not have a dedicated MusicXML dynamic element. text is +// the visible fallback; smufl, when present, names the exact glyph to draw. struct OtherDynamicsData { std::string text; @@ -97,8 +97,8 @@ class DynamicsComponent MXAPI_NOT_EQUALS_AND_VECTORS(DynamicsComponent); -// The component's letters: the symbol for a standard component, the fallback text for one that has -// no dedicated MusicXML element. +// The component's letters: the symbol for a standard component, the fallback text for one that does +// not have a dedicated MusicXML element. std::string toString(const DynamicsComponent &value); // A dynamic mark assembled from multiple symbols in order, such as ff followed by z for ffz. diff --git a/src/include/mx/api/EncodingData.h b/src/include/mx/api/EncodingData.h index cbc44a353..e7ec29ed1 100644 --- a/src/include/mx/api/EncodingData.h +++ b/src/include/mx/api/EncodingData.h @@ -67,7 +67,7 @@ class EncodingData // the bad output back to the exact mx build that produced it. That stamp is // deliberately intrusive -- it injects a node the source never had into every // file mx writes -- so this flag exists to turn it off when byte-faithful output - // is needed (e.g. reproducing a defect without mx's own marker in the diff). + // is needed (e.g. reproducing a defect without mx's marker in the diff). // // Defaults true, including for files parsed without the stamp. When false, // writeToFile/writeToStream omit mx's node; the user's own entries are diff --git a/src/include/mx/api/EyeglassesData.h b/src/include/mx/api/EyeglassesData.h index 28a25140d..f5f71186b 100644 --- a/src/include/mx/api/EyeglassesData.h +++ b/src/include/mx/api/EyeglassesData.h @@ -17,11 +17,11 @@ namespace mx { namespace api { -// The eyeglasses symbol, MusicXML's element: a cue common in commercial music -// telling the player to watch the conductor (or another player) closely at this spot. -// positionData captures default/relative x-y plus the horizontal and vertical alignment; its -// placement member is unused here because has no placement attribute (placement -// lives on the parent ). +// The eyeglasses symbol, MusicXML's element: a cue common in commercial music telling +// the player to watch the conductor (or another player) closely at this spot. positionData captures +// default/relative x-y plus the horizontal and vertical alignment; its placement member is unused +// here because does not have a placement attribute (placement lives on the parent +// ). class EyeglassesData { public: diff --git a/src/include/mx/api/FiguredBassData.h b/src/include/mx/api/FiguredBassData.h index 55f33f23a..79d0b64c4 100644 --- a/src/include/mx/api/FiguredBassData.h +++ b/src/include/mx/api/FiguredBassData.h @@ -50,7 +50,7 @@ class FigureData std::string prefix; std::string figureNumber; std::string suffix; - // The figure's continuation line. Absent when the figure has none. + // The figure's continuation line. Absent when the figure does not have one. std::optional extend; FigureData() : prefix{}, figureNumber{}, suffix{}, extend{} @@ -68,7 +68,7 @@ MXAPI_NOT_EQUALS_AND_VECTORS(FigureData); // The MusicXML element: an ordered list of figures plus optional parentheses and an // optional duration. Figured bass takes its position from the first regular note that follows it in -// score order, so it carries no explicit tick position of its own here; it is attached to the +// score order, so it does not carry a tick position of its own here; it is attached to the // DirectionData whose tickTimePosition locates it. class FiguredBassData { diff --git a/src/include/mx/api/GlissandoData.h b/src/include/mx/api/GlissandoData.h index c6d387b68..d2a21c20a 100644 --- a/src/include/mx/api/GlissandoData.h +++ b/src/include/mx/api/GlissandoData.h @@ -42,8 +42,8 @@ struct GlissandoStart LineData lineData; // line-type, dash-length, space-length // Playback timing (MusicXML's trill-sound attributes). These exist on only -- a - // glissando has no accelerate/beats/first-beat/last-beat -- and are left at their defaults - // when glissandoType is glissando. + // glissando does not have accelerate/beats/first-beat/last-beat -- and are left at their + // defaults when glissandoType is glissando. Bool accelerate; std::optional beats; std::optional firstBeat; diff --git a/src/include/mx/api/HarpPedalsData.h b/src/include/mx/api/HarpPedalsData.h index 01de07a0f..94e67fa01 100644 --- a/src/include/mx/api/HarpPedalsData.h +++ b/src/include/mx/api/HarpPedalsData.h @@ -49,7 +49,7 @@ MXAPI_NOT_EQUALS_AND_VECTORS(HarpPedalTuning); // harp's seven pedals. A complete diagram lists all seven in the standard order of the harp's // pedal mechanism (D, C, B on the left foot; E, F, G, A on the right); pedalTunings preserves // the order given. positionData captures default/relative x-y plus the horizontal and vertical -// alignment; its placement member is unused here because has no placement +// alignment; its placement member is unused here because does not have a placement // attribute (placement lives on the parent ). class HarpPedalsData { diff --git a/src/include/mx/api/Id.h b/src/include/mx/api/Id.h index 7fdca905d..03dc947e0 100644 --- a/src/include/mx/api/Id.h +++ b/src/include/mx/api/Id.h @@ -41,7 +41,7 @@ namespace api // of one name, so Id cannot check it. An id you invent must not collide with one already in the // score. // -// To write no id attribute at all, leave the std::optional that holds the Id empty. +// To leave out the id attribute, leave the std::optional that holds the Id empty. // // Copy, assign, compare, sort, and hash an Id the way you would any other value. Two Ids are equal // when their text is equal, and std::hash is specialized for Id, so an Id works as a key in both diff --git a/src/include/mx/api/ImageData.h b/src/include/mx/api/ImageData.h index f90588403..3082f306d 100644 --- a/src/include/mx/api/ImageData.h +++ b/src/include/mx/api/ImageData.h @@ -15,13 +15,13 @@ namespace mx { namespace api { -// A graphic placed in the music, MusicXML's direction: source locates the graphic file -// (a URL or file reference) and type gives its MIME type, e.g. "image/png". height and width, -// in tenths, scale the image; give one alone to scale proportionally. positionData captures +// A graphic placed in the music, MusicXML's direction: source locates the graphic file (a +// URL or file reference) and type gives its MIME type, e.g. "image/png". height and width, in +// tenths, scale the image; give one alone to scale proportionally. positionData captures // default/relative x-y plus the horizontal and vertical alignment of the image relative to its -// position point; an image's vertical alignment has no baseline variant, so a baseline value -// here is not written. positionData's placement member is unused because has no -// placement attribute (placement lives on the parent ). +// position point; an image's vertical alignment does not have a baseline variant, so a baseline +// value here is not written. positionData's placement member is unused because does not +// have a placement attribute (placement lives on the parent ). class ImageData { public: diff --git a/src/include/mx/api/KeyData.h b/src/include/mx/api/KeyData.h index fc7e77655..e521ab786 100644 --- a/src/include/mx/api/KeyData.h +++ b/src/include/mx/api/KeyData.h @@ -19,7 +19,7 @@ namespace api // it is A minor, with dorian it is D dorian. The mode does not change the accidentals that are // drawn; fifths alone determines those. // -// `none` is the mode of music that has no tonal center. It is how a keyless staff is written: +// `none` is the mode of music without a tonal center. It is how a keyless staff is written: // zero fifths with none. It is a mode value like any other, and it says nothing about // how the signature is drawn. // @@ -107,7 +107,7 @@ struct KeyData CancelLocation cancelLocation; // The mode of the key signature (major, minor, dorian, none, and so on). It is optional: the - // default, KeyMode::unspecified, states no mode and writes no element. KeyMode::none + // default, KeyMode::unspecified, does not state a mode or write a element. KeyMode::none // is a stated mode, and it writes none. KeyMode mode; diff --git a/src/include/mx/api/LyricData.h b/src/include/mx/api/LyricData.h index 43f7bba35..1c4a0934d 100644 --- a/src/include/mx/api/LyricData.h +++ b/src/include/mx/api/LyricData.h @@ -38,10 +38,10 @@ enum class LyricExtendType stop }; -// One additional syllabic/text run appended to a , joined to the run before it by -// . A syllable is occasionally written as more than one run - for example a source -// that embeds a non-breaking space or an undertie (‿) inside otherwise ordinary lyric text. -// The common case of a lyric with a single run needs none of this; see LyricData::continuations. +// One additional syllabic/text run appended to a , joined to the run before it by . +// A syllable is occasionally written as more than one run - for example a source that embeds a +// non-breaking space or an undertie (‿) inside otherwise ordinary lyric text. The common case of a +// lyric with a single run does not need any of this; see LyricData::continuations. class LyricTextSegment { public: diff --git a/src/include/mx/api/MarkData.h b/src/include/mx/api/MarkData.h index cc4f33fc5..11369fc07 100644 --- a/src/include/mx/api/MarkData.h +++ b/src/include/mx/api/MarkData.h @@ -171,16 +171,14 @@ enum class MarkType fermataHalfCurve, // half-curve (MusicXML 4.0) fermataCurlew, // curlew (MusicXML 4.0) - // sadly, in MusicXML we do not have an 'above' 'below' attribute for the fermata. - // instead it has an attribute 'type' that is either 'upright' or 'inverted'. - // it appears that implementations interpret 'upright' to mean the 'normal' or - // correct orientation and 'inverted' to mean that the fermata should be 'flipped' - // from its correct orientation. in other words the implementation has to figure - // out if the fermata is above or below the note on its own (using its own - // algorithm combined with any default-y and relative-y values). this mess is - // going to be beyond the scope of the MusicXML Class Library at this time. You - // cause the fermata 'type' attribute to be set using the enum values below, but - // it's not possible for this library to calculate the correct fermata glyph. + // MusicXML does not have an 'above' or 'below' attribute for the fermata. Instead it has a + // 'type' attribute that is either 'upright' or 'inverted'. Implementations appear to read + // 'upright' as the normal orientation and 'inverted' as flipped from it, so each program + // works out whether the fermata is above or below the note (using its own algorithm along + // with any default-y and relative-y values). + // + // The enum values below set the fermata 'type' attribute, but this library does not work out + // which fermata glyph is correct. fermataUpright, // fermataNormalUpright, // normal fermataAngledUpright, // angled diff --git a/src/include/mx/api/MarkDataChoice.h b/src/include/mx/api/MarkDataChoice.h index 8418e25bc..14db76d86 100644 --- a/src/include/mx/api/MarkDataChoice.h +++ b/src/include/mx/api/MarkDataChoice.h @@ -32,15 +32,15 @@ struct TremoloMarkData // // A one-note tremolo states its slash count in the MarkType itself // (MarkType::tremoloSingleOne through MarkType::tremoloSingleFive), and an unmeasured tremolo - // has no slash count, so neither uses this field. + // does not have a slash count, so neither uses this field. std::optional tremoloMarks; // The canonical SMuFL glyph name to draw for an unmeasured tremolo (MarkType::tremoloUnmeasured), // from SMuFL's Tremolos range -- for example "pendereckiTremolo" or "unmeasuredTremolo". Leave // it absent to accept SMuFL's "buzzRoll", the glyph MusicXML assumes when no name is given. // - // MusicXML allows this on a one-note or two-note tremolo too, but defines no meaning for it - // there, and most applications ignore it. + // MusicXML allows this on a one-note or two-note tremolo too, but does not define a meaning + // for it there, and most applications ignore it. std::optional smufl; }; @@ -343,7 +343,7 @@ class MarkDataChoice // Builds a compound dynamic, unless the value is a single standard symbol, in which case the // result is a Kind::dynamic choice (auto-collapse). A lone other-dynamics symbol does not - // collapse -- it has no dedicated MusicXML element, so it stays a compound of one. + // collapse -- it does not have a dedicated MusicXML element, so it stays a compound of one. MarkDataChoice(CompoundDynamicsData value); MarkDataChoice(OtherNotationMarkData value); diff --git a/src/include/mx/api/NoteData.h b/src/include/mx/api/NoteData.h index f77eb716f..9973c3573 100644 --- a/src/include/mx/api/NoteData.h +++ b/src/include/mx/api/NoteData.h @@ -141,11 +141,11 @@ class NoteData // 's slash attribute. Only meaningful when isGrace is true. Bool graceSlash; - // A grace note has no written duration, so a player takes its time from a neighboring note or - // adds time of its own. The next three fields say how much. Leave them empty to let the player - // decide. They can be set alone or together, along with graceSlash, and like graceSlash they are - // ignored when isGrace is false. A steal time outside 0 to 100 is written as the nearer of 0 and - // 100, and a steal time that is not a number is not written. + // A grace note does not have a written duration, so a player takes its time from a neighboring + // note or adds time of its own. The next three fields say how much. Leave them empty to let the + // player decide. They can be set alone or together, along with graceSlash, and like graceSlash + // they are ignored when isGrace is false. A steal time outside 0 to 100 is written as the + // nearer of 0 and 100, and a steal time that is not a number is not written. // How much of the previous note's time this grace note takes, as a percentage from 0 to 100. // The previous note is cut short by that share and the grace note sounds in its place. @@ -171,11 +171,11 @@ class NoteData // The element's smufl attribute: the canonical SMuFL glyph name to draw for this // notehead, for example "noteheadSlashHorizontalEnds". It names a glyph directly, so a - // notation program can draw a notehead that MusicXML has no value for -- pair it with + // notation program can draw a notehead that MusicXML does not have a value for -- pair it with // Notehead::other -- or narrow one that MusicXML names only broadly, such as // Notehead::cluster. Note-name noteheads (the SMuFL ranges U+E150-U+E1AF and U+EEE0-U+EEFF) // are not written this way; MusicXML spells those out in . An empty string - // names no glyph and is written the same as leaving this empty. + // does not name a glyph and is written the same as leaving this empty. std::optional noteheadSmufl; // The element's parentheses attribute: whether the notehead is drawn inside diff --git a/src/include/mx/api/NoteRelationData.h b/src/include/mx/api/NoteRelationData.h index bfb02d871..608473b6c 100644 --- a/src/include/mx/api/NoteRelationData.h +++ b/src/include/mx/api/NoteRelationData.h @@ -59,7 +59,7 @@ struct MetronomeBeam // A tuplet drawn on a note figure inside a metric-relationship metronome mark: the // time-modification ratio -- actualNotes in the time of normalNotes, where a normal note is a // normalType note carrying normalDots dots -- plus how the bracket and numbers are shown. -// normalType unspecified means the ratio uses the figure's own note type. +// normalType unspecified means the ratio uses the figure's note type. struct MetronomeTuplet { int actualNotes; diff --git a/src/include/mx/api/OtherDirectionData.h b/src/include/mx/api/OtherDirectionData.h index 7eda94432..48f99d071 100644 --- a/src/include/mx/api/OtherDirectionData.h +++ b/src/include/mx/api/OtherDirectionData.h @@ -17,7 +17,7 @@ namespace mx { namespace api { -// A direction that has no dedicated MusicXML element, carried by the +// A direction that does not have a dedicated MusicXML element, carried by the // catch-all. The text is the direction's content; when the direction is best shown as a // standard SMuFL glyph, smufl names that glyph (a canonical SMuFL glyph name) and the text // serves as a fallback description. Set printObject to no for a direction that affects diff --git a/src/include/mx/api/OttavaData.h b/src/include/mx/api/OttavaData.h index a4628b509..1bd76c73a 100644 --- a/src/include/mx/api/OttavaData.h +++ b/src/include/mx/api/OttavaData.h @@ -34,7 +34,7 @@ class OttavaStart // Most users can ignore this; leave it false. It only controls whether an 8va/8vb line writes // the redundant size="8" attribute. The size (8, 15, or 22) follows from ottavaType, and 8 is // MusicXML's default, so a 15ma/15mb or 22ma/22mb line always writes its size while an 8va/8vb - // line omits it. When true, that redundant size="8" is written too; the flag has no effect on + // line omits it. When true, that redundant size="8" is written too; the flag does not affect // the larger lines, whose size is always written. It exists for round-trip fidelity - reading a // file sets it when the source spelled the attribute out. bool writeDefaultSize; @@ -51,7 +51,7 @@ class OttavaStop // Most users can ignore this; leave it unspecified. It only controls whether the stop end of an // octave-shift line repeats the line's size (8, 15, or 22). The size is not required on the - // stop and carries no new information. Omitting it is legal, but but some importers (MuseScore) + // stop and does not add information. Omitting it is legal, but some importers (MuseScore) // expect it. Therefore `mx::api` defaults to automatically writing the redundant stop size. // // For the sake of file round-trip fidelity, this will be set to 'no' if a the file being read diff --git a/src/include/mx/api/PageTextData.h b/src/include/mx/api/PageTextData.h index c9a55760c..f052179f2 100644 --- a/src/include/mx/api/PageTextData.h +++ b/src/include/mx/api/PageTextData.h @@ -65,8 +65,8 @@ class PageTextData // attributes on ``. HorizontalAlignment justify = HorizontalAlignment::unspecified; - // A shape drawn around the text. Enclosure::unspecified draws no enclosure; Enclosure::none - // states explicitly that there is none. + // A shape drawn around the text. Enclosure::unspecified does not draw an enclosure; + // Enclosure::none states explicitly that there is no enclosure. Enclosure enclosure = Enclosure::unspecified; // The stretches of text that follow `text` in the same credit, in order. Empty for a credit diff --git a/src/include/mx/api/PartData.h b/src/include/mx/api/PartData.h index c042812f4..6c0757754 100644 --- a/src/include/mx/api/PartData.h +++ b/src/include/mx/api/PartData.h @@ -23,24 +23,24 @@ struct MidiData std::string virtualLibrary; std::string virtualName; - // The name of the playback device or sound bank for this instrument — for example the name of a + // The name of the playback device or sound bank for this instrument, for example the name of a // MIDI output device, or a sound or bank label. This is often left empty. std::string device; // An optional port (or cable) number, from 1 to 16, that says which output the device uses. It - // has no value when the file doesn't specify one, and most files leave it unset — you only need + // is empty when the file doesn't specify one, and most files leave it unset; you only need // it when you are aiming at a particular MIDI port. You don't have to attach the device to this // part's instrument yourself; the library keeps them together automatically when the music is // written out. std::optional devicePort; // Most users can ignore this; leave it unspecified. It only controls whether the saved file - // spells out the little link that ties this part's playback device to its instrument. unspecified - // (the default) does the sensible thing on its own: it leaves that link out, because each part - // here has a single instrument and so the link isn't needed. yes/no force it to be written or left - // out. You never have to set this when you are creating music — when the library reads a file that - // did spell the link out, it sets this for you, so that saving the file brings it back the same way - // it came in. + // spells out the little link that ties this part's playback device to its instrument. + // unspecified (the default) does the sensible thing on its own: it leaves that link out, + // because each part here has a single instrument and so the link isn't needed. yes/no force it + // to be written or left out. You never have to set this when you are creating music. When the + // library reads a file that did spell the link out, it sets this for you, so that saving the + // file brings it back the same way it came in. Bool writeDeviceId; std::string name; @@ -85,13 +85,13 @@ struct MidiData // MusicXML itself allows a single part to carry more than one instrument. The most common case is // a drum kit or percussion part, where the bass drum, snare, ride cymbal, and so on are each // treated as a separate instrument. You also see it when a part layers or doubles two sounds that -// play together — two sampled instruments sounding at once, or a single "Clarinet 1 & 2" line that +// play together: two sampled instruments sounding at once, or a single "Clarinet 1 & 2" line that // really holds two separate instruments. // // For now this library brings in only the first instrument of such a part. Any additional // instruments in the file are not available here and are not written back out. We're sorry for the // limitation: we looked for a way to offer several instruments per part without making the ordinary -// single-instrument case — which covers the vast majority of music — harder to understand and use, +// single-instrument case (which covers the vast majority of music) harder to understand and use, // and we didn't find one we were happy with, so it stays one instrument per part for the time // being. // diff --git a/src/include/mx/api/PedalLineData.h b/src/include/mx/api/PedalLineData.h index 40cce86df..d85fba7ea 100644 --- a/src/include/mx/api/PedalLineData.h +++ b/src/include/mx/api/PedalLineData.h @@ -56,11 +56,11 @@ struct PedalLineData PositionData positionData; // Which line this event belongs to when two pedal lines are held down at once; MusicXML calls - // it the number. It can be left unspecified, and usually should be: a lone line, and any run - // of lines that do not overlap, needs no number. A number is only needed to tell two lines + // it the number. It can be left unspecified, and usually should be: a lone line, and any run of + // lines that do not overlap, does not need a number. A number is only needed to tell two lines // apart when they overlap -- when a line opens before the one before it has closed, as a - // sostenuto line under a damper line does -- because otherwise a lift cannot be matched to - // the right downstroke. In non-ambiguous cases, leaving this unset is fine. + // sostenuto line under a damper line does -- because otherwise a lift cannot be matched to the + // right downstroke. In non-ambiguous cases, leaving this unset is fine. SpannerNumber number; // The element's id attribute (see Id.h). diff --git a/src/include/mx/api/PercussionData.h b/src/include/mx/api/PercussionData.h index a2ea4abb5..372ffc70e 100644 --- a/src/include/mx/api/PercussionData.h +++ b/src/include/mx/api/PercussionData.h @@ -465,19 +465,19 @@ class PercussionDataChoice MXAPI_NOT_EQUALS_AND_VECTORS(PercussionDataChoice); -// A percussion pictogram, MusicXML's element: a symbol for an unpitched instrument -// or the implement striking it, used in percussion parts and legends. Several PercussionData in -// one direction read as a group (for example a membrane pictogram followed by a beater). -// positionData captures default/relative x-y plus the horizontal and vertical alignment; its -// placement member is unused here because has no placement attribute (placement -// lives on the parent ). +// A percussion pictogram, MusicXML's element: a symbol for an unpitched instrument or +// the implement striking it, used in percussion parts and legends. Several PercussionData in one +// direction read as a group (for example a membrane pictogram followed by a beater). positionData +// captures default/relative x-y plus the horizontal and vertical alignment; its placement member is +// unused here because does not have a placement attribute (placement lives on the +// parent ). class PercussionData { public: PercussionDataChoice choice; - // A shape drawn around the pictogram. Enclosure::unspecified draws no enclosure; - // Enclosure::none states explicitly that there is none. + // A shape drawn around the pictogram. Enclosure::unspecified does not draw an enclosure; + // Enclosure::none states explicitly that there is no enclosure. Enclosure enclosure; PositionData positionData; diff --git a/src/include/mx/api/RehearsalData.h b/src/include/mx/api/RehearsalData.h index e299c6962..a1ae02108 100644 --- a/src/include/mx/api/RehearsalData.h +++ b/src/include/mx/api/RehearsalData.h @@ -26,8 +26,8 @@ class RehearsalData ColorData colorData; FontData fontData; - // A shape drawn around the text. Enclosure::unspecified draws no enclosure; Enclosure::none - // states explicitly that there is none. + // A shape drawn around the text. Enclosure::unspecified does not draw an enclosure; + // Enclosure::none states explicitly that there is no enclosure. Enclosure enclosure; // The `justify` attribute of ``: how the mark's lines sit relative to each other diff --git a/src/include/mx/api/Result.h b/src/include/mx/api/Result.h index baf1a33aa..1ed07832f 100644 --- a/src/include/mx/api/Result.h +++ b/src/include/mx/api/Result.h @@ -18,8 +18,8 @@ namespace api // The mx::api error vocabulary. mx::api owns its own codes: the core-boundary // failures are mirrored (public headers never -// include private mx::core headers), and the api adds the codes core has no -// business knowing. No exceptions escape the MusicXml boundary. +// include private mx::core headers), and the api adds the codes that are not +// core's business. No exceptions escape the MusicXml boundary. enum class ResultCode { ioError, // file open/read/write failure (api-level) diff --git a/src/include/mx/api/ScoreData.h b/src/include/mx/api/ScoreData.h index 38ddb1308..ec351de56 100644 --- a/src/include/mx/api/ScoreData.h +++ b/src/include/mx/api/ScoreData.h @@ -115,9 +115,9 @@ class ScoreData // entry with no role does the same for composer when no one is credited as composer. std::vector additionalCreators; - // The score's rights statement, usually its copyright notice. When a score has several, this - // is the first one, whatever it covers. Leave it empty when the score has none. MusicXML calls - // this . + // The score's rights statement, usually its copyright notice. When a score has several, this is + // the first one, whatever it covers. Leave it empty when the score does not have one. MusicXML + // calls this . std::string copyright; // What kind of statement `copyright` is, or what it covers: "copyright" (the default), or for @@ -155,7 +155,7 @@ class ScoreData /// reports any other transposition as a diagnostic, without changing it. /// /// A score that has any ForPartData is always written as a concert score, so you only need to - /// set this for a concert score that has no transposing instruments. + /// set this for a concert score without transposing instruments. bool concertScore; std::vector parts; @@ -164,8 +164,8 @@ class ScoreData /// The score's time resolution: how many `tickTimePosition`/duration ticks make up one /// quarter note. Corresponds to the `` MusicXML element. Choose a value with /// enough factors for the durations you need (the default accommodates tuplets through - /// septuplets). Zero means the score has no time resolution at all — no durations exist - /// anywhere — and no `` is written. + /// septuplets). Zero means the score does not have a time resolution: no durations exist + /// anywhere, and no `` is written. int ticksPerQuarter; /// Specifies page breaks, system breaks, and changes to system and page layout. Global/default page and diff --git a/src/include/mx/api/SegnoData.h b/src/include/mx/api/SegnoData.h index 880932d9c..bdbd78a8c 100644 --- a/src/include/mx/api/SegnoData.h +++ b/src/include/mx/api/SegnoData.h @@ -19,8 +19,8 @@ namespace api { // The MusicXML element carries the empty-print-object-style-align attribute group plus the // smufl glyph-name and optional-unique-id. positionData captures default/relative x-y plus the -// horizontal and vertical alignment; its placement member is unused here because has no -// placement attribute (placement lives on the parent ). +// horizontal and vertical alignment; its placement member is unused here because does not +// have a placement attribute (placement lives on the parent ). class SegnoData { public: diff --git a/src/include/mx/api/StaffDivideData.h b/src/include/mx/api/StaffDivideData.h index fb5644e16..119e45602 100644 --- a/src/include/mx/api/StaffDivideData.h +++ b/src/include/mx/api/StaffDivideData.h @@ -25,11 +25,11 @@ enum class StaffDivideType upDown }; -// A staff-divide arrow, MusicXML's element (MusicXML 3.1): the arrow symbol used -// in condensed conductor scores where one staff carries two players, showing whether the music +// A staff-divide arrow, MusicXML's element (MusicXML 3.1): the arrow symbol used in +// condensed conductor scores where one staff carries two players, showing whether the music // continues on the staff below (down), above (up), or both (up-down). positionData captures -// default/relative x-y plus the horizontal and vertical alignment; its placement member is -// unused here because has no placement attribute (placement lives on the parent +// default/relative x-y plus the horizontal and vertical alignment; its placement member is unused +// here because does not have a placement attribute (placement lives on the parent // ). class StaffDivideData { diff --git a/src/include/mx/api/StringMuteData.h b/src/include/mx/api/StringMuteData.h index 5c2414867..e2ee3781b 100644 --- a/src/include/mx/api/StringMuteData.h +++ b/src/include/mx/api/StringMuteData.h @@ -27,7 +27,7 @@ enum class StringMuteType // A string mute change, MusicXML's element: the "mute on" or "mute off" symbol // telling string players to apply or remove their mutes. positionData captures default/relative // x-y plus the horizontal and vertical alignment; its placement member is unused here because -// has no placement attribute (placement lives on the parent ). +// does not have a placement attribute (placement lives on the parent ). class StringMuteData { public: diff --git a/src/include/mx/api/SymbolData.h b/src/include/mx/api/SymbolData.h index ad9d65eb0..0d7496f28 100644 --- a/src/include/mx/api/SymbolData.h +++ b/src/include/mx/api/SymbolData.h @@ -32,8 +32,8 @@ class SymbolData FontData fontData; std::optional color; - // A shape drawn around the symbol. Enclosure::unspecified draws no enclosure; Enclosure::none - // states explicitly that there is none. + // A shape drawn around the symbol. Enclosure::unspecified does not draw an enclosure; + // Enclosure::none states explicitly that there is no enclosure. Enclosure enclosure; // The `justify` attribute of ``. `unspecified` means the attribute is absent. Distinct diff --git a/src/include/mx/api/TimeChoice.h b/src/include/mx/api/TimeChoice.h index e1f742dcd..c8687174e 100644 --- a/src/include/mx/api/TimeChoice.h +++ b/src/include/mx/api/TimeChoice.h @@ -61,7 +61,7 @@ class TimeChoice Bool display{Bool::unspecified}; // The