Skip to content

fix: keep measures_to_force_display a valid set of in-range indices - #576

Open
FelipeDefensor wants to merge 2 commits into
devfrom
fix/measures-to-force-display-invariants
Open

fix: keep measures_to_force_display a valid set of in-range indices#576
FelipeDefensor wants to merge 2 commits into
devfrom
fix/measures-to-force-display-invariants

Conversation

@FelipeDefensor

Copy link
Copy Markdown
Collaborator

Summary

Two independent bugs in how BeatTimeline maintains measures_to_force_display. The list is treated everywhere as a set of in-range measure indices, but nothing enforced either property.

1. The same measure could be forced twice. force_display_measure_number appended unconditionally, so setting a measure number to a value it already had left a second entry. unforce_display_measure_number removes a single occurrence, so one "Reset measure number" was no longer enough to hide the label again — and the duplicates were written to the .tla file.

Easy to hit by accident, since the number is applied to every selected measure: re-confirming a number for one measure re-forces all of them.

2. Out-of-range indices survived pruning. reduce_measure_numbers popped from the tail of the list while the last index was out of range. But the list is appended to in the order the user edits measures and is never sorted, so an out-of-range index sitting before an in-range one was left behind. Those stale indices are saved to the file and resurface as spuriously displayed measure numbers once the timeline grows back.

Found while reviewing #480, but independent of it — opened off dev rather than folded into that branch.

Test plan

  • test_set_measure_number_twice_forces_display_once — backend, fails on dev with [1, 1] == [1]
  • test_reset_measure_number_unforces_display_after_repeated_sets — backend, fails on dev with [1, 1] == []
  • test_reduce_measure_numbers_drops_forced_display_out_of_range — backend, fails on dev with [4, 1] == [1]
  • test_reset_measure_number_hides_label_set_more_than_once — end-to-end through the commands; fails on dev with '2' == '', i.e. the label stays visible after Reset
  • Full suite green (1834 passed, 13 skipped, 2 xfailed, 2 xpassed)
  • Randomised fuzz over the beat commands (25 seeds x 200 steps of set/reset measure number, set beats in measure, add, delete, distribute) checking the two invariants: 138 violations before, 0 after

Manual check

  1. Beat timeline, several measures, "Measure number periodicity" set so a middle measure is normally hidden.
  2. Right-click a beat in that measure, "Set measure number", give it a number — the label appears.
  3. Repeat step 2 with the same number.
  4. "Reset measure number" — before this change the label stayed visible; it should now disappear.

🤖 Generated with Claude Code

force_display_measure_number appended unconditionally, so setting a
measure number to a value it already had left a second entry in
measures_to_force_display. unforce_display_measure_number removes a
single occurrence, so one "Reset measure number" was no longer enough to
hide the label again, and the duplicates were written to the .tla file.

Easy to hit by accident: the number is applied to every selected measure,
so re-confirming a number for one measure re-forces all of them.
reduce_measure_numbers popped from the tail of measures_to_force_display
while the last index was out of range. The list is appended to in the
order the user edits measures and is never sorted, so an out-of-range
index sitting before an in-range one survived the pruning.

Those stale indices are saved to the .tla file and resurface as
spuriously displayed measure numbers once the timeline grows back.
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