Skip to content

api: fix notehead value fa up silently missing#177

Merged
webern merged 1 commit into
mainfrom
claude/eloquent-euler-dvndnf
Jun 16, 2026
Merged

api: fix notehead value fa up silently missing#177
webern merged 1 commit into
mainfrom
claude/eloquent-euler-dvndnf

Conversation

@webern

@webern webern commented Jun 16, 2026

Copy link
Copy Markdown
Owner

Summary

  • noteheadMap in Converter.cpp was missing the faUp entry, so any MusicXML note with <notehead>fa up</notehead> was silently read back as Notehead::none (data loss)
  • Add the single missing row: {core::NoteheadValue::faUp(), api::Notehead::faUp}
  • Add a round-trip test (noteheadFaUpRoundtrip) that verifies both the write path (serialized XML contains "fa up") and the read path (notehead survives deserialization as faUp)

Test plan

  • make test passes (219 test cases, 3917 assertions — all green)
  • make fmt / make check passes (no format diff)
  • New test noteheadFaUpRoundtrip_NoteData confirmed RED before fix, GREEN after

`noteheadMap` in Converter.cpp was missing the faUp entry, so any note
with a <notehead>fa up</notehead> in MusicXML was read back as
Notehead::none (silent data loss). Add the missing row and a round-trip
test that verifies both the write path (XML contains "fa up") and the
read path (notehead survives the trip as faUp).
@github-actions

Copy link
Copy Markdown

gen-quality gen/

gen-quality: 84.5 / 100   (floor 84.5, +0.0)

  structure     86.5  x0.50   [fn 90.5 / file 82.6]
  cyclomatic    88.4  x0.25
  cognitive     76.6  x0.25

  409 functions across 31 files, 7702 lines (largest file 1044)
  max cc 56  max cognitive 44  max fn loc 152

Worst offenders (top 5 per axis; full lists in score.json):
  cyclomatic gen/xsd/analyze.py:311     report                             56
  cyclomatic gen/plates/build.py:956    _validate_config_against_ir        35
  cyclomatic gen/press/context.py:145   plate_context                      34
  cyclomatic gen/__main__.py:46         _ir                                23
  cyclomatic gen/tests/test_ir.py:102   _check_references                  20
  cognitive  gen/xsd/analyze.py:311     report                             44
  cognitive  gen/ir/resolve.py:119      flat_elements                      40
  cognitive  gen/tests/test_ir.py:102   _check_references                  38
  cognitive  gen/press/context.py:145   plate_context                      37
  cognitive  gen/xsd/analyze.py:207     _sccs                              37
  size       gen/xsd/analyze.py:311     report                             152
  size       gen/press/context.py:145   plate_context                      96
  size       gen/plates/build.py:533    _value_plate                       89
  size       gen/plates/build.py:956    _validate_config_against_ir        89
  size       gen/ir/resolve.py:119      flat_elements                      78

Commit f3f9d14e2eac6a802f2bfa6eea887e9da81a8b23.

@github-actions

Copy link
Copy Markdown

Coverage report

Core-dev coverage src/private/mx/core/

Metric Coverage Covered / Total
Lines 77.9% 28539 / 36619
Functions 74.4% 6360 / 8547
Branches 50.7% 22672 / 44725

API coverage src/private/mx/{api,impl,utility}/

Metric Coverage Covered / Total
Lines 68.7% 4735 / 6896
Functions 54.8% 1475 / 2692
Branches 40.3% 3850 / 9558

Core HTML report | API HTML report

Commit f3f9d14e2eac6a802f2bfa6eea887e9da81a8b23.

@webern webern changed the title Fix NoteheadValue::faUp silently mapping to Notehead::none api: fix notehead value fa up silently missing Jun 16, 2026
@webern webern merged commit 69fde78 into main Jun 16, 2026
7 checks passed
@webern webern deleted the claude/eloquent-euler-dvndnf branch June 16, 2026 20:16
webern added a commit that referenced this pull request Jun 16, 2026
## Summary

The `api-feature-audit` doc now doubles as a tracker. Added a Tracker
table mapping every audited
item to its GitHub issue or PR, plus a pointer near the top to the
parent tracker issue.

The three section-1a enum bugs are marked done against their fix PRs
(#177, #178, #179). The
remaining 16 items now have issues (#181-#196), all linked as sub-issues
of #159, which was rewritten
into a parent tracker with live checkboxes.

## References

- Progresses #159
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant