fix: beats added after a deletion lose measure structure - #597
fix: beats added after a deletion lose measure structure#597FelipeDefensor wants to merge 2 commits into
Conversation
`BeatTimeline.delete_components` disabled the component manager's `compute_is_first_in_measure` flag for the duration of the deletion, but re-enabled a misspelled attribute (`update_is_first_in_measure`). The real flag stayed `False` for the rest of the timeline's life, so every beat created afterwards skipped `recalculate_measures()` and kept the default `is_first_in_measure = False`: no measure numbers, no long dashes. Deleting *all* beats made it worse. The recompute was guarded by `if not self.is_empty:`, so an emptied timeline kept a stale `beats_in_measure` / `measure_numbers` describing beats that no longer existed. Fix the attribute name, and move `recalculate_measures()` and the measure-number-change post out of the emptiness guard so an emptied timeline drops its stale measure data and the UI clears its labels.
A fix under review is verified by generating a .tla into .repro/ and opening it on both the base ref and this branch. Nothing there is tracked, so keep it out of git status rather than leaving the reviewer to notice it themselves.
1a365b4 to
84c1bab
Compare
|
Superseded by the combined Renaming the misspelled restore to The release branch routes the suppression through a context manager that restores the value it found rather than a literal The repro bundle was removed as the commit message intended. Follow-up work for |
Fixes the beat timeline losing its measure structure for every beat added after a deletion.
The bug
BeatTimeline.delete_componentsdisables the component manager'scompute_is_first_in_measureflag for the duration of the deletion, then re-enables a misspelled attribute:The real flag stays
Falsefor the rest of the timeline's life. Every beat created afterwards takes the early-out inBeatTLComponentManager.create_component, so it skipsrecalculate_measures(), skipsbeat.is_first_in_measure = ..., and skips theBEAT_TIMELINE_MEASURE_NUMBER_CHANGE_DONEpost — leaving theBeat.__init__defaultis_first_in_measure = False. Result: short dashes everywhere, no measure numbers.Deleting all beats made it worse. The recompute was guarded by
if not self.is_empty:, so an emptied timeline kept a stalebeats_in_measure/measure_numbersdescribing beats that no longer existed.Not limited to deleting everything — any multi-beat deletion leaves the flag stuck, so beats added after a partial delete are equally broken.
The fix
Correct the attribute name, and move
recalculate_measures()and the measure-number-change post out of the emptiness guard so an emptied timeline drops its stale measure data and the UI clears its labels.Two regression tests in
tests/timelines/beat/test_beat_timeline.pycover delete-all and delete-some followed by reinsertion. Full suite green: 1349 passed, 9 skipped, 2 xfailed, 2 xpassed.Repro bundle
Bundle format per #580. Nothing repro-specific is committed — the fixture is generated into
.repro/, which this PR adds to.gitignore.One-time setup, and note that re-running a checkout after you have made local edits can throw them away:
Build the fixture, from this branch's checkout at the repo root:
That leaves you on a "Measures" beat timeline with beat pattern 4 and twelve beats — three full measures. It is media-free (
media_pathempty,media lengthset), so it opens with no prompt and no error.See the bug, on the base ref:
See the fix, back in this branch's checkout:
uv run --python 3.12 tilia "$PWD/.repro/beat-delete-reinsert.tla"Close with
git worktree remove ../tilia-base. Same fixture file both times; only the app code differs.On Windows, run these in PowerShell — Git Bash expands
$PWDto an MSYS path (/c/Users/...) that the interpreter cannot open.Acceptance criteria
What this bundle does not verify
The fix, the regression tests and the scenario all come from one author. The before/after pair shows that the scenario's behavior changed; it cannot show that the scenario is the bug as reported. The scenario and the criteria above are taken from the issue text rather than from the fix, but they exercise one path only: delete-all followed by clicking beats back in. Not covered — deleting a subset,
Fill with beatsas the reinsertion route, undo/redo across the deletion, and beat patterns other than 4. The regression tests cover delete-all and delete-some at the model level.