Make the comment-button clearance test able to fail again - #103
Conversation
test_the_comment_button_stands_on_no_control asks whether floating chrome covers a page control, reading the control's own centre point twice: once through elementFromPoint and once by pressing Accept. Both readings go quiet together when the bar stops reaching that centre, and neither says so. That happened. Carrying six reaction pills the bar reached 219px past sug-refill's Accept/Reject row; carrying the comment glyph and one ellipsis it misses the row by 2.9px. Between those two shapes the test passed on a page where nothing was ever in its way — confirmed by disabling placeClear's step-down, which leaves the hit test finding nothing covered at the default width and at every width in the 900-970px band where the row still hangs in the margin. Its sibling below states the same arrangement and so failed loudly instead, which is what put the staleness on the record. State it here too: assert the bar stepped past the row before asking what it stands on. The assertion passes on 38bef20, whose 6px sharing gutter restored the step, and fails on 891a2d4, where the bar cleared the row and the test proved nothing.
leaf-agent
left a comment
There was a problem hiding this comment.
Two things about the arrangement, both measured on this branch.
The stepping premise clears by 2.9px of a 6px allowance. At the default width the bar and the row don't overlap at all — bar right edge 896.6, row left edge 899.5. placeClear steps only because its sharing filter keeps a gutter (r.left < box.right + 6), so the new assertion holds with 3.1px of slack, and that slack is the width of a 💬 and an ellipsis. The comment sitting on that filter is explicit that this is not a thing to depend on: "A one-glyph difference between system fonts must not decide whether two controls almost touch or the float steps clear." A bar 3.1px narrower — an emoji fallback, a padding change, a different system font — and the test fails saying the bar never stepped past the row, so standing on no control proves nothing, which reads as staleness when the cause is font metrics. That is a fresh way for this file to go red in CI, on the test family the run that produced this PR already tripped over.
Narrowing the window fixes the premise even though it doesn't restore the coverage claim: opened at 930×900 the bar and the row overlap horizontally by 20.3px, so the step no longer turns on the gutter, and the test still passes there. Suggestion inline — it needs resized added to the render_support import block (~line 48), which isn't in this diff, the same import #101 adds for the sibling.
The assertions the test is named for still can't fail. With placeClear's step-down disabled (if (false && y !== box.top) …) and the new assertion deleted, the test is green: under comes back [], the Accept press lands, the composer stays hidden. So after this change the only assertion in the test carrying a causal contrast is the precondition, and what it proves is that placeClear steps for a 3px near-miss — not that floating chrome stands on no control. The new docstring paragraph is honest about that ("the walk stepping is the arrangement"), but the name and the first two paragraphs still promise a coverage test, and the next reader sees green. tests/CLAUDE.md's Make a green test non-vacuous names this shape: "A test is vacuous when some unrelated mechanism makes its assertion true under both the good and bad implementation." The tripwire stands in front of that rather than removing it — worth saying so where the docstring makes the coverage claim, or retargeting the test to what it can now hold.
On #101 being superseded. The CSS half landed: .lf-fab-bar:not(.lf-react-stacked) > .lf-react-palette { flex-wrap: nowrap; } is in 38bef20. The test half did not — the sibling on main carries no resized(...) call, and that is the half that would take both tests off the 3px margin.
How each measurement was taken
All on the merge commit for this PR (c0e4a6b on 38bef20), uv run pytest tests/test_render_anchors.py -q -n0.
Geometry, probed straight after the new assertion at the default 1200 viewport:
{'bar': [831.06, 196.5, 896.56, 218.05], 'row': [899.5, 168.95, 1075.5, 190.5], 'gap': 2.94}
Same probe with resized(page, 930, 900) inserted before the selection: {'barR': 759.3, 'rowL': 739.0, 'gap': -20.3} — a real overlap — and the test passes.
Vacuity: placeClear's last line edited to if (false && y !== box.top) place(node, left, y).
| tree | result |
|---|---|
| step disabled, new assertion present | fails on the new assertion (156.95 > 190.5), same as the sibling |
| step disabled, new assertion removed | passes — under == [], Accept lands, composer hidden |
Review of the precondition found it resting on placeClear's 6px sharing gutter: at a desk's width the compact bar clears sug-refill's row outright by 2.9px and steps only because that filter keeps a gutter, leaving 3.1px of slack. The filter was added saying a one-glyph difference between system fonts must not decide whether a float steps clear, so a precondition spending that allowance would go red for a font and blame staleness. At 1200 there is no collision left to assert. Open the page at 930 instead, where the bar and the row overlap by 20.3px and the step is the row's own doing; below 900 the row docks under its block and is out of the way again. This costs the tripwire its catch on the f30a585-shaped shrink, which at 930 still overlaps — the price of asserting an arrangement that is real rather than one held up by an allowance. Also say in the docstring what the two readings can and cannot fail on, so the next reader is not told a coverage claim the fixture no longer supports.
|
Both findings taken, in The 3px premise. Agreed, and it is worse than brittle — at 1200 there is no collision to assert at all. The bar clears the row and steps only because the sharing filter keeps a gutter, so the assertion was reading that allowance rather than the arrangement. The page now opens at 930, where the two overlap by 20.3px and the step is the row's own doing. This costs the tripwire the The named assertions still can't fail. Correct, and narrowing doesn't change it — the bar reaches the row's left edge and stops well short of the Accept centre both readings want, at every width in the band. The docstring now says so in its own paragraph: the precondition is a tripwire on the arrangement, not the coverage claim restored, and restoring the readings needs a fixture whose control the bar can cover. That is a separate change and I have not attempted it here. On #101. The |
…entres
The hit test and the press both ask where a control's centre is, and the
compact bar cannot reach one: it hangs 6px above the line it stands beside
and is the row's own height, so a bar that never stepped covers the row's
top edge and stops 1.25px short of its centre. Measured at every width
where the row still hangs in the margin, with placeClear's step disabled —
`under` came back empty at all of them.
Sampling the control's corners as well as its centre puts the claim back
where a broken walk falsifies it, and keeps the hit test rather than
trading it for rectangles. With the step disabled and the precondition
removed, the reading now fails on its own:
floating chrome is standing on controls:
['lf-sug-actions lf-ui', 'lf-pill lf-sug-accept lf-ui']
leaf-agent
left a comment
There was a problem hiding this comment.
The narrowing holds — reproduced on this branch's merge commit (d8ced76): at 930 the bar and the row overlap horizontally by 20.3px, the step is the row's own doing, and the precondition clears by placeClear's fixed 6px rather than by a font. Sweeping the band confirms the comment above the resized call too: the row docks at 899 and hangs at 900, so 930 sits 30px clear of the breakpoint with 20.3px of overlap to spare.
One correction to the new tripwire paragraph, since it is the paragraph the next reader will act on. It attributes the vacuity to a horizontal shortfall "at every width where the row still hangs in the margin", and that quantifier is false at the bottom of the band: at 900 the bar's right edge is 759.3 and the Accept centre is 751.5, so the bar is already 7.8px past the point both readings sample. What actually keeps them quiet at every width is vertical, and it is small — with placeClear's step disabled the bar sits at [693.8, 157.0, 759.3, 178.5] at every width in the band, and the pill's centre is at y 179.7. A 1.2px miss.
That changes the sentence after it. "Restoring the readings themselves needs a fixture whose control the bar can actually cover" reads as a fixture-sized problem; the measurement says the bar is 1.2px of its own height from covering that point, at the width this test already runs. Still a separate change, but worth recording at its real size so the next reader doesn't price it as a new fixture. Suggestion inline.
How each measurement was taken
All on d8ced76 (PR merge commit), uv run pytest -q -n0 --run-nightly with a temporary parametrized probe over SUGGESTION_PAGE, same selection gesture as the test.
Geometry across the band, as shipped:
W=899 docked=True stepped=False
W=900 docked=False stepped=True hoverlap= 50.30 bar=[693.8, 196.5, 759.3, 218.0] row=[709, 169.0, 885, 190.5]
W=930 docked=False stepped=True hoverlap= 20.30 bar=[693.8, 196.5, 759.3, 218.0] row=[739, 169.0, 915, 190.5]
W=960 docked=False stepped=True hoverlap= 18.06 bar=[721.6, 196.5, 787.1, 218.0] row=[769, 169.0, 945, 190.5]
W=1200 docked=False stepped=True hoverlap=-2.94 bar=[831.1, 196.5, 896.6, 218.0] row=[899.5, 169.0, 1075.5, 190.5]
The Accept pill and the two centres both readings sample:
W=900 accept=[709, 169.0, 794, 190.5] centre=[751.5, 179.7] bar right 759.3 -> 7.8px past
W=930 accept=[739, 169.0, 824, 190.5] centre=[781.5, 179.7] bar right 759.3 -> 22.2px short
Vacuity, with surface.js's last line of placeClear edited to if (false && y !== box.top) place(node, left, y):
| W | un-stepped bar | under |
|---|---|---|
| 900 | [693.8, 157.0, 759.3, 178.5] |
[] |
| 905 | [693.8, 157.0, 759.3, 178.5] |
[] |
| 910 | [693.8, 157.0, 759.3, 178.5] |
[] |
| 920 | [693.8, 157.0, 759.3, 178.5] |
[] |
| 930 | [693.8, 157.0, 759.3, 178.5] |
[] |
Bar bottom 178.5 against pill centre y 179.7 at every one — the conclusion in the docstring is right, the stated reason isn't.
The dock breakpoint is @media screen and (min-width: 900px) in assets/theme.css, on the body:has(.lf-sug-actions) rail reservation.
leaf-agent
left a comment
There was a problem hiding this comment.
501235c landed while the review above was being written, and it supersedes it: the review's correction — that the shortfall keeping both readings quiet is vertical and about 1.2px, not the horizontal one the old paragraph named — is what this commit acts on, and it goes further than the correction did by making the reading falsifiable again rather than just restating the size of the gap. I have deleted that review's inline suggestion, which targeted the paragraph this commit rewrote and would have clobbered it if applied. The measurements in the review body still stand as taken; treat the geometry table as background for 2cad1d2 and the rest as answered here.
Verified on 501235c:
- The test passes.
- The reading is genuinely non-vacuous now. With
placeClear's step disabled and the precondition assertion deleted,underfails on its own —floating chrome is standing on controls: ['lf-sug-actions lf-ui', 'lf-pill lf-sug-accept lf-ui'], exactly what the commit body claims. On2cad1d2the same tree passed. That is the coverage claim the test is named for, back under a causal contrast, which was the open finding from the first review. - The 1.25px in the docstring matches what I measured independently at 1.2px, and its "at every width where the row still hangs in the margin" is right for the vertical reading — the un-stepped bar sits at
[693.8, 157.0, 759.3, 178.5]at 900, 905, 910, 920 and 930 alike, against a pill centre at y 179.7.
No further findings. The precondition is now a tripwire in front of a claim that can fail on its own rather than in front of a quiet one, which is the arrangement the PR set out to state.
test_the_comment_button_stands_on_no_controlstopped testing anything when the comment bar shrank, and said nothing. It asks whether floating chrome covers a page control, and read the answer only at that control's centre — a point today's compact bar cannot reach at any width, so both of its readings went quiet together. This opens the page where the row is genuinely in the bar's way, asserts the bar stepped past it, and reads each control at its corners as well as its centre, which is where a bar that stopped stepping actually lands.Problem
The test's arrangement is the collision its docstring describes: a selection filling its lines puts the 💬 out in the margin, on the line where the suggestion hangs its Accept/Reject row.
placeClearsteps the bar down past that row, and the test checks nothing ended up covered.The arrangement went stale. The bar's width decides whether there is anything to step past. Carrying six reaction pills it measured 287.6px and reached 219px past
sug-refill's row. Afterf30a585(Simplify reaction controls) it carries 💬 and one ellipsis, measures 65.5px, and stops 2.9px short of the row. Its siblingtest_the_composer_opens_where_the_button_stoodstates its arrangement ("the button never stepped aside, so where it stood proves nothing") and so failed loudly on that commit; this test slept through it.And the reading could not have caught it either. Both readings ask where the control's centre is — once through
elementFromPoint, once by pressing Accept, which Playwright also does at the centre. The bar hangs 6px above the line it stands beside (besidereturnsrect.top - 6) and is the row's own height, so an unstepped bar spans y 157–178.5 against a row at 169–190.5: it covers the row's top edge and stops 1.25px short of its centre. Measured withplaceClear's step disabled at every width where the row still hangs in the margin —undercame back[]at all of them. So the walk could have been removed outright and this test stayed green, at any width; narrowing the window alone does not change that.Solution
Open the page where the collision is real. At the default 1200 the bar clears the row and steps only through
placeClear's 6px sharing gutter — 3.1px of slack, which is the allowance38bef20added saying "a one-glyph difference between system fonts must not decide whether two controls almost touch or the float steps clear." A precondition spending it would go red for a font and blame staleness. At 930 the bar (693.8–759.3) and the row (739–915) overlap by 20.3px and the step is the row's own doing; below 900 the row docks under its block and is out of the way again.Read each control at its corners as well as its centre. A press lands where the reader aimed, and the top of a pill is as much of it as the middle — a bar standing on it is a press stolen either way, which is what
placeClearitself treats as sharing. Sampling a 3×3 grid inset 4px keeps the hit test rather than trading it for rectangles, and it lands on the part the bar does reach:The precondition stays in front of it as the staleness tripwire, and the docstring states both — what each reading holds and why the centre alone could not.
What this costs, stated plainly. At 930 the pre-
f30a585wide bar overlaps the row too, so the precondition alone would not have caught that shrink — it passes on891a2d4. The coverage reading is what now carries the regression, and it fails on a walk that stops stepping regardless of the bar's width.Testing
The two assertions discriminate independently, measured on
501235cwithplaceClear's last line edited toif (false && y !== box.top) place(node, left, y):156.95 > 190.5floating chrome is standing on controls: ['lf-sug-actions lf-ui', 'lf-pill lf-sug-accept lf-ui']The third row is the one the centre-only reading could not produce at any width.
uv run pytest tests/test_render_anchors.py --run-nightly— 58 passed.uv run pytest tests— 700 passed, 6 skipped.pre-commit run --files tests/test_render_anchors.py— ruff check, ruff format, typos all pass.Geometry the diagnosis rests on
SUGGESTION_PAGEwith#replaceselected, probed straight after the fab appears. The step moves y only, so the bar's left/right are the same stepped or not.undermeasured with the step disabled:[left, top, right]sug-refillrow[left, right]under[831.1, 157, 896.6][899.5, 1075.5][][743.1, 157, 808.6][811.5, 987.5][][721.6, 157, 787.1][779, 955][][693.8, 157, 759.3][739, 915][][693.8, 157, 759.3][709, 885][][777.6, 157, 843.1][]Row and pills sit at y 169–190.5 throughout, so their centre is 179.75 and the unstepped bar's bottom is 178.5 — 1.25px short at every width, including 900 where it does reach the Accept pill's centre horizontally. That is why the centre-only reading was unfalsifiable rather than merely narrow, and why the corners restore it without needing a different fixture.
This came out of the
ci-fixrun for ci run 33229420192. The three failures that run reported are already fixed onmainby38bef20. #101's CSS half landed there independently; its test half — the sibling's narrowed window — has not, and stays that PR's to carry.