[azfoo]

🤖 Claude

Moves #621 (beat units and time signatures)'s rules into tilia-core, so that the core computes each measure's meter as the app does.

• tilia_core.timemap.units is feat: beat patterns, beat units, time signatures and quarter stamps #621's tilia/timelines/beat/units.py at a17bbd83, unchanged: the same git blob. It has DEFAULT_DENOMINATOR, DEFAULT_UNITS, MAX_DENOMINATOR, validate_denominator, UnitsParseResult, parse_units, format_units, fit_units, is_fit_ambiguous and MeasureMeter. The app's tilia/timelines/beat/units.py is not touched.
• tilia_core.timemap.meters.meters_of(rows) -> list[MeasureMeter] runs the loop of feat: beat patterns, beat units, time signatures and quarter stamps #621's BeatTimeline._compute_measure_meters over the RowIns from rows_of, reading the units set on each row's beats:
  • the first unit set in a measure governs it and is carried forward; any other unit in that measure flags it as conflicting;
  • beat_unit_id is the id of the beat that carries the governing unit, because a beat unit in a .tla file has no id of its own.
• Before the first unit, the measures take that unit's denominator and units, assumed. With no unit at all, a measure of N taps is N/4, assumed. Either way, measure 1 has starts_here true, and those measures have no beat_unit_id, since no unit sits on their beats in the file.
  • This is the one place where meters_of differs from _compute_measure_meters, which never meets such a timeline.
  • It follows feat: beat patterns, beat units, time signatures and quarter stamps #621's loader instead: _ensure_first_beat_unit keeps a unit in the first measure, copied from the first unit or the default.
  • feat: beat patterns, beat units, time signatures and quarter stamps #621's copy keeps the later unit's assumed flag. Until feat: beat patterns, beat units, time signatures and quarter stamps #621 marks its copy assumed, the core and TiLiA differ in assumed on these files, which TiLiA never writes. Their time signatures already agree.
• A unit feat: beat patterns, beat units, time signatures and quarter stamps #621 refuses to create (units parse_units rejects, or a denominator outside 1–128) is left out of RowIn.units by rows_of, with a warning that names the timeline and the beat. The unit before it carries on, as when TiLiA drops such a unit from a file it loads. meters_of parses each unit once per call.
• Not changed: MeasureMeter.beat_unit_id keeps feat: beat patterns, beat units, time signatures and quarter stamps #621's annotation, int | None, although the core puts beat ids (str) there. The file stays as feat: beat patterns, beat units, time signatures and quarter stamps #621 has it, so the annotation can change in both copies at once.

timemap/__init__.py doesn't export the new names yet. Nothing is stored or written, and tilia_core/tla/ is unchanged.

Tests

• test_timemap_units.py ports feat: beat patterns, beat units, time signatures and quarter stamps #621's TestParseUnits and TestFitUnits case for case, plus a test of validate_denominator.
• test_timemap_rows.py gains tests of the units rows_of leaves out, and of the shortened warning.
• test_timemap_meters.py ports feat: beat patterns, beat units, time signatures and quarter stamps #621's TestMeasureMeters, except the two tests about editing in the app (barlines moving, the cache). It adds tests of:
  • the measures before the first unit: no unit in the first measures, or only one that can't be read, and a unit later; a pattern read from its start there; no readable unit at all; a unit on a later beat of measure 1;
  • is_assumed following the governing unit;
  • the first unit of a conflicting measure being carried forward;
  • unreadable units, including one before a readable unit in the same measure, and the warning that names the beat;
  • 2/4 assumed, 6/8 and 5/8 tapped in two, 7/8 ambiguous, a 3/8 pickup, and (4/3)/4, with each beat's length in quarter notes.
• 16 of the added cases also ran against feat: beat patterns, beat units, time signatures and quarter stamps #621's app code at a17bbd83, with the same expectations, and agree. feat: beat patterns, beat units, time signatures and quarter stamps #621's own test_beat_units.py passes there (81 tests). The cases before the first unit agree on time signatures, but not yet on assumed; see above.
• uv sync --all-packages, then uv run --package tilia-core pytest packages/tilia-core -q: 310 passed.
• python scripts/check_packages.py: all package checks pass.
• black 22.12.0 and ruff 0.15.0, through pre-commit, are clean.

Guarantees. I removed or neutralised the code behind each one, ran the suite (310 tests), then restored the code:

Guarantee	Code neutralised	Tests failing
unreadable units are left out of RowIn.units	_rows._readable returns True	19
units that aren't text are left out	the isinstance check	2
units parse_units rejects are left out	the is_valid check	12
a denominator outside 1–128 is left out	the validate_denominator check	5
the warning shortens the unit	%.60r → %r	1
the first unit in a measure governs it	in_measure[0] → in_measure[-1]	2
a second unit makes the measure conflicting	is_conflicting=False	2
a unit carries forward to later measures	the governing unit is reset in every measure	10
beat_unit_id is the governing unit's beat	beat_unit_id=None	15
before the first unit, the measures take its values	the default's values instead	3
before the first unit, the measures are assumed	not assumed when no unit governs	4
measure 1 always has starts_here	or index == 0 removed	4

Untested: none of the above. units.py is #621's code, tested by the ported cases.

Part of TimeLineAnnotator/project-management#213

🤖 Generated with Claude Code

[azfoo]

🤖 Claude

This review comes from a code-review run plus a read-through, on 3472048. I reproduced the first bug and the #250 case; the rest come from reading the code.

Waits on TimeLineAnnotator/project-management#250 (the measures before the first beat unit)

This PR does #250's option 1: the measures before the first unit are N/4, assumed. #250 is still open, and it blocks #213. Once it's decided:

• with option 2, meters_of changes (meters.py:64);
• with either option, add a test for the case. No test has a first measure without a unit followed by a unit in a later measure: every test has a unit on beat 0, or none at all.

A related case is now on #250 too: a timeline with no readable unit anywhere (meters.py:75). TiLiA makes a default unit on the first beat, so its measure 1 has starts_here=True and a governing unit. meters_of gives measure 1 starts_here=False and beat_unit_id=None.

Bugs

1. MeasureMeter can't be hashed (units.py:95). It is frozen but has list fields, so hash(meter) raises TypeError: unhashable type: 'list', and the lists can still be changed in place. Once #214 (time signatures) puts the meter into the frozen MeasureRow, hashing a row fails. Use tuples, and make the same change in desktop#621's copy so that the two stay the same.
2. A beat unit in a skipped measure is lost (meters.py:54). When rows_of skips a measure because its downbeat has no time, a unit set in that measure goes too. Later measures keep the unit before it, and nothing says so. Take beats 0–5 in measures of 2, with {4, "1"} on beat 0 and {8, "3"} on beat 2, and beat 2 without a time: measure 3 comes out 2/4 instead of 6/8. Carry the skipped measure's unit on to the measures after it, as TiLiA would, and warn.
3. Whether a long unit can be read depends on the Python version (units.py:61). parse_units uses Fraction(item). On Python 3.10.7 and later, that refuses a string of more than 4300 digits; on 3.10.0–3.10.6, it doesn't. The same file then gets different meters on different Pythons, and the error message holds the whole item. Refuse items over a fixed length, in both copies, and shorten the item in the message.

Smaller

4. Units are checked in two places (meters.py:84). _rows._beat_unit refuses only units whose structure is broken. It keeps units whose units or denominator can't be read, and meters_of checks those again in _readable. Check once, in _rows. Then RowIn.units holds only readable units for everything that reads it, such as #214's table and FR-023a's rows, and the warnings have one format, with the timeline's id.
5. The warning for an unreadable unit repeats (meters.py:93). It is logged on every call to meters_of, and it names only the beat. Once the table calls meters_of for each time map, one bad unit is logged once per query. Log it once, where the rows are read (with 4), naming the timeline.
6. units.py is a copy of desktop#621's tilia/timelines/beat/units.py. TimeLineAnnotator/project-management#220 (shared meter examples in the app) will run the same examples against both copies, which catches a change to the rules on one side. Nothing needs changing here for that.

[azfoo]

🤖 Claude

Point by point, as of 3b6454f0:

TimeLineAnnotator/project-management#250 (the measures before the first beat unit). Nothing changes until it is decided. The test for that case comes with the decision.

1. MeasureMeter can't be hashed. Not changed here. units.py stays a byte-for-byte copy of feat: beat patterns, beat units, time signatures and quarter stamps #621 (beat units and time signatures)'s file. The fix goes into feat: beat patterns, beat units, time signatures and quarter stamps #621 first, and then the file is copied again, as TimeLineAnnotator/project-management#247 (fixes to feat: beat patterns, beat units, time signatures and quarter stamps #621's units.py) records. feat: beat patterns, beat units, time signatures and quarter stamps #621's tests compare beat_values with lists, so they change with it.
2. A beat unit in a skipped measure. Not changed: there is no TiLiA behaviour to follow.
   • The format requires a time on every beat, so this can't come from a file.
   • TiLiA doesn't keep the unit either. Loading a file whose beat has no time raises KeyError: 'time' in _validate_component_creation (feat: beat patterns, beat units, time signatures and quarter stamps #621 at a17bbd83, checked).
   • rows_of already warns that it skips the measure. The skip only guards documents built in memory.
3. Long units. Not changed in units.py, for the same reason as 1; it is recorded on Make RequestHandler.on_request return type consistent and remove request post-processing #247. The core's own warning now cuts the unit to 60 characters.
4. Units checked in two places. Done.
   • rows_of leaves out any unit feat: beat patterns, beat units, time signatures and quarter stamps #621 would refuse to create: units parse_units rejects, or a denominator outside 1–128.
   • RowIn.units holds only readable units, and meters_of no longer checks them.
   • New tests in test_timemap_rows.py.
5. The warning repeats. Done with 4. rows_of logs it once per read of the rows, naming the timeline and the beat; meters_of logs nothing.
6. No change.

uv run --package tilia-core pytest packages/tilia-core -q: 305 passed. python scripts/check_packages.py passes.

[azfoo]

🤖 Claude

TimeLineAnnotator/project-management#250 (the measures before the first beat unit) is decided, and 8649ac04 follows it:

• Before the first unit the core can read, the measures take that unit's denominator and units, assumed.
• With no readable unit at all, a measure of N taps is N/4, assumed.
• Either way, measure 1 has starts_here true, and those measures have no beat_unit_id.
• Tests, written first (TestBeforeTheFirstUnit, 7 cases):
  • no unit in the first measures, or only one that can't be read, and a unit later;
  • a pattern read from its start there;
  • no readable unit at all;
  • a unit on a later beat of measure 1, which stays its own.
• _compute_measure_meters is otherwise unchanged. The docstring says where meters_of differs from it, and why: it follows feat: beat patterns, beat units, time signatures and quarter stamps #621's loader (_ensure_first_beat_unit).
• Not yet the same as TiLiA: assumed on these files. The rest of the meter already agrees.

The review's other points stand as in my reply above:

• 1 and 3 go through feat: beat patterns, beat units, time signatures and quarter stamps #621, and then a new copy of units.py (TimeLineAnnotator/project-management#247, fixes to feat: beat patterns, beat units, time signatures and quarter stamps #621's units.py);
• 2 doesn't change;
• 4 and 5 are done in 3b6454f0;
• 6 doesn't change.

v1-prerelease hasn't moved, so there was nothing to rebase.

uv run --package tilia-core pytest packages/tilia-core -q: 310 passed. python scripts/check_packages.py passes. The description's guarantees table now covers the three new guarantees (3, 4 and 4 tests fail when their code is neutralised).

[FelipeDefensor]

🤖 Claude

Felipe approves this PR. Before merge: two tests, and a few changes to the guarantees table.

Two reviews, run independently, found no wrong meter. meters_of gave the same meters as a hand port of #621's loop on 3,000 and 4,000 random timelines, and units.py is #621's file byte for byte. The suite passes on Python 3.10, 3.12 and 3.14: 310 tests, and 371 merged with #707. Anne's earlier points are done as described, or recorded on TimeLineAnnotator/project-management#247.

Before merge

1. No test checks that the measures before the first unit take the first unit's values.
   • Search the rows in reverse at meters.py:54, and all 310 tests still pass. Each test in TestBeforeTheFirstUnit has only one readable unit after the gap.
   • A test that would catch it: 8 beats in measures of 2, with {8, "3"} on beat 4 and {4, "1"} on beat 6. Measures 1 and 2 must be {8, "3"}, assumed. With the search reversed, they are {4, "1"}.
2. No test checks that a beat_unit that isn't a mapping is left out.
   • Catch only KeyError at _rows.py:107, and all 310 tests still pass. Then "beat_unit": "4/4" raises out of rows_of: TypeError: string indices must be integers, not 'str'.
   • "4/4", ["1"] and 5 in test_a_beat_unit_desktop_621_refuses_is_left_out would cover it.
3. The table leaves out claims that the tests do catch. Principle V wants each one with its count:
   • the warning names the timeline: 14 tests fail without it (_rows.py:112);
   • the warning names the beat: 14 (_rows.py:113);
   • with no readable unit, a measure of N taps is N/4: 2 for the denominator and 2 for the units (meters.py:56).
4. The table should name what no test catches. With the code removed, all 310 tests still pass:
   • "meters_of parses each unit once per call" (meters.py:92). It's also one off: the first unit is parsed twice, at line 54 and again in the loop, so three units take four calls. Reusing first would make it true.
   • Three in units.py: parse_units's message for blank text (line 45), its message for an empty item such as "1+" (line 51), and "/", "1/" or "1//2" refused rather than raised (line 62, the ValueError half). Their tests belong in #621, and come over with the next copy (TimeLineAnnotator/project-management#247).

MeasureMeter can't be hashed. Nothing to change here, since the file stays #621's. Felipe suggests that TimeLineAnnotator/project-management#247 lands before TimeLineAnnotator/project-management#214, and says why there.

Can follow later

• meters_of raises on a row that rows_of didn't make. A hand-built RowIn with units="x" gives ZeroDivisionError: integer modulo by zero. The function is public. Say in its docstring that rows come from rows_of, or skip such units.
• A long unit pattern makes meters_of slow. For every measure, it copies the pattern (meters.py:71) and is_fit_ambiguous builds a set of it (units.py:92). On 50,000 beats in measures of 4, it took 0.2 s with a pattern of 10 entries and 1.9 s with 100. Real patterns have 1 to 5 entries, but a damaged file with thousands would stall a corpus scan. Working both out once per governing unit would fix it. #621's loop has the same cost.

Reproduction

From the repository root at 8649ac0, with Python 3.12: PYTHONPATH=packages/tilia-core/src:packages/tilia-core/tests/timemap python repro.py

import logging
import time
from fractions import Fraction

from docs import beat_timeline
from tilia_core import tla
from tilia_core.timemap import meters as meters_module
from tilia_core.timemap._rows import RowIn, rows_of
from tilia_core.timemap.meters import meters_of


def piece(count, per_measure, units):
    """`count` beats a second apart, in measures of `per_measure`, with units on some beats."""
    return beat_timeline(
        [
            {
                "time": float(i),
                "measure": {} if i % per_measure == 0 else None,
                "beat_unit": units.get(i),
            }
            for i in range(count)
        ]
    )


# MeasureMeter can't be hashed, and its lists can be changed in place.
meter = meters_of(rows_of(piece(4, 4, {0: {"denominator": 4, "units": "1"}})))[0]
try:
    hash(meter)
except TypeError as error:
    print("hash(meter):", type(error).__name__, error)
meter.units.append(Fraction(9))
print("units changed in place on a frozen meter:", meter.units)

# Before merge, 1: two units after the gap; the measures before take the FIRST one's values.
timeline = piece(8, 2, {4: {"denominator": 8, "units": "3"}, 6: {"denominator": 4, "units": "1"}})
meters = meters_of(rows_of(timeline))
print(
    "1. measures 1-4 (denominator, units, assumed):",
    [(m.denominator, [str(u) for u in m.units], m.is_assumed) for m in meters],
)

# Before merge, 2: a beat_unit that isn't a mapping is left out, with a warning.
records = []
handler = logging.Handler()
handler.emit = records.append
logging.getLogger("tilia_core.timemap._rows").addHandler(handler)
for bad in ("4/4", ["1"], 5):
    records.clear()
    rows = rows_of(piece(4, 4, {0: {"denominator": 4, "units": "1"}, 2: bad}))
    print(f"2. beat_unit {bad!r}: units kept {len(rows[0].units)}, warnings {len(records)}")

# Before merge, 4: how often meters_of parses a unit, for three units.
calls = 0
real = meters_module.parse_units


def counting(text):
    global calls
    calls += 1
    return real(text)


meters_module.parse_units = counting
meters_of(rows_of(piece(12, 4, {i: {"denominator": 4, "units": "1"} for i in (0, 4, 8)})))
meters_module.parse_units = real
print("4. three units, parse_units calls in meters_of:", calls)

# Later: a hand-built row with a unit rows_of would have left out.
row = RowIn(
    id="a",
    number=1,
    label="1",
    beat_ids=("a", "b"),
    beat_times=(0.0, 1.0),
    units=(("a", tla.BeatUnit(denominator=4, units="x", assumed=False)),),
    cadenza=False,
    restart=False,
    force_display=False,
    next=None,
    source="tapped",
)
try:
    meters_of([row])
except Exception as error:
    print("later: meters_of on a hand-built row:", type(error).__name__, error)

# Later: a long unit pattern, one unit on beat 0, 50,000 beats in measures of 4.
for entries in (10, 100):
    timeline = piece(50_000, 4, {0: {"denominator": 8, "units": "+".join(["1"] * entries)}})
    rows = rows_of(timeline)
    start = time.perf_counter()
    meters_of(rows)
    print(f"later: {entries} entries: meters_of took {time.perf_counter() - start:.1f} s")

Output:

hash(meter): TypeError unhashable type: 'list'
units changed in place on a frozen meter: [Fraction(1, 1), Fraction(9, 1)]
1. measures 1-4 (denominator, units, assumed): [(8, ['3'], True), (8, ['3'], True), (8, ['3'], False), (4, ['1'], False)]
2. beat_unit '4/4': units kept 1, warnings 1
2. beat_unit ['1']: units kept 1, warnings 1
2. beat_unit 5: units kept 1, warnings 1
4. three units, parse_units calls in meters_of: 4
later: meters_of on a hand-built row: ZeroDivisionError integer modulo by zero
later: 10 entries: meters_of took 0.2 s
later: 100 entries: meters_of took 1.9 s

Every guarantee we checked: 40, of which 35 are caught and 5 untested

For each one, the code behind it was removed or neutralised, the core's 310 tests run on Python 3.12, and the code put back. All 12 of the table's counts match ours.

	Guarantee	Code	Tests failing	The PR's count
G01	unreadable units are left out of RowIn.units	_rows.py:120	19	19
G02	units that aren't text are left out	_rows.py:123	2	2
G03	units parse_units rejects are left out	_rows.py:124	12	12
G04	a denominator outside 1-128 is left out of RowIn.units	_rows.py:125	5	5
G05	the warning shortens the unit	_rows.py:111	1	1
G06	the first unit set in a measure governs it	meters.py:66	2	2
G07	a second unit makes the measure conflicting	meters.py:81	2	2
G08	a unit carries forward to later measures	meters.py:65	10	10
G09	beat_unit_id is the governing unit's beat	meters.py:78	15	15
G10	before the first unit, the measures take its denominator and units	meters.py:58	3	3
G11	before the first unit, the measures are assumed	meters.py:82	4	4
G12	measure 1 always has starts_here	meters.py:79	4	4
G13	format_units writes canonical '2+3' form (agrees with #621's cases)	units.py:73	1
G14	fit_units repeats the pattern over a longer measure	units.py:83	4
G15	is_fit_ambiguous flags a measure that doesn't hold a whole number of patterns	units.py:92	6
G16	validate_denominator: upper bound 128	units.py:29	3
G17	validate_denominator: lower bound 1 (zero and negatives refused)	units.py:29	3
G18	validate_denominator: a bool is not a denominator	units.py:28	1
G19	validate_denominator: a non-int is refused	units.py:27	2
G20	the warning names the timeline	_rows.py:112	14
G21	the warning names the beat	_rows.py:113	14
G22	a measure of N taps with no readable unit is N/4: denominator 4	meters.py:56	2
G23	a measure of N taps with no readable unit is N/4: units '1'	meters.py:56	2
G24	is_assumed follows the governing unit's assumed flag	meters.py:82	2
G25	each measure's beat values are in quarter notes (quarter_length)	units.py:121	6
G26	the numerator is the sum of the beat values	units.py:114	17
G27	the assumed flag on a unit in the file is read	_rows.py:105	3
G28	a beat_unit that isn't a mapping (TypeError) is ignored with a warning, not raised	_rows.py:107	0, untested
G29	a beat_unit missing its denominator/units (KeyError) is ignored with a warning, not raised	_rows.py:107	1
G30	parse_units: blank text gives the 'Enter at least one unit.' error	units.py:45	0, untested
G31	parse_units: an empty item ('1+') gives the 'Missing unit around +' error	units.py:51	0, untested
G32	parse_units: decimals, signs, exponents are refused	units.py:58	2
G33	parse_units: '1/0' is refused, not raised	units.py:62	1
G34	parse_units: '/', '1/', '//' (Fraction ValueError) are refused, not raised	units.py:62	0, untested
G35	parse_units: units must be greater than 0 ('0', '0/3')	units.py:64	3
G36	meters_of parses each unit once per call	meters.py:92	0, untested
G37	a measure whose downbeat has no time is skipped whole	_rows.py:68	1
G38	a beat without a time is left out of the row	_rows.py:55	2
G39	units are kept in beat order	_rows.py:67	2
G40	each measure has one beat value per tapped beat	meters.py:77	28