Skip to content

Repaint the key line when a drag crosses into a selection - #177

Merged
max-sixty merged 2 commits into
mainfrom
fix/ci-33445784773
Sep 1, 2026
Merged

Repaint the key line when a drag crosses into a selection#177
max-sixty merged 2 commits into
mainfrom
fix/ci-33445784773

Conversation

@leaf-agent

Copy link
Copy Markdown
Collaborator

Problem

Run 33445784773 on bdf9f44 came back 4 failed, 1586 passed, 6 skipped. Three belong elsewhere: the anchor pair is what #174 already carries and fixes, and the catalog preview digest is gone from the suite as of 4b42d15. The fourth is new — test_render_navigation::test_a_label_press_keeps_the_controls_keyboard_standing read '/\nsearch page\nesc\nlet go\n?\nmore\nd / u\npage down / up' where it wanted unselect, mid-drag, with getSelection() already holding after state.

rung()'s first rung is unselect whenever pageSelection() stands, so at that moment Escape would have cleared the selection while the line promised to let go of the control. The line was not wrong about the selection — nothing had repainted it. selectionchange returns early while pointerSelecting stands, deliberately, so the bar is not re-decided under a live drag, and no other writer repaints inside a press. The word the test read was therefore whichever side of the drag the frame that the press itself scheduled (through focusoutpaintHere) happened to land on: after the moves on an idle machine, before them on a loaded one, and then stale until the two-second heartbeat.

Solution

The pointerSelecting branch now repaints when the answer crosses — once per press in each direction. The bar keeps waiting for the release, so nothing about updateFab or the snap changes; only the line follows the drag. A drag that grows a selection already standing paints nothing, which is what keeps a whole paintHere out of every frame of a drag.

hold_selection gains frame_the_press, which lets the press's own frame land before the drag begins, and the test states it. Under that ordering the line's only route to the word is the selection the drag makes, which is the behaviour the assertion was always meant to be about.

Testing

The failure reproduces deterministically once the ordering is stated, and the fix is what clears it.

  • With frame_the_press=True and the runtime change reverted: FAILED … assert 'unselect' in '/\nsearch page\nesc\nlet go\n?\nmore\nd / u\npage down / up' — the run's exact line.
  • With both: 1 passed.

Suites, all on 4b42d15: uv run pytest tests (the PR gate) 749 passed, 6 skipped in 4:48. Under --run-nightly, test_render_navigation.py 63 passed; test_render_aim/reactions/conversations/commands/margin.py 155 passed; test_render_controls/drafts/options/options_addition/options_settled/outbox/projection/widgets/startup.py 394 passed. test_render_anchors.py fails only on the two #174 already owns.

node --check on the module, ruff check/ruff format --check and prettier@3.9.6 --check on the touched files: all clean. pre-commit itself is not on this sandbox's PATH, so eslint and typos went unrun here and CI's lint job is the first to run them.

How the ordering was measured

A probe recorded every change to .lf-keyline through a MutationObserver while driving the same drag. Press, then drag, reading at the end:

SEL: after state
LINE(now):  /|search page|esc|unselect|?|more|d / u|page down / up
LOG@read:   [[39, '…|esc|unselect|…']]
LOG@3s:     [[39, '…|esc|unselect|…'], [1895, '…|esc|unselect|…']]

One repaint at 39ms — the press's frame, landing after the ten moves — and then nothing until the heartbeat at 1895ms. Inserting a 120ms wait between the press and the moves, which is what a loaded runner supplies for free:

SEL: after state
LINE(now):  /|search page|esc|let go|?|more|d / u|page down / up
LOG@read:   [[38, '…|esc|let go|…']]
LOG@3s:     [[38, '…|esc|let go|…'], [1892, '…|esc|unselect|…']]

The same run's line, and the word arriving 1.9 seconds later from the heartbeat rather than from anything the reader did. With the fix, the same probe repaints during the drag and the press's frame no longer decides it.


Automated fix for failed run

The Escape rung reads the live selection, but nothing repainted the key line
inside a pointer press: `selectionchange` returns early while `pointerSelecting`
stands, so the word the line showed was whatever the frame the press itself
scheduled happened to catch. Idle, that frame landed after the drag had run and
the line said "unselect"; loaded, it landed first and the line went on promising
"let go" — the wrong key — until the two-second heartbeat repainted it.

The handler now repaints when the answer crosses, which is once per press in
each direction. The bar still waits for the release; only the line follows the
drag, and a drag growing a standing selection paints nothing, so no whole
`paintHere` lands inside every frame of one.

`hold_selection` gains `frame_the_press`, which lets that frame land before the
drag begins, and the label-press test states it — the ordering CI gives every
drag and an idle machine gives almost none, and the ordering under which the
line's only route to the word is the selection.
@leaf-agent

Copy link
Copy Markdown
Collaborator Author

Second occurrence, and an independent measurement that the runtime change alone carries the fix. Run 33447477986 on 78fa4c4 came back 4 failed with the same four sets: the test_render_anchors pair #174 owns, the catalog digest 4b42d15 removed, and test_a_label_press_keeps_the_controls_keyboard_standing reading the identical line — so this is not a one-off on bdf9f44.

What that run supplies that a single-shot cannot is a rate. On 4b42d15 with no test change at all, the stock test fails 10 of 12 sequential -n0 runs while tests/test_render_anchors.py tests/test_render_semantic_selection.py --run-nightly -n 4 runs beside it, and 0 of 15 with the machine idle — the loaded runner supplying for free the ordering frame_the_press states. Applying only this PR's surface.js change, and leaving hold_selection and the test untouched, it goes to 0 of 12 under that same load. So the crossing repaint is what clears the failure, and the harness change is what makes the assertion say what it means rather than what it needs to pass.

One more datum for the merge order: run 33448680853 on 4b42d15, main's tip, came back 2 failed, 1589 passed — the anchor pair alone, with this test green on an idle runner. The staleness is load-sensitive rather than gone, which is why it took a second commit to show itself.

Runs, and what each was measured against

All on 4b42d15, same sandbox and Chrome, --run-nightly -n0, the node being tests/test_render_navigation.py::test_a_label_press_keeps_the_controls_keyboard_standing:

tree load result
unmodified idle 0 failed / 15
unmodified test_render_anchors.py test_render_semantic_selection.py -n 4 concurrent 10 failed / 12
paintHere() in the pointerSelecting branch, no test change idle 0 failed / 6
paintHere() in the pointerSelecting branch, no test change same concurrent load 0 failed / 12

Every failure printed this PR's line verbatim: AssertionError: assert 'unselect' in '/\nsearch page\nesc\nlet go\n?\nmore\nd / u\npage down / up'.

The variant I measured is the unguarded form — paintHere() on every selectionchange inside the press, coalesced by the pending-frame flag in paintHere. This PR's selectionStood crossing test is the narrower one and does strictly less work; the rate above is evidence for the repaint, not against the guard.

A probe run before either change agrees with this PR's MutationObserver reading from the other side: with the drag complete and getSelection() holding after state against an anchor node of #text:after state, the line still said let go, and took 85 and 95 rAFs — 1.58s and 1.65s — to reach unselect on two loaded runs, against 1 frame on an idle one. That is the heartbeat arriving, not the gesture.

Suites with the unguarded variant applied: uv run pytest tests 749 passed, 6 skipped in 4:52; test_render_navigation/semantic_selection/anchors/drafts/reactions/aim/conversations.py --run-nightly 302 passed, 2 failed, the two being #174's.

# Conflicts:
#	skills/leaf/assets/runtime/composing/surface.js
@max-sixty
max-sixty merged commit 28fa324 into main Sep 1, 2026
3 checks passed
@max-sixty
max-sixty deleted the fix/ci-33445784773 branch September 1, 2026 03:19
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.

2 participants