Skip to content

Start the beat units again at each restart - #753

Open
azfoo wants to merge 2 commits into
v1-prereleasefrom
v1/323-timemap-restart-units
Open

azfoo wants to merge 2 commits into
v1-prereleasefrom
v1/323-timemap-restart-units

Conversation

@azfoo

@azfoo azfoo commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

🤖 Claude

A restart marks the first measure of a new movement or song in the same file. It now starts the beat units again, as the timeline's first measure does, so a later movement whose first measures have no unit reads its own meter, not the previous movement's.

  • meters_of: every movement starts with a unit, at the first row and at each row with restart. Where a movement's first measure has none, meters_of supplies one, assumed and with no beat_unit_id, since no beat carries it:
    • a copy of the movement's first unit, if it has one;
    • else of the unit in force before the movement;
    • else the default.
  • A movement's first measure always has starts_here true.
  • No unit reaches back across a restart: a first movement with no unit stays N/4, assumed, even when a later movement has a unit.
  • Unchanged: a restart on the first measure changes nothing, and a timeline without restarts reads as before. units.py doesn't change.
  • How: meters_of splits the rows into movements once, and runs the rule for the start of a timeline on each, with the unit in force before as the fallback. It stays linear in the number of rows: 20,000 one-measure movements take 0.06 s.

Before, a pickup without a unit in movement 2 took the unit from the end of movement 1, not assumed. In the test's example (3/4, then 6/8 tapped in two with a one-tap pickup), the pickup read 1/4 instead of 3/8, and every quarter stamp after it was half a quarter note early.

Tests

  • test_timemap_meters.py, TestRestarts. Its first five tests were written first, and three of them failed before the change:

    • a pickup without a unit takes its movement's first unit, assumed, and movement 2's bar 1 starts at quarter 7.5;
    • a movement without a unit carries the one before, assumed;
    • no unit reaches back across a restart;
    • a restart on the first measure changes nothing;
    • a movement that starts with its own unit.

    After the code review, three more tests: a third movement that copies a supplied unit, a movement that carries the unit set in the one before it, and the measure table's rows for a restart (movement, time signature, assumed).

  • Restart cases stay out of shared_examples.json: the app's format has no place for them.

  • uv run --package tilia-core pytest packages/tilia-core -q: 874 passed, 21 skipped, 7 xfailed, on v1-prerelease as of 2026-10-19. python scripts/check_packages.py passes. black and ruff are clean.

Guarantees. I neutralised the code behind each one, ran the suite, then restored it:

Guarantee Code neutralised Tests failing
a restart starts a movement _movements never splits 6
the supplied unit is assumed the copy keeps its flag 8
a movement without a unit carries the one before no fallback to it 3
no unit reaches back across a restart the first unit taken from the whole timeline 4
a movement's first measure starts a unit starts_here only on the timeline's first row 3
the supplied unit has no id the copy keeps its id 5

Part of TimeLineAnnotator/project-management#323

🤖 Generated with Claude Code

@azfoo

azfoo commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 Claude

Code review of fad3da68: no correctness bug found. The review traced meters_of through these cases:

  • a restart on the first row, and none on it;
  • consecutive restarts;
  • a restart row with its own unit;
  • a movement with no unit;
  • a pickup before a movement's first unit;
  • empty input.

Each gives what the rule asks, and a timeline without restarts reads as before. What it raised, and what 8d10fa52 does about it:

  • Changed:
    • Quadratic in the number of movements. Each movement start without a unit copied the rest of the table, so 20,000 movements took 0.46 s. meters_of now splits the rows into movements once, and takes 0.06 s.
    • The first unit of a movement was parsed twice, and one condition could never be true. Both went with the restructure.
    • Tests:
      • a third movement copying a supplied unit;
      • a movement carrying the unit set in the one before it;
      • build_rows's rows for a restart, so that the table, not only meters_of, is covered;
      • the restart on the first measure is now set on the row only, so the test shows which source it reads.
  • Not changed here:
    • The app and restarts. feat: beat patterns, beat units, time signatures and quarter stamps #621's loader copies the timeline's first unit onto beat 1 whatever restarts lie between, so once the app reads files with restarts, it and the core would disagree. The app can't read them yet. This is noted for the work that switches the app to the core's rules, TimeLineAnnotator/project-management#10.
    • MeasureMeter's docstring lists where beat_unit_id is None without movement starts. units.py is feat: beat patterns, beat units, time signatures and quarter stamps #621's file, copied unchanged, so the wording changes there first. It's still right in general: None where no unit set on a beat governs.
    • Movement starts are worked out in three places: table.py, meters_of and _movements. Working them out once, in rows_of, would touch more than this task needs.
    • A restart on a row whose downbeat has no time is lost by rows_of. build_rows refuses such a timeline before meters_of runs.

802 tests pass, and check_packages.py passes. The guarantees table in the description is updated: removing each guarantee's code now fails 3 to 8 tests.

azfoo and others added 2 commits October 19, 2026 17:34
A restart, the first measure of a new movement or song in the same file,
now starts the beat units as the timeline's first measure does. Where a
movement's first measure has no unit, meters_of supplies one, assumed and
with no beat_unit_id: a copy of the movement's first unit, else of the unit
in force before the movement, else the default. A movement's first measure
always starts a unit, and no unit reaches back across a restart, so a first
movement without a unit stays N/4 though a later one has a unit. A restart
on the first measure changes nothing, and a timeline without restarts reads
as before.

Before, a later movement whose pickup had no unit took the previous
movement's unit, not assumed, and every quarter stamp after it was off.

Part of TimeLineAnnotator/project-management#323

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
meters_of now splits the rows into movements once and runs the rule for
the start of a timeline on each, with the unit in force before as the
fallback. Before, each movement start without a unit scanned and copied
the rest of the table, which made meters_of quadratic in the number of
movements (20000 one-measure movements: 0.46 s, now 0.06 s); it also
parsed a movement's first unit twice and kept a condition that could
never be true.

The tests add a third movement that copies a supplied unit, a movement
that carries the unit set in the one before it, and the measure table's
rows for a restart; the restart on the first measure is now set on the
row only.

Part of TimeLineAnnotator/project-management#323

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@azfoo
azfoo force-pushed the v1/323-timemap-restart-units branch from 8d10fa5 to a22d15d Compare October 19, 2026 15:35

This branch has not been deployed

No deployments
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