diff --git a/repro/harmony-roman-s.tla b/repro/harmony-roman-s.tla new file mode 100644 index 000000000..6505d3d91 --- /dev/null +++ b/repro/harmony-roman-s.tla @@ -0,0 +1,184 @@ +{ + "file_path": "C:/Users/Felipe/dev/tilia-desktop/.claude/worktrees/tilia-app-dns-fix-cb44c6/repro/harmony-roman-s.tla", + "media_path": "", + "media_metadata": { + "title": "harmony-roman-s", + "notes": "", + "media length": 100, + "composer": "", + "tonality": "", + "time signature": "", + "performer": "", + "performance year": "", + "arranger": "", + "composition year": "", + "recording year": "", + "form": "", + "instrumentation": "", + "genre": "", + "lyrics": "" + }, + "timelines": { + "0": { + "kind": "HARMONY_TIMELINE", + "level_count": 1, + "level_height": 35, + "is_visible": true, + "name": "", + "ordinal": 1, + "visible_level_count": 2, + "hash": "46fa9e532d92228f5688fb04a2cc17e9", + "components": { + "2": { + "time": 0.0, + "comments": "", + "step": 0, + "accidental": 0, + "quality": "major", + "inversion": 0, + "applied_to": 0, + "level": 1, + "display_mode": "roman", + "custom_text": "", + "custom_text_font_type": "analytic", + "kind": "HARMONY", + "hash": "6c48851babc6afa48936cdf65180bd61" + }, + "3": { + "time": 10, + "comments": "", + "step": 0, + "accidental": 0, + "quality": "major", + "inversion": 1, + "applied_to": 0, + "level": 1, + "display_mode": "roman", + "custom_text": "", + "custom_text_font_type": "analytic", + "kind": "HARMONY", + "hash": "ca8fd969194756510648e632ea0be4a0" + }, + "4": { + "time": 20, + "comments": "", + "step": 0, + "accidental": 0, + "quality": "major", + "inversion": 2, + "applied_to": 0, + "level": 1, + "display_mode": "roman", + "custom_text": "", + "custom_text_font_type": "analytic", + "kind": "HARMONY", + "hash": "2a58a41503d3d03fd02087b0738a5b58" + }, + "5": { + "time": 30, + "comments": "", + "step": 4, + "accidental": 0, + "quality": "dominant-seventh", + "inversion": 1, + "applied_to": 0, + "level": 1, + "display_mode": "roman", + "custom_text": "", + "custom_text_font_type": "analytic", + "kind": "HARMONY", + "hash": "d522cbc0c607b8b4104f947f3c10a4f4" + }, + "6": { + "time": 40, + "comments": "", + "step": 4, + "accidental": 0, + "quality": "dominant-seventh", + "inversion": 2, + "applied_to": 0, + "level": 1, + "display_mode": "roman", + "custom_text": "", + "custom_text_font_type": "analytic", + "kind": "HARMONY", + "hash": "631e562c66dc19c2e715d87a5aa5c22e" + }, + "7": { + "time": 50, + "comments": "", + "step": 4, + "accidental": 0, + "quality": "dominant-seventh", + "inversion": 3, + "applied_to": 0, + "level": 1, + "display_mode": "roman", + "custom_text": "", + "custom_text_font_type": "analytic", + "kind": "HARMONY", + "hash": "8e48290e575fc9f7b4639c44456ee5ac" + }, + "8": { + "time": 60, + "comments": "", + "step": 1, + "accidental": 0, + "quality": "dominant-seventh", + "inversion": 1, + "applied_to": 4, + "level": 1, + "display_mode": "roman", + "custom_text": "", + "custom_text_font_type": "analytic", + "kind": "HARMONY", + "hash": "04edd729caae353bf67a23f8171093aa" + }, + "9": { + "time": 70, + "comments": "", + "step": 1, + "accidental": 0, + "quality": "minor-seventh", + "inversion": 1, + "applied_to": 0, + "level": 1, + "display_mode": "roman", + "custom_text": "", + "custom_text_font_type": "analytic", + "kind": "HARMONY", + "hash": "305e60532e03f546c8ff94b209d21034" + }, + "10": { + "time": 80, + "comments": "", + "step": 4, + "accidental": 0, + "quality": "dominant-ninth", + "inversion": 3, + "applied_to": 0, + "level": 1, + "display_mode": "roman", + "custom_text": "", + "custom_text_font_type": "analytic", + "kind": "HARMONY", + "hash": "80a5eb6f96a6ee8e7df1f803e73978ad" + }, + "1": { + "time": 0.0, + "step": 0, + "accidental": 0, + "type": "major", + "comments": "", + "level": 2, + "kind": "MODE", + "hash": "93165266d353cdc0a4b8df96d5d0f91d" + } + }, + "components_hash": "1e052a4bd06b7a723f32d01cea45ea1f" + } + }, + "timelines_hash": "0f9df940ab670331652bf4d39914e000", + "app_name": "TiLiA", + "version": "0.6.4" +} diff --git a/tests/repro_harmony_roman_s.py b/tests/repro_harmony_roman_s.py new file mode 100644 index 000000000..f3b313e39 --- /dev/null +++ b/tests/repro_harmony_roman_s.py @@ -0,0 +1,61 @@ +"""Throwaway generator for the repro bundle of the rogue-"s" harmony fix. + +Harmony is not covered by the CLI's `timelines add` / `components`, so the +fixture is built through `commands.execute(...)` and saved with `file.save`, +as `CLAUDE.md` prescribes for kinds the CLI cannot populate. + +Run from the repo root: + + uv run --python 3.12 pytest tests/repro_harmony_roman_s.py -q + +Writes `repro/harmony-roman-s.tla`. Delete both this file and the `.tla` +before merge — the durable artifact is +`TestRomanNumeralDisplay::test_roman_label_has_no_blank_accidental_placeholder`. +""" + +from pathlib import Path + +from tests.mock import Serve +from tests.ui.timelines.harmony.test_harmony_timeline_ui import add_harmony, add_mode +from tilia.requests import Get +from tilia.ui import commands + +OUT = Path(__file__).resolve().parent.parent / "repro" / "harmony-roman-s.tla" + +# (time, step, quality, inversion, applied_to) -> what the label should read. +# Every entry but the last two is a one- or two-figure figured bass, which is +# where the blank-accidental placeholder leaks out of the "%" stack. +HARMONIES = [ + (0, 0, "major", 0, 0), # I — control, no figures + (10, 0, "major", 1, 0), # I6 + (20, 0, "major", 2, 0), # I64 + (30, 4, "dominant-seventh", 1, 0), # V65 + (40, 4, "dominant-seventh", 2, 0), # V43 + (50, 4, "dominant-seventh", 3, 0), # V42 + (60, 1, "dominant-seventh", 1, 4), # V65/V — applied chord + (70, 1, "minor-seventh", 1, 0), # ii65 + (80, 4, "dominant-ninth", 3, 0), # V%sss432 — control, stacked form +] + + +def test_generate_fixture(harmony_tlui, tilia_state): + tilia_state.set_duration(100, "no") + add_mode(0, step=0, accidental=0, type="major") # C major + + for time, step, quality, inversion, applied_to in HARMONIES: + add_harmony( + time, + step=step, + quality=quality, + inversion=inversion, + applied_to=applied_to, + display_mode="roman", + ) + + OUT.parent.mkdir(parents=True, exist_ok=True) + with Serve(Get.FROM_USER_SAVE_PATH_TILIA, (True, OUT)): + commands.execute("file.save") + + print("\nwrote", OUT) + for element in sorted(harmony_tlui.harmonies(), key=lambda e: e.get_data("time")): + print(f" t={element.get_data('time'):>5} label={element.label!r}") diff --git a/tests/ui/timelines/harmony/test_harmony_timeline_ui.py b/tests/ui/timelines/harmony/test_harmony_timeline_ui.py index fa820a8d4..28134bdee 100644 --- a/tests/ui/timelines/harmony/test_harmony_timeline_ui.py +++ b/tests/ui/timelines/harmony/test_harmony_timeline_ui.py @@ -131,6 +131,35 @@ def test_roman_label_does_not_start_with_accidental_when_root_is_diatonic_sharp_ assert harmony_tlui[0].label.startswith(expected_start) + @pytest.mark.parametrize( + "step,quality,inversion,expected", + [ + (0, "major", 1, "I6"), + (0, "major", 2, "I64"), + (1, "minor-seventh", 0, "ii7"), + (1, "minor-seventh", 1, "ii65"), + (4, "dominant-seventh", 1, "V65"), + (4, "dominant-seventh", 2, "V43"), + (4, "dominant-seventh", 3, "V42"), + ], + ) + def test_roman_label_has_no_blank_accidental_placeholder( + self, step, quality, inversion, expected, harmony_tlui + ): + # "s" marks an accidental slot left blank; MusAnalysis only consumes it + # inside the three-figure "%" stack, so it must not reach shorter + # figures, where it would be drawn as a literal letter. + add_harmony(step=step, quality=quality, inversion=inversion) + + assert harmony_tlui.harmonies()[0].label == expected + + def test_roman_label_keeps_blank_accidental_placeholder_in_stacked_figures( + self, harmony_tlui + ): + add_harmony(step=4, quality="dominant-ninth", inversion=3) + + assert harmony_tlui.harmonies()[0].label == "V%sss432" + def test_roman_label_for_ninth_chord_high_inversion(self, harmony_tlui): # inversion=4 places the 9th in the bass; label is dynamically computed add_harmony( diff --git a/tilia/ui/timelines/harmony/utils.py b/tilia/ui/timelines/harmony/utils.py index 223aacf8d..9caf81068 100644 --- a/tilia/ui/timelines/harmony/utils.py +++ b/tilia/ui/timelines/harmony/utils.py @@ -134,8 +134,13 @@ def _fmt_mod(mod: str | None, none_str: str = "") -> str: def _figs_to_str(figures: list[tuple[int, str | None]]) -> str: - prefix = "%" if len(figures) > 2 else "" - accs = "".join(_fmt_mod(mod, "s") for _, mod in figures) + # MusAnalysis only consumes the "s" placeholder (an accidental slot left + # blank) inside the three-figure stack introduced by "%". Outside it there + # is no blank slot, so an unaccidented figure must contribute nothing — + # otherwise the "s" is rendered as a literal letter next to the numeral. + stacked = len(figures) > 2 + prefix = "%" if stacked else "" + accs = "".join(_fmt_mod(mod, "s" if stacked else "") for _, mod in figures) nums = "".join(str(n) for n, _ in figures) return prefix + accs + nums