From 3b6a9a2902bb4758dd87657cefe5dbe66cd0ae08 Mon Sep 17 00:00:00 2001 From: Felipe Martins Date: Thu, 25 Jun 2026 15:16:20 +0200 Subject: [PATCH 1/3] fix: use full chord type for roman-numeral sevenths in harmony dialog 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 7a76fdc6 ("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. --- .../ui/dialogs/test_select_harmony_params.py | 34 +++++++++++++++++++ tilia/timelines/harmony/components/harmony.py | 2 +- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/tests/ui/dialogs/test_select_harmony_params.py b/tests/ui/dialogs/test_select_harmony_params.py index 0c7a85963..849c6164c 100644 --- a/tests/ui/dialogs/test_select_harmony_params.py +++ b/tests/ui/dialogs/test_select_harmony_params.py @@ -94,3 +94,37 @@ def test_parse_with_extensions_does_not_crash(self, extension, qtui): # extensions are not currently supported params = parse_text("C" + extension) assert params["step"] == 0 + + +class TestRomanNumeralParsing: + # Regression guard: a roman-numeral seventh used to select the triad + # quality (e.g. "V7" -> "major") because the parser read music21's + # `impliedQuality`, which only reflects the triad, instead of the full + # chord type computed from the chord's pitches. + @pytest.mark.parametrize( + "text, quality, step, inversion", + [ + ("V", "major", 4, 0), + ("V7", "dominant-seventh", 4, 0), + ("V65", "dominant-seventh", 4, 1), + ("V43", "dominant-seventh", 4, 2), + ("V42", "dominant-seventh", 4, 3), + ("ii7", "minor-seventh", 1, 0), + ("viio7", "diminished-seventh", 6, 0), + ], + ) + def test_quality_step_and_inversion(self, text, quality, step, inversion, qtui): + params = parse_text(text) + assert params["quality"] == quality + assert params["step"] == step + assert params["inversion"] == inversion + + def test_seventh_quality_is_not_collapsed_to_triad(self, qtui): + # Direct guard for the reported bug: typing "V7" selected "Major". + assert parse_text("V7")["quality"] == "dominant-seventh" + + def test_applied_seventh(self, qtui): + params = parse_text("V7/V") + assert params["quality"] == "dominant-seventh" + assert params["applied_to"] == 4 + assert params["step"] == 1 diff --git a/tilia/timelines/harmony/components/harmony.py b/tilia/timelines/harmony/components/harmony.py index 1c08a1262..6f0f515b3 100644 --- a/tilia/timelines/harmony/components/harmony.py +++ b/tilia/timelines/harmony/components/harmony.py @@ -194,7 +194,7 @@ def _get_params_from_music21_object( accidental = int(obj.root().alter) inversion = obj.inversion() if obj.inversion() else 0 if kind == "roman": - quality = obj.impliedQuality + quality = obj.letter_type applied_to = ( ROMAN_TO_INT[obj.secondaryRomanNumeral.figure.upper()] if obj.secondaryRomanNumeral From a3bb6df2b9d0a85e658107b0d2a11e30dc8f6948 Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Tue, 21 Jul 2026 11:25:26 +0200 Subject: [PATCH 2/3] fix: don't monkey patch letter_type does not actually exist on m21's RomanNumeral. RomanNumeral is also a subclass of Chord, so this removes the extra conversion. --- tilia/timelines/harmony/components/harmony.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tilia/timelines/harmony/components/harmony.py b/tilia/timelines/harmony/components/harmony.py index 6f0f515b3..45f72c808 100644 --- a/tilia/timelines/harmony/components/harmony.py +++ b/tilia/timelines/harmony/components/harmony.py @@ -178,8 +178,8 @@ def _get_music21_object_from_text( elif text.startswith(("I", "i", "V", "v")): try: roman_numeral = music21.roman.RomanNumeral(prefixed_accidental + text, key) - letter_common_name = music21.chord.Chord(roman_numeral.pitches).commonName - roman_numeral.letter_type = CHORD_COMMON_NAME_TO_TYPE[letter_common_name] + if roman_numeral.commonName not in CHORD_COMMON_NAME_TO_TYPE: + raise KeyError(roman_numeral.commonName) return roman_numeral, "roman" except (ValueError, KeyError): pass @@ -194,7 +194,7 @@ def _get_params_from_music21_object( accidental = int(obj.root().alter) inversion = obj.inversion() if obj.inversion() else 0 if kind == "roman": - quality = obj.letter_type + quality = CHORD_COMMON_NAME_TO_TYPE[obj.commonName] applied_to = ( ROMAN_TO_INT[obj.secondaryRomanNumeral.figure.upper()] if obj.secondaryRomanNumeral From 9dcf3bf5e86f8567185b0331a6fc60b2e47a8b1e Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Tue, 21 Jul 2026 11:42:56 +0200 Subject: [PATCH 3/3] chore: black --- tests/ui/timelines/hierarchy/test_hierarchy_timeline_ui.py | 4 +--- tilia/ui/qtui.py | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/ui/timelines/hierarchy/test_hierarchy_timeline_ui.py b/tests/ui/timelines/hierarchy/test_hierarchy_timeline_ui.py index a02639b1c..935f6f052 100644 --- a/tests/ui/timelines/hierarchy/test_hierarchy_timeline_ui.py +++ b/tests/ui/timelines/hierarchy/test_hierarchy_timeline_ui.py @@ -232,9 +232,7 @@ def test_pre_start_not_in_menu_when_room_below_min_length(self, tlui): menu = HierarchyContextMenu(tlui[0]) assert self.PRE_START not in get_command_names(menu) - def test_post_end_not_in_menu_when_room_below_min_length( - self, tlui, tilia_state - ): + def test_post_end_not_in_menu_when_room_below_min_length(self, tlui, tilia_state): tilia_state.duration = 100 tlui.create_hierarchy( 0, tilia_state.duration - HierarchyUI.MIN_FRAME_LENGTH / 2, 1 diff --git a/tilia/ui/qtui.py b/tilia/ui/qtui.py index fa0f4e534..f43128027 100644 --- a/tilia/ui/qtui.py +++ b/tilia/ui/qtui.py @@ -96,7 +96,6 @@ def _tilia_theme_name() -> str: scheme = QApplication.styleHints().colorScheme() return "tiliaDark" if scheme == Qt.ColorScheme.Dark else "tiliaLight" - def keyPressEvent(self, event: QtGui.QKeyEvent) -> None: if event is None: return