Skip to content

fix: make timeline components hash deterministic - #585

Open
FelipeDefensor wants to merge 1 commit into
devfrom
fix/nondeterministic-components-hash
Open

fix: make timeline components hash deterministic#585
FelipeDefensor wants to merge 1 commit into
devfrom
fix/nondeterministic-components-hash

Conversation

@FelipeDefensor

Copy link
Copy Markdown
Collaborator

Problem

TimelineComponentManager.hash_components concatenated component hashes in _components order, and restore_state recreated components by iterating a set of hash strings. Set iteration order over strings depends on PYTHONHASHSEED, so after an undo the components landed in _components in a process-dependent order and the digest changed even though the components were identical.

That order only matters because ORDERING_ATTRS is not a total order across component kinds. Staff orders by index and Clef by time, so a staff at index 0 and a clef at time 0 both have an ordinal of (0,) and neither sorts before the other. Clef ignores staff_index entirely, so the clefs of a multi-staff score all tie as well. bisect.insort_left leaves tied components in insertion order.

Worth flagging for reviewers: plain sorted(self._components) does not fix this. Timsort is stable, so tied components keep their input order and the digest still varies. A tiebreaker is required.

Impact: any undoable() test on a timeline with several components is flaky, and components_hash feeds the "is the file modified" comparison in are_tilia_data_equal.

Fix

  • hash_components iterates sorted(self._components, key=lambda c: (c.ordinal, c.hash)) — total and reproducible, and identical to the ordinal order whenever ordinals are distinct.
  • restore_state builds its delete/create lists by iterating the hash-keyed dicts rather than the sets, so the resulting component order is reproducible too.

Effect on hashes already saved in .tla files

  • Timelines whose components have distinct ordinals (every kind other than score) hash byte-identically — verified against marker, beat, hierarchy and pdf timelines.
  • Score timelines can hash differently — verified on a 27-component multistaff MusicXML score.
  • This is harmless: the stored components_hash is discarded on load, since App._do_open overwrites file.timelines with freshly computed state. Planting a pre-fix hash in a .tla and reopening it leaves is_file_modified at False.

The digest's string shape (h1|h2|...|hn|, empty string when there are no components) is unchanged, and is now pinned by a test so it cannot drift silently.

Tests

Test Pre-fix Post-fix
test_score_timeline.py::TestHashComponents (2 tests) fails on all seeds 0–5 passes on all
test_score_timeline_ui.py::TestClear::test_undo_redo fails on seeds 0, 1, 4 passes on all
test_timeline_component_manager.py::TestHashComponents (2 tests) passes (shape guard) passes

The two test_score_timeline.py tests are the real regression guards: they are deterministic, so a single-seed CI run cannot pass them by luck. test_undo_redo reproduces the user-visible symptom, which is inherently seed-dependent.

Full suite: 1834 passed. The two tests/test_app.py failures under pytest -n auto are pre-existing xdist flakiness — dev fails a different pair the same way, and all 90 pass serially.

`hash_components` concatenated component hashes in `_components` order, and
`restore_state` recreated components by iterating a set of hash strings. Set
iteration order over strings depends on PYTHONHASHSEED, so after an undo the
components landed in `_components` in a process-dependent order.

That order only matters because `ORDERING_ATTRS` is not a total order across
component kinds: a Staff at index 0 and a Clef at time 0 both have an ordinal
of `(0,)`, and Clef ignores `staff_index` entirely, so the clefs of a
multi-staff score all tie as well. `bisect.insort_left` leaves tied components
in insertion order, so the digest changed even though the components were
identical. Plain `sorted()` would not have fixed this — Timsort is stable, so
tied components keep their input order.

Hash over `sorted(self._components, key=lambda c: (c.ordinal, c.hash))`
instead, which is total and reproducible while still matching the ordinal
order whenever ordinals are distinct, and build `restore_state`'s
delete/create lists by iterating the hash-keyed dicts rather than the sets so
the resulting component order is reproducible too.

Consequences of the digest change: timelines whose components have distinct
ordinals (every kind other than score) hash exactly as before. Score timelines
can hash differently, but the `components_hash` stored in a .tla is discarded
on load — `App._do_open` overwrites `file.timelines` with freshly computed
state — so files already on disk are unaffected.

The string shape (`h1|h2|...|hn|`, empty string for no components) is
unchanged and now pinned by a test.
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