Stop the render gate reporting a hidden note as words out of reach - #143
Conversation
`unreachableWords` walks every text node under a widget's `.lf-ui` and reports the ones no declaration claims. It excused `.lf-quiet` and nothing else, so the external-link note 26dbf42 added — a `[hidden]` aria-describedby target carrying "opens in a new tab" — was reported as a word the reader is shown and cannot select. `test_example_renders[corpus]` has failed on it in both schemes since that commit, and so would any authored page with a cross-origin link inside a widget, at `version check --render`. The note cannot use `.lf-quiet`: it is placed beside its link inside whatever root the link stands in, shadow trees included, and the clip is a document rule that no shadow tree adopts. `[hidden]` is the silence available in every root, and it is safe for this reading to trust because the browser drops the attribute when it reveals `hidden="until-found"` content — a word the reader finds is a word the check sees again. The two sibling gate readings that ask what a box shows already spell the same pair.
|
Reached the same fix independently from run 33329901507 ( One datum for the first of the two exceptions above. The second exception, |
leaf-agent
left a comment
leaf-agent
left a comment
There was a problem hiding this comment.
One finding, measured on this branch.
The excuse is broader than the note needs, and the breadth is what the check loses. [hidden] matches hidden="until-found" too, and that is what HIDDEN in runtime/widget-elements.js resolves to wherever onbeforematch exists (export const HIDDEN = "onbeforematch" in document.body ? "until-found" : "") — an inactive lf-tabs panel and a settled lf-options' cards both wear it. The gate takes one settled reading per scheme — _read_scheme calls evaluate_probe(page, "unreachableWords") once — and never opens a tab, so the safety argument in the description, "the browser drops it on the reveal … a word this check sees again, at the moment it is on screen", has no second reading to land in. After this change an undeclared .lf-ui word inside a closed tab is silent at gate time and stays silent.
The repo already split this case the other way, for the reading of the same kind. render-checks/words.js, on silentWords: "a fault wherever the element stands, since a tab the reader has not opened is a tab they can open. Splitting them that way is what retired the [hidden] exemption this carried." The two siblings the new comment cites are the geometry readings — retiredSlots and coveredWords both measure rects and hold [hidden] out for a reason that is about rects, which words.js states next to that same passage: "the text in one reports the boxes it last laid out in". unreachableWords takes no rects. It asks who declared a word, which is silentWords' question rather than theirs.
Narrowing to plain hidden fixes the corpus and keeps the reach. Measured with the external-link note planted under .lf-ui in CARRIED_PAGE, once at each hidden value:
| excuse selector | note hidden |
note hidden="until-found" |
|---|---|---|
.lf-quiet (base, 5a52ad6) |
reported | reported |
.lf-quiet, [hidden] (this PR) |
silent | silent |
.lf-quiet, [hidden]:not([hidden="until-found"]) |
silent | reported |
Under the narrowed selector test_example_renders[corpus] passes, this PR's own test_render_reports_words_a_widget_puts_out_of_reach passes unchanged, and tests/test_render_gate.py --run-nightly is 64 passed / 3 failed — the same three the description already assigns to #121's -15.
Two things follow the suggestion if you take it. The paragraph above the line then says the wrong thing about until-found: the safety is that a plain hidden subtree is a collapse nothing offers to open, and hidden="until-found" is precisely the one this check keeps, because the reader can get there. And the new test is where that belongs — the same note at hidden="until-found" reported, beside the two legs already there.
How the table was measured
A throwaway module in tests/, run at -n0 --run-nightly, planting the note the runtime writes (presentation.js builds it as className: "lf-ui lf-external-note", hidden: true) into CARRIED_PAGE at each attribute value:
def _note(kind):
def go(page):
page.add_init_script(
"""addEventListener('DOMContentLoaded', () => {
const note = document.createElement('span');
note.className = 'lf-ui';
note.setAttribute('hidden', KIND);
note.textContent = 'opens in a new tab';
document.getElementById('c-lax').prepend(note);
}, {once: true});""".replace("KIND", repr(kind).replace("'", '"'))
)
return goOn 5a52ad6 both values report <lf-option id=c-lax> puts "opens in a new tab" under .lf-ui, where no comment can reach it in both schemes; on 03b4a0b both are []; with the suggested selector the until-found leg reports and the plain leg is []. prettier@3.9.6 --parser babel leaves the suggested line as written, single quotes included, at 79 columns.
| // again, at the moment it is on screen. The two sibling checks that ask what | ||
| // a box shows (render-checks/widgets.js, render-checks/standalone.js) spell | ||
| // the same pair. | ||
| if (el.closest(".lf-quiet, [hidden]")) continue; |
There was a problem hiding this comment.
[hidden] also matches hidden="until-found", which is what an inactive lf-tabs panel and a settled lf-options' cards wear — so this silences a declaration fault the gate has no second reading to catch later. Narrowing keeps the corpus green and keeps the reach:
| if (el.closest(".lf-quiet, [hidden]")) continue; | |
| if (el.closest('.lf-quiet, [hidden]:not([hidden="until-found"])')) continue; |
|
The suggestion in my review didn't land in the squash, so the finding now stands against Not opening the follow-up PR yet — five bot PRs are already open (#117, #134, #137, #140, #144), and this is a self-initiated fix rather than one anyone is waiting on. It is one line plus one test leg, ready when the queue drains or on your word: if (el.closest('.lf-quiet, [hidden]:not([hidden="until-found"])')) continue;Two things ride with it: the paragraph above that line then says the wrong thing about |
…p's own press (#140) ## Problem Two leftovers from [335bf9a](335bf9a) "Delegate page chrome to browser standards", which moved control activation off a page-wide `CONTROL` scope and into rows declared where each control is. Both are on `ci`'s red list for [the run this was dispatched for](https://github.com/max-sixty/leaf/actions/runs/33328596524) or reachable from the same change, and both still reproduce at `1ca4f40`. **`lf-worktree`'s head names fewer keys than work it.** The commit gave the head a row binding `PRESS` — Enter and Space — over a button wearing `aria-expanded`, which is ARIA's disclosure pattern. `skills/leaf/CLAUDE.md` states what that costs: "every row over a disclosure reads [`DISCLOSE`] — this scope's, and a widget's own row re-wording the same press. Two rows naming different sets is not two promises but one: `lineRows` prints the nearer row and drops the other whole, so a widget naming one key fewer takes the rest off the line". The widget row is nearer, so the arrow that opens the evidence went on working and stopped being named — on the key line and in `aria-keyshortcuts`, which is written once when the scope is declared and so froze at `Enter Space`. Nothing tested it; the identical bug in `lf-options` is what [#132](#132) is for, and that PR does not reach this package. **`test_a_widget_that_renames_its_role_keeps_the_press_offer_gave_it` still asks for the deleted scope's word.** It asserts the key line reads `press it` over a focused tab. `press it` was `CONTROL`'s `line`, and 335bf9a removed the scope in the same commit that gave `lf-tabs` its own `tab.activate` row reading `open the tab`. The commit updated several tests in that file and dropped `test_the_reference_names_the_space_that_works_a_control` outright; this assertion was missed, so the test fails before reaching the two facts it exists to hold. ## Solution `lf-worktree`'s head takes its keys from `DISCLOSE(head)`, the way `lf-diff` and `lf-draft` already do, and keeps its own `run`. `DISCLOSE` hands over only the arrow that changes the state, so the press stays a direction rather than a second toggle; it also answers for where the head stands, and the runtime's disclosure scope stops at the chrome. The head is a span from `selectableOffer`, so a copy frozen into thread markup has no platform pair underneath it the way a `details > summary` does — the row's own press is the only thing there, which is the shape [#132](#132) keeps for the sibling widget. Its `line` reads the way the tree is standing, and `aria-expanded` is written at birth so the head is never briefly a control the runtime cannot place — without that write `DISCLOSE` hands back both arrows, and that is the value `aria-keyshortcuts` is written with when the scope is declared. The repaint that keeps the line and `aria-keyshortcuts` together is the document's disclosure watch, which [#132](#132) moved from `paintHere()` to `paintKeys()` while this branch was open; the widget owes none of its own, which is what `skills/leaf/CLAUDE.md` now says in as many words. The tabs test asserts `open the tab` — the strip's own word for the same press — and its docstring now describes where that press is declared rather than the scope that used to supply it. Everything after the assertion is unchanged: Enter still has to re-select the focused tab, and Space still has to be consumed rather than scrolling the page out from under it. New coverage in `test_render_projection.py` for the worktree head, since nothing exercised it. It holds the row rather than the repaint: on the page, both surfaces naming the row's keys — the line and `aria-keyshortcuts` — plus a second `→` on an open tree, so a scope answering nothing at all could not pass it. In a message: the same head in the threads panel, where `aria-keyshortcuts` reads `Enter Space`, the arrow moves nothing, and Enter and Space fold it. The repaint behind both surfaces is held by `test_a_widgets_native_control_names_the_press_the_platform_makes`, which #132 added. ## Testing `uv run pytest tests` at `c292244` — 723 passed, 6 skipped. After merging `main` (which brought #132 and #143) and taking the repaint out at `890e7ff`, `test_render_projection.py`, `test_render_navigation.py`, and `test_render_anchors.py` — 193 passed, 4 failed, all four the version-menu group [#137](#137) owns and all four failing identically on `origin/main` in a clean worktree. With `--run-nightly`, `test_render_projection.py` and `test_render_navigation.py` — 137 passed, 1 failed, that one being `test_the_arrows_say_which_way_the_section_under_the_reader_goes`, which is #132's settled ask and untouched here; it fails the same way at the merge base `1ca4f40`. Each half of the new test was run against the code it holds and fails there: the page half at `assert 'Enter Space' == 'Enter Space ArrowRight'` on `main`'s `lf-worktree.js`, and the thread-markup half at `aria-expanded` staying `"false"` under Enter with the row's `run` removed. <details><summary>What this does not fix in that run</summary> The run reported 34 failures. Most of the rest already have a PR open: the `-15` root scrollbar-gutter family is [#121](#121) (`assert -15 == 0`, the drawn edge landing a bar's width off the hand, the covering panel's blank strip), the version-menu group is [#137](#137), the settled ask's disclosure keys were [#132](#132), now merged, the margin and float placements are [#138](#138), the gallery stills are [#134](#134), and the sidebar's stated window is [#136](#136). No guard stops the third widget from declaring a fixed key pair over a disclosure. `DISCLOSE` exists so the two surfaces cannot come apart, but a widget has to remember to call it, and two of the four widgets over a disclosure did not. A corpus-wide check — every focusable disclosure names the keys `DISCLOSE` gives it — would catch the next one, and is left out of this PR because it belongs beside the corpus checks rather than in a fix for two widgets. </details> --- Automated fix for [failed run](https://github.com/max-sixty/leaf/actions/runs/33328596524) --------- Co-authored-by: leaf-agent <318509791+leaf-agent@users.noreply.github.com>
2 participants
Problem
test_render_gate.py::test_example_renders[corpus]has been red on everycirun since 26dbf42 "Add external link treatment", in both colour schemes, and it still reproduces deterministically on1ca4f40in 11 seconds. #132 diagnosed it as "a different finding, from 26dbf42's external-link treatment rather than from 335bf9a" and left it; no PR or issue owns it.The subject is not the example.
render_versionis the passversion check --renderruns on an agent-authored page, so any page with a cross-origin link inside a widget now fails handover on a word the reader is never shown.unreachableWordswalks every text node under a widget's.lf-uiand reports the ones no declaration claims. Its excuse list held.lf-quietand nothing else. 26dbf42'srenderExternalLinkswrites a note beside each external link —class="lf-ui lf-external-note",hidden, textContent "opens in a new tab" — as thearia-describedbytarget for the link, and the walk found it: notdata-lf-said, not inside the link's ownCONTROLbox (a describedby target is referenced by id, so it sits beside the link rather than in it), not.lf-quiet.Solution
Excuse
[hidden]beside.lf-quiet, one line and its comment. The check's own opening sentence is "Words the page shows that no user can select", and a[hidden]subtree shows none.The note cannot simply become a
.lf-quietword instead — which would have been the tidier answer, sincequietWordis the repo's one idiom for "a word for a reader listening, silent on screen".renderExternalLinksis enrolled on declared shadow roots as well as the document, and the note is placed inside whatever root its link stands in, while.lf-quiet's clip is a rule in the document stylesheet that no shadow tree adopts (onlyMARK_RULESis adopted, inshadow.js). A.lf-quietnote in anx-shadowwidget would render as visible text next to the link.hiddenis the silence available in every root.[hidden]is safe for this reading to trust rather than a hole for hidden-but-revealable words: the browser removes the attribute when it revealshidden="until-found"content, so a word the reader finds is a word this check sees again, at the moment it is on screen. The two sibling gate readings that ask what a box shows already spell the same pair —widgets.jsandstandalone.js.No assertion is loosened and no runtime file is touched.
skills/leaf/CLAUDE.md's row for this gate already reads "visible page words remain in reachable flow".Testing
test_example_renders[corpus]andtest_example_renders[pr-walkthrough]— pass with--run-nightly; the first fails onorigin/mainunchanged at the two findings above.test_render_reports_words_a_widget_puts_out_of_reachgains the pair: the same.lf-uinote insidelf-optionreports nothing whenhiddenand is still reported when shown. Bug-back: with the[hidden]excuse reverted, the hidden half fails with exactly the corpus's finding.uv run pytest tests— 723 passed, 6 skipped, on this branch rebased ontoc7b3706.uv run pytest tests/test_render_commands.py tests/test_render_gate.py --run-nightly— 77 passed, 4 failed, and all four are in the failed run's own list and owned elsewhere:test_the_render_gate_measures_sideways_room_at_the_root_scrollport,test_the_reader_draws_an_edge_to_the_width_they_want[comments]andtest_a_window_with_no_room_for_a_chosen_width_does_not_un_choose_it[comments]are Keep the root's bar standing rather than reserving a gutter it never draws in #121's-15, andtest_a_reader_arrives_at_what_they_left_rather_than_watching_it_arriveis the intermittent one Keep the root's bar standing rather than reserving a gutter it never draws in #121 documents.node --check, pinnedprettier --check,ruff checkandruff format --checkpass on the two touched files.pre-commitis not on this sandbox's PATH, so its remaining hooks went unrun here.The preview digest is unmoved:
capture_input_files()hashesexamples/,skills/leaf/assets, the selected packages and three site scripts, andskills/leaf/scripts/is in none of them.What this does not fix
The failed run had 32 red. Everything else in it sits with an open PR or issue — #121, #132, #134, #136, #137, #138, #140, #141, and issues #133 and #139 — with two exceptions I did not reach:
test_render_controls.py::test_every_ring_the_layer_draws_is_shown_whole_somewhere_in_the_corpusreports one unlit ring,version-diff (.lf-version-diff:is(:focus-visible, .lf-focus-visible)). That control is inside the versions menu Give the versions menu back its way in, its way out, and the reader's place #137 is rewriting, so it is likely downstream of that branch rather than its own cause; I have not verified it against Give the versions menu back its way in, its way out, and the reader's place #137's tree.test_render_navigation.py::test_a_widget_that_renames_its_role_keeps_the_press_offer_gave_itis the generic-press question Read the settled ask's disclosure keys from DISCLOSE, and repaint every disclosure row's shortcuts #132 raised and deliberately left as a product decision.Automated fix for failed run