feat(core): choose a document's time map by role

[azfoo]

🤖 Claude

Choosing a document's time map by role, in tilia_core.timemap.

What changed

• find_time_map(document, roles=None) -> TimeMapFound (timemap/__init__.py):
  • The roles are the caller's when given (a mapping from timeline id to a role or a list of roles), else each timeline's metadata["role"]. A given mapping replaces the file's roles: a timeline it leaves out has no role. A role that is a list claims time_map when it holds it.
  • Among the beat timelines whose role is time_map, the first by ordinal wins. When several claim the role, a warning names them in ordinal order: "timelines A, B and C claim the role time_map; A, the first by ordinal, is used".
  • With no claimant, the first beat timeline by ordinal is taken, guessed=True. Timelines of other kinds never count, even with the role.
  • An ordinal that isn't a whole number (the reader doesn't refuse one yet) sorts after the others, so nothing raises. A whole float (2.0) sorts by its value.
  • When the chosen timeline gives no time map, no other timeline stands in: time_map is None, with timeline_id and the reason from build_time_map.
  • A document without a beat timeline gives TimeMapFound(None, None, False, None, ()), with no reason, since nothing is wrong with it.
  • Every warning is returned in warnings and logged once for each call. TimeMapFound is frozen, and its warnings are a tuple, so it hashes.
• The media warning ("the last beat is at or after the end of the media"):
  • It moves from table._closing_barline, which logged it each time a table was built, to find_time_map.
  • find_time_map checks every beat timeline, whether or not it is the time map, folded tables included. Each warning names its timeline: Timeline <id>: the last beat (20.0) is at or after the end of the media (19.5); the last measure ends one gap after it.
  • A table with a single beat leaves out the second clause, since its only measure ends on the beat. The closing barline and the warning take the last beat and its gap from one helper, _last_gap.
  • A table without rows has no closing barline, so it gets no warning.
  • Building rows, time_map_for and measure_rows log nothing about the media.
• The media length counts only when the format allows it: a number above zero, whole or not, in a file timed in seconds. The reader doesn't check it, so anything else (text, a bool, NaN, infinity, a whole number too large for a float, zero or less) caps nothing and warns about nothing. Before this, text made find_time_map raise TypeError, and a 400-digit length OverflowError.
• A beat time too large for a float is no time: _has_time and _media_length share one check, _is_number, which catches the OverflowError math.isfinite raises on such an int. Its timeline alone has no rows and no time map ("a beat without a time (measure N)").
• No rows for a table out of time order. build_rows gives no rows while two beats are at the same time or out of time order, as it already did for a beat without a time, so no measure ends before it starts.
  • table.table_problem(timeline) gives the reason for all of these, and build_rows, media_warning and build_time_map share it. build_time_map's reasons are unchanged. media_warning gets the reason and the times in one pass.
  • EPS lives in table.py, and timemap.py imports it.
• time_map_of(document, roles=None) returns find_time_map(...).time_map.
• measure_rows(document, timeline_id) gives a beat timeline's computed table through build_rows. That includes folded tables, which have no time map. It gives no rows when table_problem finds one, and none for an unknown id or a timeline that isn't a beat timeline. _beat_timeline holds the lookup it shares with time_map_for.
• __all__ lists the names of the tilia_core.timemap API that exist so far: MeasureRow, Positions, TimeMap, TimeMapFound, find_time_map, measure_rows, time_map_for, time_map_of. meters_of stays in timemap.meters for now, because it takes rows_of's rows, which are private.
• Tests:
  • New: tests/timemap/test_timemap_choose.py.
  • test_media_ending_at_or_before_the_last_beat_leaves_the_gap now checks that building rows logs nothing.
  • test_timemap_folded_table_is_refused_before_its_rows_are_built used the media log to tell whether rows were built, and that log is gone. It now replaces build_rows_from with a function that fails if called.
  • test_timemap_prototype.py::test_choosing_the_time_map, from Port the prototype's time-map self-test to tilia_core.timemap #734 (port the prototype's time-map self-test), loses its strict xfail now that find_time_map exists.

Tests run

• Rebased onto v1-prerelease with feat(core): time signatures in the measure table, and the shared meter examples #736 (time signatures in the measure table) and feat(core): move the beat pattern parser into tilia_core.timemap #739 (the beat pattern parser). Each of the 27 commits passes the package's tests on its own.
• uv sync --all-packages, then uv run --package tilia-core pytest packages/tilia-core -q: 853 passed, 21 skipped, 7 xfailed (Python 3.11). The 7 xfails are Port the prototype's time-map self-test to tilia_core.timemap #734's distance tests, which wait for position, length and unit_seconds.
• tests/timemap on Python 3.10, against the working tree (--with-editable): 375 passed, 18 skipped, 7 xfailed.
• python scripts/check_packages.py: all package checks passed.
• black 22.12.0 and ruff 0.15.0 (the pre-commit versions): clean.

Guarantees: the code removed, and what failed (tests/timemap: 375 passed, 18 skipped, 7 xfailed)

Guarantee	Code removed or neutralised	Failed
No other timeline stands in for one without a time map	a loop that falls back to the next beat timeline	7
Several claimants give a warning	the warning left out	3
The claimants' warning names them	the old text, which names none	3
The claimants' warning reads as decided	a comma in place of the final " and "	3
The claimants' warning is logged	only the media warnings logged	2
The role chooses	always the first beat timeline	6
First by ordinal	sort key constant (the document's order)	12
An ordinal that isn't a whole number sorts after the others, and nothing raises	sorting by the raw ordinal	6
Only beat timelines count	the kind filter removed	4
The caller's roles replace the file's	the file's role as a fallback for ids the mapping leaves out	1
A role list that holds time_map claims it	the list branch removed	2
The media warning is returned	not appended to warnings	8
The warnings are logged by find_time_map	the log call removed	7
Every beat timeline past the media is warned about, folded ones too	only the time map's timeline, when it builds	4
A table without rows gets no media warning (a beat without a time, or out of time order)	the last beat read from every beat with a time	4
One beat: no gap in the warning	the gap's clause always added	1
A whole-number media length counts	int lengths refused	1
A number too large for a float is no number, and nothing raises	_is_number without its OverflowError guard	2
A media length the format doesn't allow is no length, and nothing raises	_media_length passing the length on unchecked, as before	6
Building a table doesn't log it	a log put back in _closing_barline	6
No rows while beats are out of time order or at the same time	build_rows checking only missing_time and broken_row	2
measure_rows gives no rows while a beat has no usable time	build_rows_from(rows_of(...)) without the checks	4
measure_rows and time_map_for give nothing for a timeline that isn't a beat timeline	the kind check in _beat_timeline removed	1
A folded table is refused before its rows are built	rows built before the folded check	1
TimeMapFound hashes	warnings passed as a list	30

After each mutation the code was restored, and the tests passed again (375 passed, 18 skipped, 7 xfailed). No claimed guarantee is untested.

Part of TimeLineAnnotator/project-management#216

🤖 Generated with Claude Code

[azfoo]

🤖 Claude

This review comes from two code-review runs plus a read-through.

• The first run: its 13 findings are fixed or decided, in ff310948 and d25e3c2b. Anne decided four of them on 2026-10-14; they're recorded on TimeLineAnnotator/project-management#13 (the measure table).
• The second run: it reviewed d25e3c2b, and its findings are below. I reproduced 1 and 2.

Please fix them before it merges.

Bugs, and two decided changes

1. A media length that isn't a number raises (table.py:239, _media_length; table.py:244, _ends_by).

   • The reader keeps media.length as it finds it, and _media_length passes it on unchecked.
   • With "length": "20", find_time_map raises TypeError: '<=' not supported between instances of 'str' and 'float'.
   • With true, it warns that the last beat is after the end of the media at 1 s.
   • Since this PR, every beat timeline makes that comparison, so one bad length breaks the whole file on the index's read path.

   Have _media_length return None unless the length is what the format allows.

   • The schema allows a number above zero, or null when the length is unknown.
   • The migration already turns an old file's 0, which TiLiA writes when no media is loaded, into null.
   • So treat text, a bool, NaN, infinity, 0 and negative lengths alike, as no length: no cap and no warning. That is the same check _has_time makes for beat times.
   • It also stops a NaN length from depending on min(end, media_length)'s argument order (table.py:224): min(nan, end) would give a NaN closing barline.

   Add a test for each.

2. One beat past the media: the warning describes a gap that isn't there (table.py:113, media_warning).

   • A table with one beat at 5.0 and a media length of 4.0 gets "the last beat (5.0) is at or after the end of the media (4.0); the last measure ends one gap after it".
   • Its only measure in fact ends on the beat: measure_rows gives (5.0, 5.0).

   Leave out the second clause when the table has a single beat.

3. The warning about several claimants isn't logged (__init__.py:27). It is returned but not logged, unlike the media warnings, so a caller of time_map_of loses it. Log it too, once for each call (Anne, 2026-10-14).

4. That warning doesn't name the claimants. It says "two" when three claim, and names none of them, while each media warning names its timeline. Name them, in ordinal order, e.g. "timelines A, B and C claim the role time_map; A, the first by ordinal, is used" (Anne, 2026-10-14).

Smaller

5. The type of roles (__init__.py:42, 99, 108): it is Mapping[str, str | None], but a role that is a list claims time_map too. Make the value type str | list[str] | None, tla.Metadata's, and update the contract.
6. One chain of checks (table.py:93, _usable_times):
   • It repeats build_time_map's chain (missing_time, broken_row, disorder) without the reasons, and find_time_map checks the chosen table twice.
   • A table_problem(timeline) -> str | None that returns the reason would serve build_rows, media_warning and build_time_map, and keep them in step.
   • "Fewer than two beats" and a disorder can't both apply, so build_time_map's reasons don't change.
7. One constant (table.py:14): SAME_TIME duplicates timemap.EPS, which build_time_map used for the same test before this PR. Keep one.
8. Docstrings:
   • measure_rows (__init__.py:123) leaves out the disorder case;
   • build_rows (table.py:80) says "None" but returns [];
   • _usable_times says None when there can be no rows, but returns [] for a missing or empty table.
9. Tests:
   • assert hash(found) == hash(found) (test_timemap_choose.py:218) can't fail once hash returns: calling hash(found) is the test.
   • No test checks that a table out of time order gets no media warning.
10. Test comments:
    • (#7) (test_timemap_choose.py:136) links to desktop's own #7. Write TimeLineAnnotator/project-management#7 (format in the core).
    • The comments that start "Anne, 2026-10-14" would read better citing TimeLineAnnotator/project-management#13, where the decisions are recorded.
11. Commits: d25e3c2b holds four decisions and ff310948 three unrelated fixes. desktop's CLAUDE.md asks for each in its own commit, so that one can be reverted or bisected alone. Splitting them rewrites the pushed branch.

Known, and tracked elsewhere

• Timelines with equal ordinals, and ordinals that aren't whole numbers, keep Document.timelines' order. The reader sorts that dict by id, so in a real file a tie goes by id, not by the file's order. That's on TimeLineAnnotator/project-management#7. test_choosing_the_time_map's "ties in file order" holds only for documents built in memory.
• time_map_of drops the warnings. The index should call find_time_map to report them: TimeLineAnnotator/project-management#157 (fill positions and measures from WP9's time map).

[azfoo]

🤖 Claude

Points 1–10 are fixed in six commits, one for each concern, with the tests written first. Each new test failed before its fix.

1. A media length the format doesn't allow is no length (22dde380). _media_length counts a number above zero, in a seconds file, and nothing else: text, a bool, NaN, infinity, zero and less cap nothing and warn about nothing. Tested with "20", True, NaN, infinity, 0, 0.0, -5.0 and a list.
2. One beat past the media (b3de3490). The warning stops after "…the end of the media (4.0)"; with more than one beat it still adds "the last measure ends one gap after it". past_media is gone: media_warning builds the text.
3. and 4. The claimants' warning is logged and names them (9077c3ba). "timelines A, B and C claim the role time_map; A, the first by ordinal, is used", in ordinal order, for any count. find_time_map logs every warning it returns, once for each call.
4. roles may hold lists (7a42f9e1): Mapping[str, str | list[str] | None] in find_time_map, time_map_of and _claims_the_role.
5. One chain of checks (16f0a551). table_problem(timeline) gives the reason a table has no rows, and build_rows, media_warning and build_time_map share it. _usable_times is gone.
6. One constant (16f0a551). EPS lives in table.py, and timemap.py imports it.
7. Docstrings (16f0a551): build_rows and measure_rows name the cases without rows, and _usable_times is gone.
8. Tests (16f0a551): hash(found) alone, and a new test that a table out of time order (or with two beats at the same time) gets no media warning.
9. Test comments (e700adbb): TimeLineAnnotator/project-management#7 (format in the core) in place of #7, and the decisions cited by TimeLineAnnotator/project-management#13 (the measure table), where they're recorded.
10. Splitting ff310948 and d25e3c2b: not done yet. It rewrites the pushed branch, which waits for a go-ahead. It can go with the rebase onto v1-prerelease, which has moved again (feat(core): time signatures in the measure table, and the shared meter examples #736 (time signatures in the measure table) and feat(core): move the beat pattern parser into tilia_core.timemap #739 (the beat pattern parser)). Git merges them without conflicts, and the merged code passes: 849 passed, 21 skipped, 7 xfailed.

Every guarantee still fails a test when its code is removed: 22 rows, in the description. Python 3.10 passes: 251 passed, 7 xfailed. check_packages.py passes.

[azfoo]

🤖 Claude

Point 11 is done, and the branch is rebased onto v1-prerelease, which now has #736 (time signatures in the measure table) and #739 (the beat pattern parser).

• ff310948 is now four commits:
  • f37a2a71 makes TimeMapFound.warnings a tuple;
  • 1452f3db gives the media check one test (_ends_by);
  • 8f357bc9 shares one beat-timeline lookup;
  • ba8ade45 renames the media warning test.
• d25e3c2b is now four commits:
  • b410c0e2: no rows for a table out of time order;
  • 9f6ea730: a warning for every beat timeline past the media;
  • f2e1ddf1: a role that is a list;
  • b7db78ea: ordinals that aren't whole numbers.
• The xfail removal is folded into the first commit, eca899e4. On the new base, find_time_map and the removal of Port the prototype's time-map self-test to tilia_core.timemap #734's strict xfail have to land together: otherwise that commit fails as a strict XPASS.
• The six fix commits from my last reply have new hashes.
  • 991d1986: points 6–9;
  • f63bb64c: point 1;
  • 742e5df8: point 2;
  • d8fd44df: points 3 and 4;
  • 7fbd6a76: point 5;
  • 75a1d4f0: point 10.

Checks:

• Each group of split commits ends on exactly the tree of the commit it replaces.
• The final tree is the merge of v1-prerelease and the old head, which passed before the rebase.
• All 15 commits pass the package's tests on their own. The tip gives 849 passed, 21 skipped, 7 xfailed.
• The 22 guarantees fail the same tests as before. Python 3.10 and check_packages.py pass.

[azfoo]

🤖 Claude

The two overflow bugs from the third review are fixed in two commits, each with tests that failed before its fix (an OverflowError). table._is_number is the shared check: a finite number, not a bool, and catching the OverflowError that math.isfinite raises on an int too large for a float, as rules._is_number does in #735 (one set of rules).

• 60f75e0: a media length too large for a float is no length. _media_length uses _is_number, so a length of 400 digits, which JSON reads as an int and the reader keeps, caps nothing and warns about nothing. media_warning also returns before it reads the length when a table has no beats.
  • Tests: 10**400 joins test_a_media_length_the_format_doesnt_allow_is_no_length; test_a_media_length_too_large_for_a_float_read_from_a_file_is_no_length writes the file with canonical_bytes, puts the 400 digits in, and reads it with tla.loads, as in the review.
• ec81f9d: a beat time too large for a float is no time (TimeLineAnnotator/project-management#319 (a beat time too large for a float)). _has_time uses _is_number, so the beat counts as "a beat without a time (measure 2)". In a timeline that isn't the time map, it no longer stops the document: that timeline alone has no rows and no time map.
  • Test: test_a_beat_time_too_large_for_a_float_is_no_time, with the beat in the second of two timelines. find_time_map keeps the first's time map and its media warning, measure_rows and time_map_for give nothing for the second, and the second alone gives the reason.

What fails when each fix is removed (uv run --package tilia-core pytest packages/tilia-core):

Code removed	Failed
_media_length calling math.isfinite itself, as before	1
_has_time calling math.isfinite itself, as before	1
the OverflowError catch in _is_number	2
the early return in media_warning alone	0
that early return and the _media_length fix together	2

The early return has no test of its own: once _media_length doesn't raise, nothing shows whether the length was read. The reader test catches it together with the _media_length fix, and it will catch the _media_length fix alone once the reader builds tables. The PR's guarantee table has these rows now.

Runs: tilia-core's tests: 852 passed, 21 skipped, 7 xfailed. tests/timemap on Python 3.10: 374 passed, 18 skipped, 7 xfailed. python scripts/check_packages.py: all package checks passed. black and ruff (pre-commit): clean. CI on ec81f9d: all 26 checks pass.

Left alone: TimeMap._snapped (timemap.py) calls math.isfinite on the times a caller asks about too, so positions raises OverflowError for a unit whose time is an int of 400 digits. It isn't on find_time_map's path, so it isn't fixed here.

[azfoo]

🤖 Claude

A third code-review run, on the six commits that answered the review of 2026-10-14, as they stood at 75a1d4f0. I reproduced 1. All of it was fixed before the merge; the reply below says where.

Bug

1. A whole-number media length too large for a float raises (table.py, _media_length).
   • The reader keeps media.length as JSON reads it, so "length": 1 followed by 400 zeros is the int 10**400.
   • The new math.isfinite(length) raises OverflowError on it, so find_time_map, time_map_for and measure_rows all raise. Before these commits they read the file without a length.
   • _has_time has the same hole for beat times (TimeLineAnnotator/project-management#319 (a beat time too large for a float)). It matters more now that find_time_map checks every beat timeline's table.

Tests

2. The claimants' tests compare the code with a copy of itself. _claim builds the expected text with the same join as _several_claim. If both dropped " and ", every test would still pass. Give the timelines fixed ids, and spell the decided text out: "timelines A, B and C claim the role time_map; A, the first by ordinal, is used".
3. No test uses a valid whole-number media length. Every valid length tested is a float, and the only ints tested are the refused 0 and True. If the check were narrowed to floats, a file whose JSON says "length": 20 would lose its cap and its warning, and nothing would fail.
4. test_a_table_out_of_time_order_gets_no_media_warning looks its factories up by name with globals()[make](), only because _out_of_order is defined further down. A rename would break it only at run time. Move _out_of_order next to _same_time, and pass the functions.

Smaller

5. Commits: 991d1986 holds four points (the table_problem refactor, one EPS, docstrings, two test changes). desktop's CLAUDE.md asks for each in its own commit.
6. One number check: _media_length repeats _has_time's check (not a bool, an int or a float, finite). One _is_number helper, used by both, fixes point 1 once for both.
7. One pass in media_warning: it runs table_problem, which reads the table's times, then reads them again with _times.
8. One rule for the gap: media_warning decides whether there is a gap after the last beat with len(times) > 1, and _closing_barline with whether before is empty. They agree today, but nothing ties them together, so the warning could come to describe a gap the barline doesn't have.
9. table_problem's docstring says it gives why the table can have no rows, else None. But a timeline without a table, or with an empty one, has no rows and gets None.
10. Comments: three comment lines in test_timemap_choose.py are over 88 columns, one of them two sentences on one line.

[azfoo]

🤖 Claude

All ten points are fixed. The branch is now 27 commits on v1-prerelease, one concern each, and each passes the package's tests on its own.

1. Too large for a float: 5edaaf8d fixes the media length, and a0fe6572 the beat time (TimeLineAnnotator/project-management#319 (a beat time too large for a float)). Both use _is_number, which catches the OverflowError (point 6). A beat time like that is no time: its timeline alone has no rows and no time map.
2. The claimants' text (6ccc0692): the tests give the timelines fixed ids and spell out "timelines A, B and C claim the role time_map; A, the first by ordinal, is used", and the two-claimant and early/late cases. A comma in place of the final " and " now fails 3 tests.
3. A whole-number media length (7f7844a3): 7 caps a closing barline at 8.0 down to 7, and 6 warns, as "(6)". Refusing ints now fails a test.
4. The factories (f71abfa4): _out_of_order sits next to _same_time, and the test takes both directly.
5. 991d1986 is four commits:
   • 7e0b5bd2 adds table_problem and the docstrings that name it;
   • 5ac2616a keeps one EPS;
   • 46b9d081 is the hash(found) test;
   • bcdacdbe is the test that a table out of time order gets no media warning.
6. One number check: see 1.
7. One pass (b6519484): _checked gives the problem and the times together, and media_warning uses it. The chosen table is still checked once by build_time_map and once by media_warning. Sharing that would change build_time_map's return value, which other code uses, for a check that is linear and cheap.
8. One rule for the gap (523ad1d5): _last_gap gives the last beat and the gap before it, none for a single beat, to both _closing_barline and media_warning.
9. table_problem's docstring (b3e8d77c) says that None doesn't promise rows.
10. Comments (27ae7f73): wrapped at 88 columns. The two longer lines left in table.py come from code that is already in v1-prerelease.

Checks:

• With every guarantee's code removed in turn, the tests catch all 25. That includes three new ones: the decided text, whole-number lengths, and numbers too large for a float. The table is in the description.
• 853 passed, 21 skipped, 7 xfailed.
• Python 3.10 and check_packages.py pass.