Skip to content

Press z from the moment the line offers it - #24

Closed
leaf-agent wants to merge 4 commits into
mainfrom
fix/ci-32536744006
Closed

leaf-agent wants to merge 4 commits into
mainfrom
fix/ci-32536744006

Conversation

@leaf-agent

Copy link
Copy Markdown
Collaborator

Problem

Run 32536744006 failed test_z_waits_for_the_gesture_the_log_has_not_takenexpect(page.locator("#col-todo #card-baffle")).to_have_count(1), actual 0 — the eleventh red ci on main in a day from the same shape, one undo test per run, each reporting the page exactly as it stood before the press. z is the one press whose subject is read rather than pointed at, so the dispatcher holds it dead while the page holds a gesture the log has not taken (unrecordedGesture) — the trip its predecessor is still on included. These tests press the moment round_trip returns, and round_trip returns on the response while sending is cleared a turn later, when post has finished the poll it awaits. A z pressed in that gap is refused, and what the refusal leaves behind is the page an assertion on the un-undone state would find anyway, so the test reads a no-op as the wrong answer.

Solution

undo(page) waits for the line to offer the press, presses, and waits for the send to be counted before the trip — 9d35f25 already took that wait at the two sites it named, and this makes it the helper every acting press goes through, so the thirteen sites it didn't name are covered too. The three presses that stay bare are the two tests about a press being refused and the one whose tab cannot hear its own send back. The runtime half is what makes the first wait sound rather than lucky: sendAction moved half of unrecordedGesture with no repaint on either edge, where undoLast calls paintKeys() on both of its own, so the line could still offer z while the dispatcher was already refusing it. Both edges now paint.

Testing

Reproduced the CI failure verbatim by throttling the page's own CPU 20× before releasing the held send — which widens the window without slowing the client that presses into it — and reproducing it is what the fix is measured against.

Reproduction, and where it came from

The probe is test_z_waits_for_the_gesture_the_log_has_not_taken with Emulation.setCPUThrottlingRate at 20 sent just before held[0].continue_(), reading the send counter around the press instead of asserting:

bare press:   SENDS BEFORE: 2  AFTER PRESS: 2   card-baffle in col-done   log: [note, action, action]
undo(page):   SENDS BEFORE: 2  AFTER PRESS: 3   card-baffle in col-todo   log: [note, action, action, undo→newest]

The bare press sends nothing — refused, no undo in the log, and the move it should have taken back still standing, which is #col-todo #card-baffle resolving to 0 for the full budget. Through the helper the same press posts an undo naming the newest action, and the card comes back.

Network.emulateNetworkConditions does not reproduce it and neither does a sleeping page.route handler: the window is the poll's processing rather than its flight, and a sleep inside a sync route handler blocks the dispatcher, so the press it was meant to precede is queued behind the very poll it was widening.

Where the change comes from. #21 carries these two commits (f05a3e8, b6f6df6) under an unrelated runtime rewrite that has been in review for a day while main went red ten more times. They revert independently of it, so they are here on a branch off main; #21's merge takes either side of the same content.

What the resolution dropped. 9d35f25's two inline comments are not carried over — both state the general fact the helper's own comment block now states once, which is the reading tests/CLAUDE.md gains a paragraph for.

The suite

uv run pytest tests --run-nightly, the command CI runs, on this Linux runner: 920 passed, 6 skipped in 634.95s, taken at 9d35f25 (the base before the two commits main landed while this ran; the undo tests and the sixteen undo() sites pass on the current base too — 23 passed over -k "z_ or withdraw or rebuild or reopened_mid_fold or thread_follows or arriv"). pre-commit run --files plugins/leaf/skills/leaf/assets/leaf.js tests/test_render.py tests/CLAUDE.md is clean.


Automated fix for failed run

`ci` on main has been red on the undo tests for a day — a different one per
run, each reporting the page exactly as it stood before the press. Run
32536744006 failed `test_z_waits_for_the_gesture_the_log_has_not_taken` at
`expect(page.locator("#col-todo #card-baffle")).to_have_count(1)`, actual 0.

`z` is the one press whose subject is read rather than pointed at, so the
dispatcher holds it dead while the page holds a gesture the log has not taken
(`unrecordedGesture`) — the trip its predecessor is still on included. These
tests press the moment `round_trip` returns, and `round_trip` returns on the
response while `sending` is cleared a turn later, when `post` has finished the
poll it awaits. A `z` pressed in that gap is refused, and what the refusal
leaves behind is the page an assertion on the un-undone state would find
anyway — so the test reads a no-op as the wrong answer. Nothing is wrong with
the runtime; the gate is what keeps the walk from naming the gesture before the
one the reader just made.

So the press waits on the fact the page states. `undo(page)` waits for the line
to offer the press, presses, and waits for the send to be counted before the
trip. 9d35f25 took that wait at the two sites it named; this makes it the
helper every acting press goes through, so the other thirteen are covered too.
The three presses that stay bare are the two tests about a press being refused
and the one whose tab cannot hear its own send back.

The offer has to be the dispatcher's own answer for that wait to be sound, and
`sendAction` moved half of `unrecordedGesture` with no repaint on either edge,
where `undoLast` calls `paintKeys()` on both of its own. Both edges now paint,
so a key on screen is a key that works.

Extracted from #21, which carries these commits under an unrelated runtime
rewrite; this reverts independently of it.

@leaf-agent leaf-agent left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The runtime change reads right to me — the line's answer trailing the dispatcher's by up to a poll interval is the thing "a key on screen is a key that works" exists to rule out, and sendAction is the layer's own door, so both edges are owed there rather than by whoever remembers. The test helper is a clear win too: undo(page) waits on a fact the page states instead of on the gesture before it having painted, and the two places that must not use it (the negative z presses in test_z_reaches_the_gestures_made_on_the_version_being_read and test_z_waits_for_the_gesture_the_log_has_not_taken, and the stopped-poll tab in test_a_withdrawal_restores_what_still_stands_not_what_stood_then) are correctly left alone.

The problem is that nothing in the suite fails without the runtime hunk. I checked rather than reasoned about it: reverting both paintKeys() calls in sendAction and running all fourteen tests that call undo(), plus the hand-pressed one in test_a_withdrawal_restores_what_still_stands_not_what_stood_then, gives 15 passed, 0 failed. That is tests/CLAUDE.md's own bar, under A sweep that walks controls by index must prove it pressed them: "check a new gate by putting each bug back and watching the gate fail: a gate that has only ever passed has been tested for nothing."

The run, and why the suite can't see either edge

Against df64a23's merged tree, with the two paintKeys() calls deleted from sendAction and nothing else changed:

$ uv run pytest tests/test_render.py -q -n0 --run-nightly -k "<twelve of the undo() tests, plus the hand-pressed withdrawal>"
13 passed, 465 deselected in 48.40s
$ uv run pytest tests/test_render.py -q -n0 --run-nightly -k "test_z_waits_for_the_gesture_the_log_has_not_taken or test_the_thread_follows_the_decision_that_still_stands"
2 passed, 476 deselected in 4.71s

The end edge is invisible by construction. poll() ends by dispatching lf-actions, document.addEventListener("lf-actions", syncAsks) is unconditional, and syncAsks ends in paintHere() — so every poll schedules a frame. post awaits that poll before returning, and sendAction's finally runs as a microtask off await post(...), which is before the rAF the poll just scheduled. So the frame that lands after a successful send already reads the cleared sending whether or not the finally paints. The finally call earns its place only on the path where post returns before await poll() — a send the server refused or a dead server — and no undo test takes that path. Which also means a gate written the obvious way ("after the trip, the line offers undo") would be born vacuous, in the sense tests/CLAUDE.md gives that phrase under A test goes vacuous when the code stops being able to fail it: "Before asserting that a gesture did not travel, ask what would have stopped it anyway."

The start edge is the one a gate can bite on, and the reason the existing not_to_contain_text("undo") in test_z_waits_for_the_gesture_the_log_has_not_taken doesn't is that the gesture there is a board grab — .lf-dragging is already dropping the chip, so the assertion holds with the send edge unpainted. A gesture that isn't a drag is what separates them: complete one gesture so undoable() is truthy, page.route the next /api/event and hold it, make a pick or an accept, and read the line. Unpainted, it still offers undo — the last paint was the first gesture's — over a press the dispatcher is already refusing.

And with paintHere coalescing to a frame, the read has to consume one. #22 already added exactly the helper for that (_painted_line, which awaits two rAFs and reads once rather than through a retrying expect); its own new test is the same shape as the one this PR wants. Whichever of the two lands second inherits it.

Worth noting alongside: #22 is the other half of this same predicate — .lf-dragging's edges, where this is sending's — and undoing already paints its own. Between them unrecordedGesture is fully painted, and neither conflicts with the other textually, but the pair reads as one change split across two reviews. I don't think that's wrong, only worth saying out loud so the second one to land isn't reviewed as if the predicate were already covered.

Nothing else came up: sending.set before the paint is the right order, the burst case (accept-all) stays dead throughout because each send increments the map, paintHere's coalescing means the pair really does cost nothing, and the added expect(body).to_have_text(authored) before the strict inner_text() compare in test_z_walks_back_through_gestures_rather_than_toggling_one closes a real gap — round_trip returns on the response event, which is before the page has applied what came back, so the bare assert was reading a page that might not have rebuilt yet.

Comment thread plugins/leaf/skills/leaf/assets/leaf.js
The runtime hunk had no test that failed without it: every undo test
passed with both `paintKeys()` calls in `sendAction` deleted, because
something else always painted the frame the assertion read.

Two gates now, one per edge, each checked by putting its own bug back
with the other edge intact. The start edge holds the send in the wire
and reads the line once a frame after the press; the end edge takes the
one path where the door is the only reporter — a refused send returns
out of `post` before it polls, so the poll's own paint never comes, and
the toast written on the way out is the fact ahead of the door's edge to
read from.

The gesture is a suggestion's accept, pressed on a control already
focused with the page settled first, because every neighbouring gesture
paints for the door: a pick announces `lf-answered` before its send
leaves, a click focuses what it lands on, a dropped card repaints its
grips. That is the norm the tests carry back to tests/CLAUDE.md.

@leaf-agent leaf-agent left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The gate bites now, and per-edge — I put each bug back separately rather than trusting the pair. Against this head's merged tree:

  • Start edge deleted (paintKeys() before try): test_the_line_drops_undo_as_the_gesture_goes_into_the_wire fails on the line offered a press the dispatcher was already refusing, with the chip still reading z / undo while the send sits held in the wire.
  • End edge deleted (paintKeys() in the finally): test_the_line_offers_undo_again_when_a_send_the_server_refused_ends fails on the line withheld a press the dispatcher was ready to takeundo absent from a line that lists every other row.
  • Both deleted: the first fails, the second passes.

That last line is the one worth writing down somewhere, because it is a coupling and not a fact about either test: the refused-send test reads undo present, so it can only speak once something has taken the chip away, and the only thing that does is the start edge. Delete the start test later on the grounds that "the store is covered", and the end edge silently stops being covered too. The pair is complete over the mutation space; neither half is on its own.

The reasoning that shaped both — press-not-click so focusin doesn't paint for the door, the suggestion rather than the pick because a decision's choreography waits for the log, page_at_rest first so no frame is already owed, one reading instead of a retrying expect — checks out against the runtime: #pick dispatches lf-answered before sendAction is reached, where #decide reaches #settle only inside .then(), and the focusin listener does end in paintHere(). And the refused path really is the only one where the finally is the sole writer: post toasts and returns from its catch without ever reaching await poll().

Two assertions in the refused-send test don't hold up their end, both in the "nothing was decided" pair at the foot of it. Suggestions inline.

Why lf-new can't fail there

sug-thistle is the insertion-only suggestion — <lf-new> and no <lf-old> — so on accept there is no slot whose x-retired-when matches, and nothing retires. I let the same send through in the other new test and read the widget afterwards:

visible: True
data-lf-state: accept
accept control: ✓ Accepted

So expect(page.locator("#sug-thistle lf-new")).to_be_visible() holds identically whether the decision stood or not — it is the shape tests/CLAUDE.md names under A test goes vacuous when the code stops being able to fail it. data-lf-state is the reading that separates the two, and it is the one the sibling test one screen up (test_a_decision_the_server_never_took_goes_back_to_pending) already takes for exactly this fact.

The console line beside it is the same test's other half: that sibling pins errors == ["Failed to load resource: net::ERR_FAILED"] exactly, where the filter form here passes on none of them and on five alike. post is one attempt by construction, so exactly one is what this page produces — I ran the pinned form three times and it is deterministic.

Nothing else came up. The undo() helper's three exemptions still look right, the new tests/CLAUDE.md paragraphs describe what the code does rather than what it was meant to do, and the full suite is green on this head.

Worth restating only because it is now concrete rather than predicted: #22 lands _painted_line, which is the page.evaluate(RENDERED) + inner_text() pair these two tests spell out inline. The two runtime hunks are in different regions of leaf.js and don't conflict, so whichever merges second is the one that folds the reading into the helper.

Comment thread tests/test_render.py Outdated
Comment thread tests/test_render.py Outdated
Both assertions at the foot of the refused-send test passed on a page that
had decided as readily as on one that hadn't. `sug-thistle` is the
insertion-only suggestion — `<lf-new>` and no `<lf-old>` — so an accept
retires no slot and `lf-new` stays visible either way; letting the same send
through and reading the widget gives `visible: True` beside
`data-lf-state: accept`. The mark is the reading that separates the two, and
it is the one the sibling refusal test one screen up already takes.

The console line beside it filtered out every `net::ERR_FAILED` rather than
counting them, so it passed on none and on five alike. The count belongs to
the test here rather than to the machine: the route holds `/api/event`, which
is the send, and `post` makes one attempt — the poll is `/api/state` and goes
untouched, which is the case tests/CLAUDE.md's "a test cannot assert over
noise it makes itself" distinguishes from a refused poll. Five runs of the
pinned form were deterministic.
@leaf-agent

Copy link
Copy Markdown
Collaborator Author

ci went red again on 3600257run 32537764838, which is the commit this branch is based on, so it is the same shape one commit further along. Two things in it are worth adding here rather than opening a second PR: it is the first run to fail two undo tests at once, and the second of them failed in a shape undo() alone does not close — which is evidence that the expect(body).to_have_text(authored) line this branch adds is load-bearing rather than belt-and-braces.

I reached the same two halves independently before finding this PR, and with only those two halves in the tree — the helper plus paintKeys() on both edges of sendAction — a full uv run pytest tests --run-nightly still failed at assert body.inner_text() == authored in test_z_walks_back_through_gestures_rather_than_toggling_one. Instrumenting it showed the press was fine: both undo events were in the log, and re-reading the body a moment later inside the failure handler returned the restored text. That is the bare-read window rather than the dead-press one, so the retrying expect above it is what closes it, and a reviewer reading that line as redundant with undo() would be reopening the failure.

Independent full-suite run on a Linux runner at 6351263 (the head before the assertion-tightening commit that landed while it ran): 923 passed, 6 skipped in 627.05s.

The two failures in run 32537764838, and the instrumented reading

Both in one run, on different workers:

FAILED test_z_walks_back_through_gestures_rather_than_toggling_one
  AssertionError: assert 'Rewritten.' == 'First line o... of the note.'
FAILED test_z_waits_for_the_gesture_the_log_has_not_taken
  AssertionError: Locator expected to have count '1'
  Actual value: 0    (14 x locator resolved to 0 elements)

The second is the dead press this branch's undo() fixes — the expect spent its whole budget, so the press had genuinely sent nothing. The first is not: it failed instantly, and with the helper in place. Replacing the bare read with a handler that dumps the log gave, on a full-suite run:

PROBE body='First line of the note.\n\nSecond paragraph of the note.'
log=[note, action edit "Rewritten.", action choose [opt-a],
     undo -> ed59bee0 (the choose), undo -> 4e5c9025 (the edit)]

Both undos accepted, and the body already restored by the time the handler re-read it — the assert had taken the frame before. Traffic's own docstring is where this is written down: the counters are taken where the fetch settles, and "what the runtime then does about the answer is later than either, and it is the auto-retrying expect after the wait that covers it". A press has no retry, which is this branch's helper; a bare read has none either, which is that line.

Separately measured while diagnosing, on an idle machine: reading .lf-keyline immediately after round_trip following a widget send found the undo chip missing on 1 run in 8 — so the window is wide enough to hit without any load at all, which matches this going red on a majority of runs under the suite's own.

@leaf-agent

Copy link
Copy Markdown
Collaborator Author

Both review points are taken in 98d164f, replied inline, and ci is green on that head.

One thing found on the way, not carried into this PR: the sibling refusal test the review cites twice as test_a_decision_the_server_never_took_goes_back_to_pending is really test_a_decision_the_server_never_took_never_shows_as_taken. The old name is still on main, in the comment above refuse (test_render.py#L626, from eaa61e6) — which is where the review read it, so the comment is actively pointing readers at a test that doesn't exist. It's a one-word fix and unrelated to this branch, so it's here rather than in the diff; the bot has six PRs open, which is why it isn't its own.

@leaf-agent

Copy link
Copy Markdown
Collaborator Author

Third red in a row on this shape, now at fa59396run 32540635010, attempt 2 (2 failed, 920 passed). One of the two is a site nothing on the thread has named yet, and it is the case for sweeping past the two sites 9d35f25 fixed by hand rather than the case for the sweep in the abstract.

test_a_rebuild_keeps_what_the_reader_did_inside_the_change failed with the accept still standing after the press — ✓ Accepted, aria-disabled="true", re-resolved 14 times over the full 5s budget. Its press is bare at that commit (test_render.py#L15026), sitting directly on the round_trip after the accept click, and this branch converts it to undo(page). That the press left the un-undone state exactly as an assertion on it would find anyway is the reading this PR's helper comment describes; I have not proved the refusal from the run's own log, since the failure output carries no send counters — the alternative would be a live press whose replay declined to withdraw, which nothing else on the thread suggests.

The other failure is test_z_walks_back_through_gestures_rather_than_toggling_one again, at the same assert body.inner_text() == authored, one commit further along — a second field instance of the bare-read window the comment above already diagnosed, not new information about it.

What the pair of them says together is about coverage. 9d35f25 put the keyline wait on two sites by hand, main went green twice (2d75501, 9d35f25), and the next push went red at one site that commit had covered and one it had not. The hand-picked pair is the same fix at a smaller radius, and the radius is what failed.

The run, and the state of the two attempts

Attempt 2 in full:

FAILED tests/test_render.py::test_z_walks_back_through_gestures_rather_than_toggling_one
  AssertionError: assert 'Rewritten.' == 'First line o... of the note.'
FAILED tests/test_render.py::test_a_rebuild_keeps_what_the_reader_did_inside_the_change
  AssertionError: Locator expected to have text '✓ Accept'
  Actual value: ✓ Accepted
  14 × locator resolved to <span ... aria-disabled="true" class="lf-pill lf-sug-accept lf-ui">✓ Accepted</span>
============= 2 failed, 920 passed, 6 skipped in 645.70s (0:10:45) =============

Both z presses are bare at fa59396 in the sense this branch means: L14661 has 9d35f25's keyline wait in front of it but no wait on its own send, and L15026 has neither.

Attempt 1 of the same run is a different failure — test_a_key_on_screen_is_a_key_that_works, 30s Locator.click timeout, 1 failed, 921 passed — and #25 is open against it. Two attempts of one commit failing two disjoint sets is the rerun moving the failure rather than either PR being wrong about its own; between them the two PRs cover everything red on that commit, and neither alone greens it.

Recent ci on main, newest first:

run commit attempt conclusion
32540635010 fa59396 2 failure
32540635010 fa59396 1 failure
32538939129 2d75501 1 success
32538592183 9d35f25 1 success
32537764838 3600257 1 failure
32536744006 11b4d44 1 failure

@leaf-agent

Copy link
Copy Markdown
Collaborator Author

Fourth red on this shape, now at cf35cb0run 32549479170, 1 failed, 948 passed, test_z_walks_back_through_gestures_rather_than_toggling_one at the same assert body.inner_text() == authored. What is new is not the instance but the proof: the comment above diagnosed the bare-read window by inference and said so ("I have not proved the refusal from the run's own log"), and it reproduces on demand under a 20× CPU throttle, 3 runs out of 3, at this commit on a Linux runner.

The press is live, and the log proves it. Both undos are recorded, newest first, exactly as the test's own closing assertion wants them — so the second z was taken and withdrew the edit. What is early is the read: round_trip returns on the poll, and the draft's rebuild paints after it, so the bare inner_text() at test_render.py#L15060 reads 'Rewritten.' and has no second chance. This branch's expect(body).to_have_text(authored) in front of it is the whole of the fix for this failure — not the helper's send-count wait, and not the runtime repaint, neither of which is reachable here (the gesture before this press is undoLast, which already paints both its own edges). Running that one line in the probe turns the same throttled page green.

So this failure is separable from the refusal shape the PR is named for, and the branch closes both. It is also the only site of its kind: one bare read of a locator sitting on a round_trip across the whole of test_render.py.

The reproduction

The probe is this test with Emulation.setCPUThrottlingRate at 20 sent after the choose click, reading the body once immediately after the second press's round_trip and again after the arrival wait:

FIRST READ AFTER round_trip: 'Rewritten.'
AFTER expect ARRIVAL:        'First line of the note.\n\nSecond paragraph of the note.'

Three runs, all three identical. The log the same page left, which is what says the press was taken rather than refused:

note   fc3b44ce
action edit    db5fa191
action choose  25a62a71
undo   → 25a62a71
undo   → db5fa191

That is the pair the test asserts at the end, in the order it asserts them, so nothing about the withdrawal is wrong — only when the assertion reads the page. Unthrottled, the same read lands after the paint and the test passes, which is why it passes locally and fails on a loaded runner.

The scan for siblings, over tests/test_render.py: every round_trip(page) followed within two lines by a bare assert/assignment reading inner_text, text_content, input_value, get_attribute or count off a locator. One hit, L15060 — this one.

Both sides added to `sendAction` at the same two points and neither knows
about the other: this branch repaints the key line on each edge of a send,
main threads the minted event through the `finally` so a forced poll cannot
paint an older projection over a newer gesture. The resolution keeps both —
`paintKeys()` on entry, and once at the end of the `finally` so it runs
whether or not this was the last send in flight, which is where the branch
had it.
@leaf-agent

Copy link
Copy Markdown
Collaborator Author

Fifth red on this shape, at 002d502run 32555467993, 1 failed, 956 passed. It is test_a_rebuild_keeps_what_the_reader_did_inside_the_change at ✓ Accepted, aria-disabled="true", re-resolved 13 times over the full budget: the same site and the same reading as the third comment above, one commit further along, so it adds no diagnosis — only that the branch has now been the fix for a site main has failed at twice.

What is new is that the branch stopped merging while it waited. cf35cb0 and 871edb2 rewrote sendAction around the same two points this branch writes at, and git merge-tree origin/main conflicts in leaf.js alone — tests/test_render.py still merges clean, so the half that closes today's failure was never at risk. Resolved in 3a4a738 and pushed here: neither side knows about the other, so both stand. paintKeys() on entry, main's let minted and its setTimeout reconciliation in the finally, and paintKeys() once at the end of that block — outside the if (left), which is where the branch had it, so the line repaints whether or not this was the last send in flight.

Verified on this Linux runner at the merge commit: uv run pytest tests --run-nightly in two halves, 486 passed for test_render.py and 473 passed, 6 skipped for the rest — 959 of the 963 CI collects, the balance being the skips. pre-commit run --files plugins/leaf/skills/leaf/assets/leaf.js tests/test_render.py tests/test_interact.py tests/CLAUDE.md is clean.

The failure, and the conflict
FAILED tests/test_render.py::test_a_rebuild_keeps_what_the_reader_did_inside_the_change
  AssertionError: Locator expected to have text '✓ Accept'
  Actual value: ✓ Accepted
  13 × locator resolved to <span ... aria-disabled="true" class="lf-pill lf-sug-accept lf-ui">✓ Accepted</span>
============= 1 failed, 956 passed, 6 skipped in 636.99s (0:10:36) =============

The press was still bare at that commit — page.keyboard.press("z") sitting on the round_trip after the accept click (test_render.py#L15361) — and this branch converts it to undo(page).

The conflict, both hunks in sendAction:

  • after sending.set(...): this branch's comment and paintKeys() against main's let minted;
  • in the finally: this branch's else sending.delete(el.id); paintKeys(); against main's else { … setTimeout(…) }

Recent ci on main, newest first:

run commit conclusion
32555467993 002d502 failure — this shape
32554644583 871edb2 success
32553975712 f35dc70 failure — test_a_decision_travels_between_tabs_and_the_log_has_the_last_word, a shape nothing on this thread covers
32549479170 cf35cb0 failure — this shape
32540635010 fa59396 failure — this shape

@leaf-agent leaf-agent left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both points from the last round are taken correctly, and I checked the replacements rather than reading them. data-lf-state really is the reading that separates the two: #settle is reached only inside sendAction(...).then(...) in lf-suggestion.js, so a refusal leaves the mark unwritten, and markSettled is what would write it on the way back. The pinned console entry holds for the reason the comment gives — post is one attempt by construction and the route holds /api/event alone — and it matches the sibling at test_render.py#L9581 exactly. The merge resolution is right too: return minted is preserved, main's checkpoint and its deferred reconciliation stand untouched, and paintKeys() sits outside the if (left) so it runs on every send's end rather than the last one's.

The end-edge gate stopped biting when main merged in. The mutation testing in the last round was done at 6351263, against a merge base from before cf35cb0 and 871edb2 rewrote sendAction — and what those added is a new writer of the exact frame test_the_line_offers_undo_again_when_a_send_the_server_refused_ends reads. I put the bug back at this head, on this runner: delete the paintKeys() at leaf.js#L986 and that test passes, four runs out of four, while the start-edge test still fails on the line offered a press the dispatcher was already refusing. So the pair is no longer complete over the mutation space — and it is the half that was harder to reach that went, since the start edge is the one anything else could have covered.

The substitute painter is main's setTimeout in the finally, and both of its statements paint. reconcileState() at L981 rebuilds the suggestion, and the rebuild's own focus churn reaches paintHere through the focusout and focusin listeners; the lf-actions dispatch at L982 reaches it through syncAsks. On a refused send minted is null, so the guard in front of both is !minted and it is true every time. This is the shape the PR's own new paragraph in tests/CLAUDE.md legislates against — "Where the fact under test is what the page says at an instant, list who else paints that frame before believing the green" — arriving from the side the list can't see, which is a change to somebody else's code landing after the list was taken.

The question I'd want answered before the gate is rewritten is whether the runtime call is still load-bearing, because a rewritten gate has to sit on a path where it is. The refused send was the whole justification for the end edge — a send the server takes is answered by a poll that paints on its own account — and that path is now painted unconditionally by dispatchEvent(lf-actions) a macrotask later. What is left looks like the success path whose poll threw inside post: minted is set, events doesn't hold it, so the setTimeout guard is false and nothing paints until the next timer poll. If that is the answer, the gate belongs there; if the call is genuinely redundant now, deleting it and the test is the smaller change, and the start-edge test still carries the store.

The runs, and the trace that names the painter

At this head's merged tree (identical to 3a4a738's), pinned headless shell, one worker:

$ uv run pytest tests/test_render.py -q -n0 --run-nightly -k "<the two gates>"
2 passed, 484 deselected in 4.44s          # baseline

# end-edge paintKeys() deleted, nothing else changed
2 passed, 484 deselected in 3.02s
1 passed, 485 deselected in 1.56s          # the refused-send gate alone, x3
1 passed, 485 deselected in 1.57s
1 passed, 485 deselected in 1.58s

# start-edge paintKeys() deleted instead
FAILED test_the_line_drops_undo_as_the_gesture_goes_into_the_wire
  AssertionError: the line offered a press the dispatcher was already refusing
1 failed, 1 passed, 484 deselected in 3.24s

Deleting the lf-actions dispatch as well as the end edge still leaves the refused-send gate green, which is what sent me to the trace rather than to the dispatch. Logging every paintHere entry with its stack, on the refused send with the end edge deleted:

paintHere <- focusout listener (leaf.js:8616) <- lf-suggestion disconnectedCallback
          <- rebuild <- reconcileState <- leaf.js:981
paintHere <- focusin listener (leaf.js:8614) <- standOn
          <- rebuild <- reconcileState <- leaf.js:981
paintHere <- syncAsks (leaf.js:9064) <- leaf.js:982

Three writers where the gate assumes none, all inside the one setTimeout. The first two are the reader standing on the accept control the rebuild replaces, so they are contingent on where focus is; the third is not, which is why the dispatch alone is enough to keep the gate green whatever the reader is doing.

The rest holds up. undo()'s closing round_trip is level-triggered (t.read >= t.sends), so a send that has already settled by the time it is called returns at once rather than waiting for a trip that will never come — the one way that helper could have deadlocked. Its viewport caveat is real and no call site breaks it: all fifteen are on open_page's default 1200×900. The three exempted presses still look right, and the added expect(stale.locator(".lf-keyline")).to_contain_text("undo") in test_a_withdrawal_restores_what_still_stands_not_what_stood_then takes the half of the helper that survives a stopped poll.

@leaf-agent

Copy link
Copy Markdown
Collaborator Author

Superseded by #21, which merged as 85631b3 about ten minutes after the comment above — so read that one as overtaken. This PR's own body said #21's merge would take either side of the same content, and it did: closing it rather than asking for a conflict resolution on a branch whose fix is already on main. The branch stays; nothing is deleted.

Both halves are in main, and the site today's run failed at is covered. undo(page) is at test_render.py#L714 with 20 call sites, test_a_rebuild_keeps_what_the_reader_did_inside_the_change presses through it at L16943, and tests/CLAUDE.md carries the norm paragraph reworded for the outbox. The runtime half arrives as a different mechanism rather than as this diff: sendAction no longer holds the in-flight map at all, and the fact z reads is unaccountedGesture, over an outbox that paints the line on both of its edges — paintKeys() after the push, if (removed) paintKeys() when a read accounts for the attempt. The two gates this branch added for those edges have a namesake there too: test_z_waits_for_an_unanswered_thread_resolution holds a send in the wire and asserts the line drops undo before the press, which is what test_the_line_drops_undo_as_the_gesture_goes_into_the_wire was for.

3a4a738, pushed here half an hour ago, merged main-before-#21 into this branch and is now moot. It was green — 486 passed for test_render.py, 473 passed, 6 skipped for the rest — which says the resolution was sound, not that it is still wanted.

Main's ci at 85631b3 is the run that decides whether the shape is closed; I'm watching it and will say where it lands.

@leaf-agent leaf-agent closed this Aug 22, 2026
@leaf-agent

Copy link
Copy Markdown
Collaborator Author

ci is green on main at 85631b3run 32557146434. Fifth red closed by #21 rather than by this branch, and nothing is left open on this shape.

@leaf-agent

Copy link
Copy Markdown
Collaborator Author

Answering the question the review left, and then the thing that outranks the answer: the call was still load-bearing on this branch, but this branch closed as superseded four minutes after the review landed, so what matters is what 85631b3 has — and it has both line-paint edges with no gate on either.

On this branch, the end edge was still the only writer on one path, and it is the one the review guessed: a send the server took whose forced poll faults before its own dispatch. post catches that (leaf: poll after send) and the send stands, the log stands where it was, and so the deferred reconcile's guard — !minted || events.some(id === minted.id) — is false exactly there, where on a refusal it is true and dispatches lf-actions. I rewrote the gate onto that path: a board move rather than a suggestion accept, because #settle runs in the continuation of the send and ends in lf-answered, which is a line paint — the property that makes a suggestion the right widget for the start edge is what disqualifies it from the end. Bug-back at this head: end edge deleted, the new test fails 3/3 on the line withheld a press the dispatcher was ready to take; start edge deleted, test_the_line_drops_undo_as_the_gesture_goes_into_the_wire fails; both deleted, both fail — so the coupling the last round wrote down is gone, and neither half rests on the other.

On main the same pair says something different, and that is the finding. I ported both gates to a worktree at 85631b3 and ran the same bug-backs there. The start-edge gate ports as-is and bites (2/2, on post's paintKeys() before void drainOutbox()). The end-edge gate passes with drainOutbox's paintKeys() deleted, 3/3 — not because the call is redundant but because the path the gate reads no longer exists: #21 made a send's answer carry the state, so deliver calls receiveState(answer.state) and there is no separate read to fault. Traced rather than inferred — with the end edge deleted, the one paint in the send's window is paintHere <- paintKeys <- … <- receiveState <- deliver. An end-edge gate on main has to fault the state inside the answer instead, which is the path leaf: state in event response names.

So main has a start edge that a ported test would gate today and an end edge whose gate has to be re-derived on the answer path. I have not opened a PR: six of mine are open already, and this is not a wrong outward action. Say the word and I will open one carrying the start-edge gate, the end-edge gate re-derived on receiveState, and the tests/CLAUDE.md note below.

The end-edge test as it stands here, and the note it came with

The one line that differs on main is the console assertion: this branch's faulted read is reported (leaf: poll after send), and on main the same routing leaves the console silent, since the read is not what the send waits on.

def test_the_line_offers_undo_again_when_a_sends_own_read_faults(browser, serve):
    """The other edge of the same store, on the path left where the door is the only one
    that can report it. A send is answered by a poll, and a poll that reaches its own end
    dispatches `lf-actions`, which paints — so the ordinary trip, and the refused send
    whose deferred reconciliation dispatches the same event a task later, both have a
    writer of their own. A poll that faults partway has neither: the fault is its own news
    and the send still stands (post), the log stands where it was, and the reconciliation
    is guarded on the log holding the id the server just minted. Nothing downstream of the
    send paints the frame the cleared store belongs to.

    Live is the log's word, and the faulted read never brought this gesture back to it:
    what the offer names is the move before it."""
    page, errors = open_page(browser, serve(BOARD_PAGE))
    line = page.locator(".lf-keyline")
    move = ["Enter", "ArrowRight", "Enter"]
    page.locator("#card-heater .lf-grip").focus()
    for key in move:
        page.keyboard.press(key)
    round_trip(page)
    page_at_rest(page)
    assert "undo" in line.inner_text()

    # Answered, so the read the send ends with lands and the trip is over; unreadable, so
    # the poll throws where it parses and none of what it ends with runs.
    page.route(
        "**/api/state",
        lambda route: route.fulfill(
            status=200, content_type="application/json", body="{"
        ),
    )
    page.locator("#card-baffle .lf-grip").focus()
    for key in move:
        page.keyboard.press(key)
    # The toast is the widget's word for a send the server took, written in the
    # continuation of a promise the door's own edge has already left — which is what
    # makes it a fact to read the line from.
    expect(page.locator(".lf-toast")).to_contain_text("Moved to Done")
    page.evaluate(RENDERED)
    assert "undo" in line.inner_text(), (
        "the line withheld a press the dispatcher was ready to take"
    )
    # The send the poll faulted after is in the log, so what stood between the press and
    # this reading was the read alone.
    assert [e["detail"]["card"] for e in actions(serve.page_dir)] == [
        "card-heater",
        "card-baffle",
    ]
    page.unroute("**/api/state")
    # Every read is faulted for as long as the window is held open, and how many of them
    # a run gets through is the machine's answer rather than this test's — so what is
    # asserted is that the page said nothing but that: the send's own read, and any timer
    # poll that landed beside it, each reported by the runtime under its own name.
    assert {e.split(" ")[0] for e in errors} == {"leaf:"}, errors
    page.close()

The paragraph it added to tests/CLAUDE.md, under A test goes vacuous when the code stops being able to fail it:

The list is a fact about the code at the moment it was taken, and the other end of the same send is where that came due. The gate on the door's closing edge was written on a refused send, whose whole justification was that a refusal never polls and so has no painter of its own — and while it sat in review, main gave the failed path a deferred reconcile that dispatches lf-actions, which paints. The green survived the merge and the gate stopped biting, in a branch whose subject was this very question. So a bug-back is owed again whenever the code around such a gate moves, and the path a gate reads on is chosen for where the call is the only writer rather than for where it is the natural one.

max-sixty added a commit that referenced this pull request Aug 24, 2026
## Problem

`ci` went red on main at
[fa59396](fa59396)
([run 32540635010, attempt
1](https://github.com/max-sixty/leaf/actions/runs/32540635010/attempts/1),
921 passed / 1 failed) on `test_a_key_on_screen_is_a_key_that_works`,
timing out for 30s trying to click the first thread inside the resolved
disclosure. The commit itself touches an example, `SKILL.md`, the
bundled registry and one new unrelated test — nothing on this path.
(That run was rerun, and attempt 2 failed on a different pair — the
`z`-press shape #24 is open against. The two attempts are disjoint, so
neither PR alone turns `main` green.)

The test resolves two threads and takes `Resolved (2)` on the summary as
its cue to press the disclosure open. That count is the log's:
`renderThreads` writes it the frame the resolve settles, while the
thread itself only reaches the disclosure when its 220ms fold is over
("Counted off the log, listed off the page",
[leaf.js](https://github.com/max-sixty/leaf/blob/31502f5580285a7f3e8ba2aac349cceede8a2746/plugins/leaf/skills/leaf/assets/leaf.js#L4788-L4791)).
So the press lands inside the fold, on a list still moving under the
pointer — which is what the failing call log reports on the very next
action, `element is not stable` twice before it settles into `element is
not visible` for the remaining 27 seconds. It is the shape
`tests/CLAUDE.md` names in "A state the page passes through is not a
state to poll for".

## Solution

Wait for the fact the fold states — the disclosure listing the threads
it now holds — before pressing the summary, at the three sites that
press without it. That is the pattern every other resolve test in the
file already follows (e.g.
`test_resolving_an_early_thread_renumbers_the_rest_in_place`, which
asserts `.lf-details .lf-thread[data-id=…]` before its own summary
press). Nothing is relaxed: each site gains an assertion.

Two of the three took the count instead. The third,
`test_a_resolved_thread_can_be_reopened`, took `round_trip` — a tighter
window rather than a safer one, since `foldOut` is driven from the
reconcile that trip comes back on, so the trip lands as the fold
*begins* and the press goes out into the whole 220ms. It had been
surviving on Playwright's own stability retry, which is exactly the thin
part the failing run's `element is not stable` names.

`test_a_late_reply_to_a_resolved_thread_stays_above_its_reopen_footer`
presses the summary without the wait too and is deliberately left alone:
its `resolve` is in the log before `open_page`, so the thread is never
drawn open and `foldOut` returns with no fold to race.

## Testing

`uv run pytest tests/test_render.py -q --run-nightly` — 479 passed on
this branch; `pre-commit` clean on the changed file. The whole suite
(`uv run pytest tests --run-nightly`, 922 passed) is green at the
failing commit locally too, on the same 4-CPU / 8-worker shape CI uses,
so the window is narrow — see the note below on what is verified and
what is inferred.

<details><summary>Evidence, and what is inferred</summary>

**Verified.** The gap is real and measured at each site. Putting an
assertion's own bug back prints Playwright's re-ask trail at exactly the
point the old code pressed. In
`test_a_key_on_screen_is_a_key_that_works` (`to_have_count(3)`):

```
- waiting for locator(".lf-details .lf-thread")
   4 × locator resolved to 1 element
     - unexpected value "1"
  10 × locator resolved to 2 elements
     - unexpected value "2"
```

Four re-asks with one thread listed: when the loop's last `expect`
returned, the second thread was still folding, and the summary press
went out into that window. In `test_a_resolved_thread_can_be_reopened`
(`to_have_count(2)`), the same trail, one step earlier:

```
   4 × locator resolved to 0 elements
     - unexpected value "0"
  10 × locator resolved to 1 element
     - unexpected value "1"
```

Four re-asks with the disclosure holding nothing at all — the fold had
not finished when `round_trip` returned, so that press was the least
protected of the three.

The failing run's own call log agrees from the other side — the element
was visible-but-moving (`element is not stable`) for the first ~120ms
after the press.

**Inferred.** What turned "moving" into "invisible for 27s" is not
proven. A stale handle is ruled out: a replaced node makes Playwright
log `element was detached from the DOM, retrying` (checked against a
standalone probe), and the CI log carries no such line, so the same node
stayed connected and lost its box — i.e. the disclosure ended up closed
after a press that should have opened it. Nothing in the runtime ever
writes `open = false`, so this reads as the press being delivered twice
or lost against a moving target, which is the risk of pressing
mid-animation rather than a runtime fault.

**Not reproduced.** ~160 targeted repetitions of the sequence, including
CPU throttling at 4/6/10/20× via CDP and a full-machine load, all
passed. Recent main history shows a rotating cast of different browser
tests failing per run, the contention signature `running-tend` names —
so this fix removes a real window rather than one proven to be the whole
story of that run.

</details>

---
Automated fix for [failed
run](https://github.com/max-sixty/leaf/actions/runs/32540635010)

---------

Co-authored-by: leaf-agent <318509791+leaf-agent@users.noreply.github.com>
Co-authored-by: Maximilian Roos <m@maxroos.com>
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.

1 participant