Restore native copy for passage selections - #193
Merged
Conversation
Keep the browser selection active when Leaf offers its response field. Focus the field only after an explicit Comment action, and cover native copy and context-menu behavior in the browser suite.
leaf-agent
reviewed
Sep 1, 2026
leaf-agent
left a comment
leaf-agent
left a comment
Collaborator
There was a problem hiding this comment.
Two things, both narrow.
The new negative focus assertions mostly can't fail. tests/CLAUDE.md names this exact trap under A state the page passes through is not a state to poll for: "A retrying assertion that a paint has not happened is the same trap wearing the other sign, and it is worse... A positive assertion polls until the frame arrives; a negative one is satisfied by the first poll, and the first poll is before the frame." It even names the path this PR is about — "the runtime raises chrome from deferred steps — updateFab runs inside the mouseup handler's setTimeout". .lf-fab-input is a permanent node (leaf.js builds it once and only toggles style.display), so expect(...).not_to_be_focused() resolves and passes on the first poll, before the queued updateFab has decided anything. That leaves this PR's central claim — a passage selection does not take the focus — asserted by a check that would stay green with the old unconditional composerInput.focus() still in place.
test_render_reactions.py gets it right in this same PR: expect(bar).to_be_visible() and then the negative. The sites with no positive fact in front of the negative are tests/render_cases_navigation.py:797 (the compose helper, so every case through it), tests/test_render_navigation.py:3892 and :4014, tests/test_render_semantic_selection.py:173, tests/test_render_drafts.py:978/:982/:993, tests/test_render_options.py:1122, tests/test_render_projection.py:218/:343, tests/test_render_startup.py:2665, and tests/test_render_widgets.py:2350. tests/test_render_options_settled.py:118 and tests/test_render_outbox.py:2224 do add the positive fact, just after the negative rather than before it — swapping the two lines settles those.
Six of them are then followed straight away by page.keyboard.press("c") (including the two bare presses at test_render_navigation.py:4047 and :4101). There the vacuous read has a second cost: with the bar not yet up, commentDestination() finds no fabAnchorAt() and routes to "Go to the threads", so the to_be_focused behind it goes red rather than vacuously green. The Enter-search sites in test_render_semantic_selection.py are fine as they stand — chooseMatch sets the selection and calls updateFab() synchronously inside the keydown, so nothing is deferred there.
One prose line left behind. skills/leaf/CLAUDE.md still says showComposer "states the whole visible and focus outcome" two lines below the sentence this PR rewrote, but the focus outcome is now openComposer's focus option and is derivable from none of composerOpen, pendingAnchor, or fabAnchor.
The runtime change itself reads clean to me: the fabInputTakingFocus = options.focus !== false handoff keeps the selectionchange guard honest on the explicit path, focusFabComment still pre-sets it before the collapse, and routing the composer's Escape row through dismissFab is what stops the now-standing selection re-raising the bar on the Escape keyup.
Wait for a positive composer or quote transition before asserting that automatic passage selection leaves focus alone. Correct the internal ownership note for composer visibility and focus.
leaf-agent
approved these changes
Sep 1, 2026
…copy # Conflicts: # tests/test_render_anchors.py
max-sixty
added a commit
that referenced
this pull request
Sep 1, 2026
#193 kept the response field passive so a drag leaves the browser's own selection alone and a native copy still has something to take. It aligned the render tests with that contract; test_site.py is nightly-only, so its two readings went unread and stayed on the old one. The label test turned on `document.activeElement === field`, which is now false for the page's own words and the site's label alike — the discriminator it rests on was gone, not merely inverted. It reads whether the field was offered at all, which is the difference it was always after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0149R6uAvYxfu8xSjMrbmj2z
max-sixty
added a commit
that referenced
this pull request
Sep 1, 2026
## Problem [`ci` run 33516407327](https://github.com/max-sixty/leaf/actions/runs/33516407327) came back **10 failed, 1614 passed, 6 skipped**. Eight belong to #189, whose `ci` run was cancelled by #190's push before the nightly leg ever ran on it. #188's `test_mcp_app.py` case arrived on the same collision as the MCP test below and is carried here rather than left to red the next nightly. The everyday suite that gates a pull request does not run `test_render_*` or `test_site.py`, so a PR can land its nightly half unread — which is how these arrived, and how five more have arrived since, from #193, #194 and #200. `main` has moved a long way under this branch; the ones it now owns are taken on merge, and three of the new ones are cleared here. #203 reached `tests/conftest.py`'s `headless_shell` first, so the four `test_render_commands.py` setup errors are its. ## Solution Grouped by cause rather than by test, since several tests share one. **The example the tests still name.** #189 rewrote `examples/pr-walkthrough.html` into the Worktrunk review packet, and the shipped patch is now a collapsed manifest that builds no lines until a reader opens a file, which the scroller gate read as `getComputedStyle(null)`. The gate reads the diffs that have drawn lines, with its own non-vacuity floor kept. **The new package's chrome and surfaces.** `.lf-pr-description` stacked a second translucent accent onto a card that is already a tint of one, and the layer's own `--muted` reads 4.32:1 there; the description now sits on the raised-surface token. CallDiff's disclosure was a bare `<button>` rather than an `offer`, so an exported copy kept a hand over a press nothing can take. Its location anchor carried an `href` on the header row, which names no location at all — and `reachScrollers` reads a candidate for a focusable descendant before granting it a stop, so that hidden anchor answered "there is already a way in here" with a link nobody can reach, for a box whose words run off the side. **A reading that counted clipped text as painted.** `coveredWords` measured each run's whole rect, so an ellipsised name in a narrow column read as covering its neighbour. It now intersects each run with the boxes that clip it, stopping at an out-of-flow ancestor where a hidden overflow further out need not reach. The one test that plants this fault plants it properly now: the runtime's note is parked in a one-pixel box with hidden overflow, so opacity alone never put its characters on the screen. **Three focus rings the corpus stopped painting.** `code-pre-light` and `code-pre-shadow` are the tab stops on a scrolling code block and on a diff's lines, and with the old example gone no example held either. The page gets one rendered hunk and one code line long enough to scroll. Then `.lf-diff-review`: it took the outward ring it shared with `.lf-diff-next`, which stands on its own in the toolbar, while this one rides a file's summary row inside a box that clips at the row's bottom edge — its lower run fell 3px past what the sweep can see. It takes the inset the summary beside it already takes, and splitting the rule made each half declare the ring name neither had. **`--diffs-fg-number` on changed rows.** Surfaced by that rendered hunk: a changed row's number is drawn in the base its own fill is mixed from, green on green at 4.03:1. Pierre's number overrides now take the layer's tinted-surface inks. **A sidebar the page got shorter under.** #190's zero-height ToC anchor shortened the release page 58px, putting a named `scrollTo(0, 900)` past the stretch where the box stands on its own offset. Both edges of that stretch are the page's, so they are read off it, with a floor asserting the stretch exists before a point halfway along it says anything. **Playwright's loop against `asyncio.run` and `anyio.run`.** `sync_playwright()` keeps an asyncio loop running in its thread for its whole lifetime, and the `browser` fixture is session-scoped per xdist worker — so an MCP test's own loop start raises in any worker that has opened a browser and passes in one that has not, leaving the schedule to decide. Verified directly: `asyncio.events._get_running_loop()` is `None` before `sync_playwright()`, the loop inside it, `None` after. `interact_support.run_async` runs the entry point on a thread with no loop on it, and both MCP modules go through it — #204's three new exchanges included, since they landed on the direct call while this was in flight. **A declared verb no page replayed.** #194 added `lf-diff`'s `review` to the registry without an event for it on the standing-state page — the one fixture built so that exactly this fails rather than going unexercised. It gets a diff and a standing review. **A focus contract two site tests never heard about.** #193 kept the response field passive so a drag leaves the browser's own selection alone and a native copy still has something to take, and aligned the render tests with it. `test_site.py` is nightly-only. Its label test turned on `document.activeElement === field`, which is now false for the page's own words and the site's label alike — the discriminator was gone, not inverted — so it reads whether the field was offered at all, which is the difference it was always after. ## Testing `uv run pytest tests --run-nightly` and `pre-commit run --all-files` on this head. The everyday suite and lint are green on this branch in CI. Every claim about what `main` does was taken from a control run of the same tests in a clean checkout of `main`, not from reading. <details><summary>Where each failure landed</summary> | test | fix | | --- | --- | | `test_render_gate.py::test_the_runtime_holds_a_scroller_the_page_wrote` | reads the diffs that drew lines | | `test_render_export.py::test_an_exported_example_stands_on_its_own[pr-walkthrough]`, `[corpus]` | widget, probe, theme | | `test_render_controls.py::test_every_ring_the_layer_draws_is_shown_whole_somewhere_in_the_corpus` | example, then `.lf-diff-review`'s ring and both ring names | | `test_render_pages.py::test_a_left_sidebar_uses_the_margin_until_the_page_needs_it_back` | scroll position read off the page | | `test_interact_mcp.py::test_stdio_protocol_carries_the_app_resource_and_private_tool_result` | own thread | | `test_mcp_app.py::test_registered_server_prefers_full_page_and_keeps_snapshot_as_fallback` | own thread | | `test_render_projection.py::test_the_render_gate_applies_every_standing_action_a_second_time` | standing fixture (#194's) | | `test_site.py::test_the_label_is_chrome_rather_than_words_to_quote`, `::test_a_comment_lands_in_the_thread_with_its_quote` | the field is offered, not entered (#193's) | | `test_render_anchors.py::test_a_data_bound_diff_aims_and_selects_one_source_line` | **#194's and #193's, taken on merge** | | `test_site.py`, `test_render_export.py` stale `<h1>` | **#202's, taken on merge** | The two exported-copy cases each carried four faults, three of them masked: `offering` asserts before `covered`, which asserts before the copy's axe run, so each fix uncovered the next. The ring sweep did the same — the geometry fault stood in front of a naming check that had been silent about a rule missing its name since it was written. </details> <details><summary>Three this leaves, all of them main's</summary> **`test_render_controls.py::test_examples_have_no_serious_wcag_a_or_aa_violations[pr-walkthrough]` stays red.** The `.lf-pr-description` contrast above is fixed and confirmed gone. What holds the test red is 46 `nested-interactive` findings from #194: each file's "Mark reviewed" `<button>` sits inside that file's `<summary>`, and a disclosure with a focusable descendant is a serious WCAG failure. Clearing it means taking the press out of the disclosure and re-laying the row — `entry.node` is the `<details>` in ten places, and the theme, print and export rules key on it — which is a change to that widget's shape with visual verification of its own, not a line this branch should slip in. **`test_render_anchors.py::test_a_failed_fragment_hydration_waits_for_a_reader_retry` fails whenever anything runs before it.** Confirmed on unmodified `main`: three runs of the file at `-n4`, three failures, and the same failure at `-n0` over the whole file. It passes alone every time. `assert len(requests) == 2` sees one — the reopened disclosure's second fragment fetch never reaches the route. Arrived with #189. **`test_render_anchors.py::test_a_drag_released_mid_word_hugs_words_and_sentences` fails over the whole file and passes alone.** `assert 'inside' == 'inside it'` — the selection did not grow to the word boundary. It looks like the next instance of what #191 diagnosed, Chromium 151 collapsing a synthetic range on mouse release. Also still behind: the gallery's `docs/example-pr-walkthrough.jpg`. Regenerating here rewrites all nine stills in substituted faces, because `theme.css` asks for Charter and this runner's `fc-match` has none of the serif stack — the same reading #175 made. Left for the authoring machine. </details> --- Automated fix for [failed run](https://github.com/max-sixty/leaf/actions/runs/33516407327) > _This was written by Claude Code on behalf of @max-sixty_ --------- Co-authored-by: leaf-agent <318509791+leaf-agent@users.noreply.github.com> Co-authored-by: Maximilian Roos <m@maxroos.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Selecting a passage recently focused Leaf's response textarea and collapsed the browser selection. That made Command-C copy an empty field and routed right-clicks through Leaf instead of the selected passage.
Keep the response field passive until the reader explicitly chooses Comment, so the browser selection remains available for native copy and the context menu. This also aligns the surrounding selection, draft, and Escape coverage with the restored focus contract and records the standard-shortcut rule in the browser guidance.
Tests:
uv run pytest tests(779 passed)