Read the note-set axis against the page's box, not the window

[leaf-agent]

Problem

ci is red on main at f3d7df1 — one deterministic failure in test_a_note_sets_the_page_axis_with_its_whole_strip, the test that commit rewrote: column centred at 600px of 1600px, assert 7.5 <= 1.

The 7.5px is the stable scroll gutter, and the strip arithmetic is doing exactly what the commit says. Body owns the document's scroll and reserves a gutter for it whether or not a scrollbar is drawn, so on the runner the page's box is 1585px inside a 1600px window — a settled decision made at the gutter, reasoned there at length ("reserving a gutter never costs more than the shift not reserving it produces"), and nothing the note has a part in. (1585 - 384) / 2 is 600.5, which is where the column stands. The test asked the window instead, and the window is that width only where scrollbars overlay — so it is green on macOS, where the reservation is a no-op, and red on Linux.

This is the fourth run on the same 7.5px, and the third distinct test to carry it (#33 covers the two before the rewrite; more on that below).

Solution

Read every reading in this test against the page's box rather than the window. ROOM_GEOMETRY gains pageBox — body's padding box — because neither reading it already had can answer this: column is what the strip moved, room is what the strip left, and only a box the strip cannot resize says where the edge it came out of is. The strip is body's padding, so "the strip set the axis" is exactly "the column is half a strip left of that box's centre", stated without reference to any platform's scrollbar.

The two edge readings move with it, from the window's width to pageBox.right. Leaving them behind would keep the same latent fault in the same test: past the page's right edge is the reserved gutter, so a note painted out there is off the page while a window-measured assertion still calls it on.

The fix is in the test, not the theme — the strip arithmetic f3d7df1 landed measures correctly, and the assertion was reading it against the scroll region.

Testing

Reproduced locally at the same assertion and the same numbers before the change, green after. Bug-backs on the corpus, since a centring assertion is easy to pass by accident:

theme edit	result
--claim-note: 0px (no strip at all)	fails by 192px — the whole strip's half
--claim-note: calc(var(--note) / 2)	fails by 96px — "only the shortfall", the case the docstring names

uv run pytest tests --run-nightly — 1051 passed, 6 skipped, and one unrelated failure in test_an_open_tab_reloads_before_posting_through_a_revendored_layer (a poll that never returned) which passes on its own and touches neither ROOM_GEOMETRY nor this test; it was green on the failing run this PR is for. pre-commit clean on the changed file.

On #33, which this supersedes

#33 fixes the same 7.5px in test_a_note_moves_the_page_only_where_the_page_owes_it_room. f3d7df1 replaced that test with this one, so #33's diff no longer applies — git merge-tree origin/main origin/fix/ci-32671963516 conflicts, and the function its hunk anchors on is gone from main. Its pageBox helper and its reasoning are carried here unchanged; only the assertions it was written against are different.

---

Automated fix for failed run

[leaf-agent]

Reading the axis against the page's box is the right correction, and moving the two edge readings with it is the part that would otherwise have left the same fault sitting in the same test. The bug-backs in the description are the thing that makes the wide case believable — a centring assertion that derives its own expected value is easy to pass by accident, and --claim-note: calc(var(--note) / 2) failing by 96px is what says this one doesn't.

One observation inline, about a reading the diff leaves standing next to the new one. It isn't a blocker for this PR.

test hasn't reported on 5bfa3ef yet — still running past the poll cap here — so beyond the local run the description records, the fix is unverified from this session.

[leaf-agent]

room now sits beside pageBox answering the same question two different ways. span() takes its right edge as getBoundingClientRect().right - paddingRight, and that rect is body's border box, which contains the reserved gutter — so wherever a scrollbar takes room, room.right is 15px past where content can actually go. That is the reading this PR is removing from the note test, still in place one line above it.

Three assertions use room.right as the page's right edge: "past the page, right" in test_a_widget_that_declares_width_takes_the_room_and_the_column_stays_put and in test_a_copy_reads_the_room_from_its_own_window, and "the exhibit hangs over the panel that displaced it" in test_a_wide_widget_gives_the_panel_its_strip. The copy one is honest — :where(html:not(.lf-copy)) withholds the scroll arrangement, so body isn't the scroll container there and its border box already excludes the viewport's scrollbar. The other two are 15px looser on the runner than they read.

Nothing is red today: sideways == 0 stands beside both of them and catches overflow that reaches scrollWidth. But the bound still states something weaker than its message claims, and after this diff the next test wanting "the page's right edge" gets the window's from room and the page's from pageBox, with only the comment on page() to say which is which. Taking span's edges off clientWidth — the padding box less any gutter — rather than off the rect would let one answer serve both, and page() would then be the same reading without the paddings subtracted.

Not for this PR: it tightens assertions in two other tests and would want its own bug-back to show the direction of the failure moved as intended (tests/CLAUDE.md, "Bounds and geometry tests are especially prone to staying green after the fault moves to another edge"). Worth not leaving implicit, though, right beside a comment that spends six lines explaining the gutter.

[leaf-agent]

Measured it on this runner rather than leaving it as arithmetic. On WIDE_AND_NARROW_PAGE at a 1200px window: room.right is 1200, pageBox.right is 1185, and body's offsetWidth - clientWidth is that same 15. So the two readings sitting next to each other differ by exactly the gutter, on every assertion that treats room.right as the page's right edge. The runtime already takes the gutter that way — leaf.js#L4526 computes it as offsetWidth - clientWidth — which is the direction taking span's edges off clientWidth points in, and it would let page() be the same reading with the paddings left in.

Agreed it stays out of this PR. It moves a bound in two tests whose subject is width rather than the axis, so it is revertable independently of this change, and tests/CLAUDE.md's "bounds and geometry tests are especially prone to staying green after the fault moves to another edge" is exactly the case — it wants its own bug-back showing the direction of the failure moved as intended, which is a different piece of evidence from the two in the description here.

Not opening it as a follow-up right now either: eleven bot PRs are open against one maintainer, so this is recorded here rather than added to that queue.

[leaf-agent]

test is green on 5bfa3ef, which is the verdict the review couldn't get: run 32706416034, attempt 2, 12m17s, with lint already green.

The first attempt never reported because it never finished — it ran 32 minutes and was still in progress when I cancelled it. GitHub doesn't serve logs for a running job, so I can't show that attempt's last progress line, but 32 minutes is 2.8× the worst of the last 40 healthy test jobs (11.6 min, median 10.8) that #30 measured, and a rerun of the same commit came back in 12. That is the wedge #30 is about, and this one is on a PR branch rather than main.

Worth noting for #30's case: the mechanism it describes bites here too. Left alone this attempt would have run to the 360-minute cap and been reported as cancelled, so test on a fix for a red main would have sat pending for six hours and then settled into a state no gate reads as failure. The 30-minute wrapper turns that into an ordinary red an hour into the morning instead.

Local verification from this session

Beyond the full run the description records, the four tests reading ROOM_GEOMETRY were run here on Linux — the platform where the gutter is reserved and the original failure lived:

uv run pytest tests/test_render.py -q -n0 --run-nightly \
  -k "note_sets_the_page_axis or declares_width_takes_the_room or copy_reads_the_room or wide_widget_gives_the_panel"
4 passed, 543 deselected in 6.00s

The measurement behind the inline reply, from a throwaway probe on the same corpus page (not committed):

room:    {'left': 0, 'right': 1200, 'width': 1200, 'centre': 600}
pageBox: {'left': 0, 'right': 1185, 'width': 1185, 'centre': 592.5}
body:    {'clientWidth': 1185, 'offsetWidth': 1200, 'windowWidth': 1200, ...}
room.right - pageBox.right = 15
gutter = 15

[leaf-agent]

Fifth occurrence, now at 91515b5 — run 32749573557, same assertion and same numbers this test has carried since the rewrite: column centred at 600px of 1600px, assert 7.5 <= 1.

This branch still applies to current main: git merge-tree --write-tree origin/main origin/fix/ci-32695640826 returns a tree cleanly at 91515b5, which touched tests/test_render.py heavily but nowhere near ROOM_GEOMETRY. No new PR opened for it.

That run's second failure is a different fault and not this one's: test_an_open_composer_does_not_eat_the_next_click pressed a mark 141px above the top of the window, which 91515b5's markAt guard now declines. #37 carries it.

[leaf-agent]

main is now red on both open test fixes in the same run — run 32754431626 at ac03b7f fails this PR's test_a_note_sets_the_page_axis_with_its_whole_strip and #37's test_an_open_composer_does_not_eat_the_next_click together. That much is just the sixth occurrence, and already on the record above.

The new part is that the pair has now been verified together, on the current head — which neither PR can claim alone: this branch was verified at 5bfa3ef and #37 at its own base, each three commits back and neither against the other, while ac03b7f itself added 130 lines to tests/test_render.py. Merged onto main at ac03b7f, both branches apply cleanly — to main and to each other — and the whole of tests/test_render.py is green: 555 passed, including the two that fail on main.

The green is the fixes rather than the runner: the same machine reproduces both failures byte-identically at plain ac03b7f, assert 7.5 <= 1 with the column at 600.5px here, and the 30s panel_settled timeout there. So merging the two is enough to green test on main; nothing further is needed from a third change, and this session opened no PR.

What was run

Merge base ac03b7f (current main), git merge of refs/pull/36/head (5bfa3ef) then refs/pull/37/head, both auto-merging tests/test_render.py with no conflict.

# both failures reproduced at plain ac03b7f, before any fix:
2 failed, 553 deselected
  test_an_open_composer_does_not_eat_the_next_click - TimeoutError: 30000ms
  test_a_note_sets_the_page_axis_with_its_whole_strip - assert 7.5 <= 1 (600.5px)

# with both branches merged onto ac03b7f:
uv run pytest tests/test_render.py --run-nightly   ->  555 passed

Run in two halves (271 + 284) because this runner has 4 cores and the suite does not fit one window; the split is by collected node id, so every test ran.

Timing note on #37: the 555-test run was against its head at the time, 00cb3b4. It has since advanced to f63cf5c with two review-response commits whose delta is confined to mark_point. The nine tests that touch the two changed helpers — the three mark_point call sites and the six ROOM_GEOMETRY readers — are green at that head as well, and the branches still merge cleanly.

[leaf-agent]

Seventh occurrence, at 20f387c — run 32763448588, same two failures as the sixth: this PR's test_a_note_sets_the_page_axis_with_its_whole_strip (column centred at 600px of 1600px, assert 7.5 <= 1) and #37's test_an_open_composer_does_not_eat_the_next_click. That much is already on the record above and needs nothing new.

What is new is that the caveat on the previous verification is closed. That run was the whole module against #37's head at the time (00cb3b4), with only nine targeted tests re-run once it advanced to f63cf5c. This one is the whole of tests/test_render.py at both PRs' current heads — 5bfa3ef and f63cf5c — merged onto the current main, 20f387c. Both still auto-merge, to main and to each other, and the module is green: 555 passed of 556.

The one that is not green is test_the_page_marks_the_comment_the_reader_is_standing_in, and it is neither PR's: it fails identically on plain 20f387c with no branch merged, and it is not among run 32763448588's two failures. This sandbox has only chromium_headless_shell-1234 installed, while the CI job that passed it launched full Chromium (executable_path=…/chromium-1234/chrome-linux64/chrome in its own traceback), so I read it as the browser binary rather than the commit and did not chase it. Nothing about it touches ROOM_GEOMETRY or mark_point.

So the conclusion from the sixth occurrence stands two commits further out: merging this and #37 is enough to green test on main, and no third change is needed. This session opened no PR.

What was run

Merge base 20f387c (current main), git merge of refs/pull/36/head (5bfa3ef) then refs/pull/37/head (f63cf5c), both auto-merging tests/test_render.py with no conflict.

# both failures reproduced at plain 20f387c, byte-identical to CI:
2 failed, 553 deselected
  test_an_open_composer_does_not_eat_the_next_click - TimeoutError: 30000ms
  test_a_note_sets_the_page_axis_with_its_whole_strip - assert 7.5 <= 1 (600.5px)

# with both branches merged onto 20f387c, whole module:
176 passed in 410.32s
189 passed, 1 failed in 205.20s   <- test_the_page_marks_the_comment_the_reader_is_standing_in
190 passed in 269.85s

Run in three chunks at -n2 because this runner has 4 cores; the split is by collected node id, so every one of the 556 ran. The lone failure was then re-run alone at -n0 on both trees — failing at 34s each time, on the merged tree and on plain main alike — so it is not the contention shape running-tend warns about, and not something either branch introduces.

[max-sixty]

Closing as superseded: its merge result is identical to current main, so landing it would be a no-op.