fix: use full chord type for roman-numeral sevenths in Add harmony dialog#552
Merged
azfoo merged 3 commits intoJul 21, 2026
Merged
Conversation
Typing a roman-numeral seventh (e.g. "V7") in the Add harmony dialog
selected the triad quality ("Major") instead of the seventh quality
("Dominant seventh").
The roman-numeral parse path read music21's RomanNumeral.impliedQuality,
which only reflects the triad. Commit 7a76fdc ("more accurate naming")
renamed the attribute holding the full chord type from chord_type to
letter_type, but changed the reader to impliedQuality instead of
letter_type, leaving letter_type set-but-unread. Read letter_type so the
full chord type (derived from the chord's pitches) is used.
Adds regression tests covering V/V7/V65/V43/V42, ii7, viio7, and V7/V.
letter_type does not actually exist on m21's RomanNumeral. RomanNumeral is also a subclass of Chord, so this removes the extra conversion.
azfoo
force-pushed
the
fix/harmony-roman-seventh-quality
branch
from
July 21, 2026 09:43
c3fb3d8 to
9dcf3bf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Warning
AI-generated — needs verifying. This investigation, fix, and tests were produced by an AI agent (Claude Code) and have not been independently verified by a human. Please review carefully before merging — see the checklist at the bottom.
Summary
Typing a roman-numeral seventh chord (e.g.
V7) in the Add harmony dialog selected the triad quality (Major) instead of the seventh quality (Dominant seventh). Reported against v0.6.3.Root cause
The roman-numeral parse path in
_get_params_from_music21_object(tilia/timelines/harmony/components/harmony.py) read music21'sRomanNumeral.impliedQuality, which only reflects the triad quality (major/minor/diminished/augmented) and ignores the seventh/extension. SoV7resolved tomajor, and the dialog's quality combobox selected "Major".This is a regression from commit 7a76fdc ("chore: more accurate naming"), a
chord_* → letter_*rename. It correctly renamed the set side (roman_numeral.chord_type→roman_numeral.letter_type), but on the read side changedobj.chord_typetoobj.impliedQualityinstead ofobj.letter_type. That leftletter_typeset-but-never-read, and the reader silently picked up music21's triad-only attribute.letter_typeis computed from the chord's actual pitches (Chord(roman.pitches).commonName→CHORD_COMMON_NAME_TO_TYPE), so it carries the correct full chord type.Fix
One line:
Tests
Adds
TestRomanNumeralParsingtotests/ui/dialogs/test_select_harmony_params.py. It drives the dialog the way a user types (char-by-char into the line edit via the existingparse_texthelper) and asserts on the resulting combobox selection — i.e. it exercises the exact reported symptom, not just the parse function.Cases:
V(triad, unchanged),V7,V65,V43,V42,ii7,viio7, and the applied chordV7/V. Confirmed these fail on the buggy code (assert 'major' == 'dominant-seventh') and pass with the fix. Full harmony suite (171 tests) green; ruff clean.Reviewer checklist (AI output — please verify)
letter_typeis always set whenever the parse path returnskind == "roman"(it is assigned inside thetryblock in_get_music21_object_from_text).letter-typeimpliedQualitybehavior for roman numerals.