From 6f93fd1aaef3363e7684b187dec65594e2643a80 Mon Sep 17 00:00:00 2001 From: Felipe Martins Date: Tue, 28 Apr 2026 18:53:26 +0200 Subject: [PATCH 1/2] fix: clamp harmony inversion to valid range when quality changes When the chord quality is changed to one with fewer available inversions (e.g. from dominant-seventh to major triad), music21 raised ChordException if the stored inversion exceeded the new maximum. Clamping the inversion via get_inversion_amount() before constructing ChordSymbol and to_roman_numeral() prevents the crash. Closes #468. --- tilia/ui/timelines/harmony/elements/harmony.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tilia/ui/timelines/harmony/elements/harmony.py b/tilia/ui/timelines/harmony/elements/harmony.py index 6f7bfc7b9..9f193f48d 100644 --- a/tilia/ui/timelines/harmony/elements/harmony.py +++ b/tilia/ui/timelines/harmony/elements/harmony.py @@ -10,6 +10,7 @@ from tilia.ui.coords import time_x_converter from tilia.ui.timelines.base.element import TimelineUIElement from tilia.ui.timelines.drag import DragManager +from tilia.timelines.harmony.constants import get_inversion_amount from tilia.ui.timelines.harmony.constants import ( INT_TO_NOTE_NAME, INT_TO_ROMAN, @@ -71,14 +72,16 @@ def key(self): @property def letter_symbol(self): + quality = self.get_data("quality") + inversion = min(self.get_data("inversion"), get_inversion_amount(quality)) symbol = music21.harmony.ChordSymbol( INT_TO_NOTE_NAME[self.get_data("step")] + Accidental.get_from_int( "music21", self.get_data("accidental"), ) - + QUALITY_TO_ABBREVIATION[self.get_data("quality")], - inversion=self.get_data("inversion"), + + QUALITY_TO_ABBREVIATION[quality], + inversion=inversion, ) applied_to = self.get_data("applied_to") if applied_to: @@ -113,13 +116,14 @@ def alternate_label(self): @property def roman_numeral_label(self): + quality = self.get_data("quality") return to_roman_numeral( self.get_data("step"), self.get_data("accidental"), - self.get_data("quality"), + quality, self.key, self.get_data("applied_to"), - self.get_data("inversion"), + min(self.get_data("inversion"), get_inversion_amount(quality)), ) @property From 47c22bb60a8904b3cda3ecdeb54fad57d7f9f141 Mon Sep 17 00:00:00 2001 From: Felipe Martins Date: Wed, 6 May 2026 18:01:29 +0200 Subject: [PATCH 2/2] fix: also clamp inversion in letter_symbol_label suffix The previous fix clamped inversion when constructing ChordSymbol and when calling to_roman_numeral(), but letter_symbol_label still appended '/&' using the unclamped stored value. So after switching from dominant-seventh + 3rd inversion to a triad, the rendered label read 'C/&7' even though a triad has no 3rd inversion. Extracts clamping into a clamped_inversion property and uses it in all three rendering paths so the display stays consistent. --- tests/ui/timelines/harmony/test_harmony_ui.py | 22 +++++++++++++++++++ .../ui/timelines/harmony/elements/harmony.py | 22 +++++++++++-------- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/tests/ui/timelines/harmony/test_harmony_ui.py b/tests/ui/timelines/harmony/test_harmony_ui.py index 5a2ddebf9..d9c3b4a30 100644 --- a/tests/ui/timelines/harmony/test_harmony_ui.py +++ b/tests/ui/timelines/harmony/test_harmony_ui.py @@ -23,6 +23,28 @@ def test_right_click(self, tlui): exec_mock.assert_called_once() +class TestInversionClamp: + def test_letter_symbol_does_not_crash_when_quality_loses_inversion(self, tlui): + harmony, _ = tlui.create_harmony(0, quality="dominant-seventh", inversion=3) + tlui.timeline.set_component_data(harmony.id, "quality", "major") + # Pre-fix: music21.chord.ChordException — no 3rd inversion on triad. + _ = tlui.get_element(harmony.id).letter_symbol + + def test_roman_numeral_label_does_not_crash_when_quality_loses_inversion( + self, tlui + ): + harmony, _ = tlui.create_harmony(0, quality="dominant-seventh", inversion=3) + tlui.timeline.set_component_data(harmony.id, "quality", "major") + _ = tlui.get_element(harmony.id).roman_numeral_label + + def test_letter_symbol_label_omits_dropped_inversion(self, tlui): + harmony, _ = tlui.create_harmony(0, quality="dominant-seventh", inversion=3) + tlui.timeline.set_component_data(harmony.id, "quality", "major") + # Major triad max inversion is 2, so the 3rd-inversion suffix + # (INVERSION_TO_INTERVAL[3] == 7) must not appear in the label. + assert "/&7" not in tlui.get_element(harmony.id).letter_symbol_label + + class TestCopyPaste: def test_paste_single_into_timeline(self, tlui, tilia_state): _, hui = tlui.create_harmony(0) diff --git a/tilia/ui/timelines/harmony/elements/harmony.py b/tilia/ui/timelines/harmony/elements/harmony.py index 9f193f48d..9c8274735 100644 --- a/tilia/ui/timelines/harmony/elements/harmony.py +++ b/tilia/ui/timelines/harmony/elements/harmony.py @@ -7,10 +7,10 @@ from PySide6.QtWidgets import QGraphicsItem, QGraphicsTextItem from tilia.requests import Get, Post, get, post +from tilia.timelines.harmony.constants import get_inversion_amount from tilia.ui.coords import time_x_converter from tilia.ui.timelines.base.element import TimelineUIElement from tilia.ui.timelines.drag import DragManager -from tilia.timelines.harmony.constants import get_inversion_amount from tilia.ui.timelines.harmony.constants import ( INT_TO_NOTE_NAME, INT_TO_ROMAN, @@ -70,18 +70,23 @@ def font_type(self): def key(self): return self.timeline_ui.get_key_by_time(self.get_data("time")) + @property + def clamped_inversion(self): + return min( + self.get_data("inversion"), + get_inversion_amount(self.get_data("quality")), + ) + @property def letter_symbol(self): - quality = self.get_data("quality") - inversion = min(self.get_data("inversion"), get_inversion_amount(quality)) symbol = music21.harmony.ChordSymbol( INT_TO_NOTE_NAME[self.get_data("step")] + Accidental.get_from_int( "music21", self.get_data("accidental"), ) - + QUALITY_TO_ABBREVIATION[quality], - inversion=inversion, + + QUALITY_TO_ABBREVIATION[self.get_data("quality")], + inversion=self.clamped_inversion, ) applied_to = self.get_data("applied_to") if applied_to: @@ -116,14 +121,13 @@ def alternate_label(self): @property def roman_numeral_label(self): - quality = self.get_data("quality") return to_roman_numeral( self.get_data("step"), self.get_data("accidental"), - quality, + self.get_data("quality"), self.key, self.get_data("applied_to"), - min(self.get_data("inversion"), get_inversion_amount(quality)), + self.clamped_inversion, ) @property @@ -154,7 +158,7 @@ def letter_symbol_label(self): case 2: figure = figure.replace("##", "`#`#") - if inversion := self.get_data("inversion"): + if inversion := self.clamped_inversion: # bass_step = harmony.calculate.bass_step(self.get_data('step'), inversion) # figure += '/' + INT_TO_NOTE_NAME[bass_step] # TODO calculate bass note figure += "/&" + str(INVERSION_TO_INTERVAL[inversion])