Read the page-map chord's progress from its keycaps, not the key line's text - #157
Read the page-map chord's progress from its keycaps, not the key line's text#157leaf-agent wants to merge 2 commits into
Conversation
|
This PR and #156 are between them the whole of what is still red on What was measured, and where the remaining three sitReproduced on c0a5b65 with the 33 node ids under
The margin pair traces to aa41f7a "Keep chord hints stable through progress", which moved On the digest: #156's manifest records |
leaf-agent
left a comment
leaf-agent
left a comment
There was a problem hiding this comment.
The route reading itself checks out against the runtime: range(n) in address.js is n > 1 ? "1–n" : "1" over addressed(entry) (capped at MAX_NUMBERED_ADDRESSES = 9), so address_span derives the same string, and keyline.js builds [chord[0], ...completeRowSteps(row)] with progressStates(steps, chord.length) — exactly g m 1–n with the leader painted pressed. I ran both tests on this branch (2 passed), and the all-neutral mutation fails at At index 0 diff: 'pressed' != 'neutral', reproducing the PR body's table.
One finding, on the settle rather than the claim.
expect(keys.nth(pressed - 1)).to_have_attribute("data-lf-key-state", "pressed") is an auto-retrying assertion on the key line, which is the one place tests/ writes down a rule against. key_line() in tests/render_harness.py says why: "Read once and never retried, which is the point of it: a disclosure's word is either what the watch painted within the press or what the two-second heartbeat paints later, and an assertion that retries goes green on whichever tick lands inside its budget — reading a stale line as an eventually right one." So if the repaint that marks a step pressed stopped happening within the press and only arrived on the heartbeat, this gate still passes inside its five seconds. That is the axis the PR body argues on, and it is the one the retry gives away.
It also isn't the shape test_render_navigation.py uses at the equivalent site, which the body claims. That site presses g, calls key_line(page) once, and then reads both the steps and the states with plain assert ... == states over evaluate — expect(...) is reserved for to_have_count, the aria-label, and the word, none of which move. The read shape here does match it; the settle is the opposite of it.
The inline suggestion swaps to that form. It needs key_line added to the render_support import, which is outside the diff:
from render_support import (
...
key_line,
leaf_page,
...
)I ran the suggested form on this branch: 2 passed at -n4, and 5 consecutive -n0 runs green, so the non-retrying read is not buying a flake here.
|
The residue on What was measuredFailure sets compared by diffing Replay on
The ring failure is the one #155 names: |
|
Run 33346442884 (6d68a1b) reported five nightly failures and adds no new work: this PR's two margin cases, #156's catalog digest, and I also closed #159, the duplicate of this PR, in favour of this one — it was opened twelve minutes later, its checks were still running, and its own comment had already said only one should merge. Its |
The helper waited for the pressed face with an auto-retrying `expect(...).to_have_attribute` on the key line. `key_line()` in tests/render_harness.py writes down the rule against exactly that: a disclosure's word is either what the watch painted within the press or what the two-second heartbeat paints later, so a retried read of a value that moves goes green on whichever tick lands inside its budget. A repaint that stopped happening within the press would still have passed. Settle the way tests/test_render_navigation.py does at the equivalent site: take the repaint's own frame once with `key_line(page)`, then read the steps and their states with plain asserts over `evaluate_all`. `expect` is left only for the count and the row's word, neither of which the chord advances. `to_have_count(1)` keeps the locator honest now that no text assertion implies a single match.
|
This is the first full nightly run on That settles the "four, not three" reading above. Verified locally rather than inferred: on VerificationThe digest arithmetic checks out too: No fix PR from run 33347270702; the two open PRs are the whole of it. |
… is hidden (#160) ## Problem `ci` on [72be4be](https://github.com/max-sixty/leaf/actions/runs/33345068975) failed with 35 nightly tests red. Rerunning all 35 node ids against `c0a5b65` leaves four: the catalog digest (#156), the two page-map chord reads (#157), and this one, which nothing owns. ``` FAILED tests/test_render_controls.py::test_a_wide_banner_spends_status_copy_before_action_reach AssertionError: Locator expected to be hidden 6 × resolved to <button ... class="lf-btn lf-answer-all ...">✓ Accept all (3)</button> 3 × resolved to ... ✓ Accept all (2) 4 × resolved to ... ✓ Accept all (1) ``` The count is walking down and running out of budget. The blanket answer [decides its decisions one at a time](https://github.com/max-sixty/leaf/blob/8683b9b1d2b9d408506c9f88bf3baf5a4f84685c/skills/leaf/assets/runtime/decisions/view.js#L68-L71) — `await source[verb]?.()` per decision — so the press owes one round trip per decision, and the test released the held first answer straight into `expect(answer_all).to_be_hidden()`. That single 5s budget was covering three sequential server trips, not the repaint it is for. `tests/CLAUDE.md` names this exactly: "Letting `expect` absorb the page's next read hides which mechanism supplied the wait and spends its timeout budget on transport rather than on the assertion." Instrumenting the sequence on an idle runner gives `event → state → event → state → event`, 1.89s end to end. Three trips at ~0.6s each fit inside 5s on an idle machine and do not on a loaded one, which is why this is nightly-and-under-load only. `test_accept_all_decides_every_pending_suggestion` drives the same control and does not have the bug: it waits for each widget's own settle first, so its `to_be_hidden` only has to cover the last repaint, and [its comment says why](https://github.com/max-sixty/leaf/blob/8683b9b1d2b9d408506c9f88bf3baf5a4f84685c/tests/test_render_widgets.py#L1541-L1543). This site was the one that skipped the staging. ## Solution State the transport with `_until` on the `Traffic` counters — the same causal helper this test already uses eight lines above to hold the answer in the wire — then let `to_be_hidden` assert only the repaint it is about. The number of trips owed is read off the control's own face (`✓ Accept all (3)`) rather than written out, so the fixture's arithmetic stays in one place, and an `owed > 1` guard fails loudly if a fixture change ever leaves this test a single trip and stops it exercising the sequence at all. This is not a raised timeout. `_until` is bounded at 30s with the counters in its failure message, so a genuinely stuck answer now fails naming the missing evidence instead of pointing at a control that "never went". ## Testing Playwright driving real Chromium on this runner, at `8683b9b`: - `uv run pytest tests` — **740 passed, 6 skipped**. This is the gate `ci` runs on the PR; the test fixed here is nightly-only and not exercised by it. - `uv run pytest tests/test_render_controls.py --run-nightly -n4` — **64 passed**, three times consecutively on an otherwise-idle machine. - `ruff check` and `ruff format --check` on the changed file, at the `v0.16.1` `.pre-commit-config.yaml` pin — clean. <details><summary>Reproducing the failure, and the mutation checks</summary> The test passes 6/6 at `-n0` on `c0a5b65` unmodified, so `-n0` alone leaves it unclassified. Stalling the driver for 300ms between `held[0].continue_()` and the hide assertion reproduces the CI shape exactly on the unfixed test — the button walks `(3) → (2) → (1)` and the expect runs out: ``` 5 × ... ✓ Accept all (3) 5 × ... ✓ Accept all (2) 3 × ... ✓ Accept all (1) ``` With this change in place the same stall passes. Two mutations confirm the new wait bites rather than just returning: | mutation | result | | --- | --- | | `traffic.sends == owed` → `owed + 1` | fails: `the page never settled every one of the 3 answers the blanket press owed: the wait began on sends=1 acked=0 pending=1 asked=3 heard=3 and gave up on sends=3 acked=3 pending=0 asked=6 heard=6` — and this also confirms `owed` reads 3 and that the predicate's own terms are what carry it | | driver stall re-applied to the fixed test | passes in 8.55s | Timed trace of the released sequence on an idle runner, seconds from `continue_()`: ``` 0.38 FIN event 0.66 FIN state 1.17 FIN event 1.38 FIN state 1.81 FIN event → hidden at 1.89 ``` The other `.lf-answer-all` sites were checked for the same shape and do not have it: `test_render_gate.py` navigates away, `test_render_startup.py` puts `wait_for_function(BOTH_STAMPS)` between the release and the hide assertion and has one decision rather than three, and `test_render_projection.py` asserts the control is absent. </details> --- Automated fix for [failed run](https://github.com/max-sixty/leaf/actions/runs/33345068975) Co-authored-by: leaf-agent <318509791+leaf-agent@users.noreply.github.com>
2 participants
Problem
cion 1c7d033 failed with 31 nightly tests red. Rerunning all 31 node ids againstc0a5b65leaves these two, which nothing owns (the rest of the residue is owned: the catalog digest by #156, and, at the suite's own parallelism, the ring walk by #155) —Both press
g, assert the key line readsm page-map items, pressm, then assert it reads1–9 page-map items— reading the chord's progress out of the line's text.aa41f7a("Keep chord hints stable through progress") made that reading impossible on purpose: every destination now carries its complete route from the momentgarms the chord, and a press changes a keycap's face rather than adding a key. So the row readsgm1–9page-map itemsat both stages — the first assertion never matches, and the second would match beforemwas pressed at all.test_render_navigation.pywas updated for the new contract in that commit (the route table asserts exactly["g", "m", "1–3"]with["pressed", "neutral", "neutral"]aftergalone); these two sites were missed. They are nightly-only, so the gap surfaced onmainrather than on the PR.Solution
Read the route and its progress the way the same commit's own coverage does: locate the row by
data-lf-commands~="navigation.page-map-item", assert its keycaps spellgmand the span the capped list holds, and assert each cap'sdata-lf-key-state.address_spanderives that span from the item count usingaddress.js's own rule (1–n, or1for a single member) rather than writing1–9out, so the helper states the cap instead of restating one page's arithmetic.This is strictly more than the text match proved. The old assertion could not distinguish
gfromg m; the new one fails if the route loses a step, gains one, spells a step differently, or stops advancing its faces. Mutating the expected states to all-neutral fails it; mutating the span fails it.The steps and the states are both read once, on the repaint's own frame taken by
key_line(page), with plain asserts overevaluate_all— the settletest_render_navigation.pyuses at the equivalent site. A retryingexpectis whatkey_line()writes down a rule against, because a value that moves with the chord goes green on whichever tick lands inside the assertion's budget, so a repaint that arrived only on the two-second heartbeat would still pass.expectis left forto_have_count(1)and the row's word, neither of which the chord advances.Testing
On this branch at
136e068, Playwright driving real Chromium on this runner:uv run pytest tests/test_render_margin.py --run-nightly -n4— 22 passed. Both named tests fail onc0a5b65unmodified and pass here.-n0, three consecutive runs — green each time, so the non-retrying read is not buying a flake.uv run pytest tests— 740 passed, 6 skipped. This is the gateciruns on the PR; the two tests fixed here are nightly-only and are not exercised by it, so the local nightly runs above are the evidence for them.ruff checkandruff format --checkon the changed file — clean, at thev0.16.1.pre-commit-config.yamlpins.Mutation check, and why the third failure is not here
Mutating the expected states to
["neutral"] * 3fails the assertion atAt index 0 diff: 'pressed' != 'neutral', so the states claim bites. Instrumented states on the real page:['pressed', 'neutral', 'neutral']afterg,['pressed', 'pressed', 'neutral']afterm.The rest of the run's residue on
c0a5b65is owned elsewhere and does not overlap this PR's one file:test_site.py::test_the_public_catalog_is_a_visual_index_of_full_page_routes(tracked in #133) is #156's and touches onlydocs/, andtest_render_controls.py::test_every_ring_the_layer_draws_is_shown_whole_somewhere_in_the_corpus, which surfaces at the suite's own parallelism, is #155's.Automated fix for failed run