fix(gate-45): a reduced-motion block that exists is not a fallback — judge every motion selector under the cascade - #788
Conversation
…judge every motion selector under the cascade Gate-45 asked one question per file: is there a `@media (prefers-reduced-motion …)` block in it? thematiq css/systems/nldesign/theme.css had one, so the gate said PASS at 6dcbbaf9 — where c495c99c had added `:not(.action-button)` to four `!important` motion selectors while the reset kept the bare `.button-vue, button, .button`. Equal specificity before (reset wins on source order), strictly less after (reset loses on every button in the app). PASS again at 67caeb85, once the selectors were mirrored back. Same verdict on the broken and the fixed file. The checker now flattens the stylesheet into rules, resolves nesting (`&`), and requires every selector carrying motion to be overridden by a rule inside a reduced-motion block under the cascade's actual rules: equal importance → more specific wins, then source order; unequal → `!important` wins. The repo-wide universal reset the pre-pass detects is therefore a `*` guard fed into the same check instead of an early exit: it covers every plain motion declaration and no `!important` one, which is what thematiq's own comment above its reset block says. Recognised without a matching selector: `no-preference` motion, and a duration token zeroed inside the reduced-motion block (the atom-design convention). Brace-less .sass falls back to the presence question. Measured with the package runner at full scope: theme.css @ 6dcbbaf9 FAIL — 3 finding(s), naming exactly the three narrowed selectors theme.css @ 67caeb85 PASS thematiq @ 7466a5a, openregister @ cabd4106, portaliq @ aa6dc05 PASS (no fleet noise) test_gate_45_selector_parity.sh pins the thematiq shape as the failing arm and every correct idiom as a control (mirrored selectors, `!important` guard over plain motion, `*` over plain vs `!important` motion, `no-preference`, duration token, `&`-nesting, `:is(.a, .b)`, `html`-prefixed guard, the repo-wide reset in another file). test_gate_45_stylesheet_scope.sh, test_gate_a11y_markup_scope.sh, test_gate_45_to_55_acceptance.sh and tests/test-hydra-gates-bin.sh stay green. Co-Authored-By: Claude Fable 5.1 <[email protected]>
WilcoLouwerse
left a comment
There was a problem hiding this comment.
Inline findings for the Thorough review of #788 — verdict follows in a separate submission.
WilcoLouwerse
left a comment
There was a problem hiding this comment.
Verdict: APPROVE (Thorough) — self-review posted as COMMENT (GitHub blocks self-APPROVE)
No 🔴. Six non-blocking findings posted inline: three 🟡 on the cascade model, three 🟢.
What I verified rather than took on trust — every claim below came from running the runner at a8c8ba9c and at a8c8ba9c^ against the same fixture, one variable at a time:
- The new suite is 24/24 green locally and
PASS — test_gate_45_selector_parity.shappears in thehydra-gates packageCI log, withALL discovered helper suites PASSED.run-helper-suites.shpicks it up throughfind "${LIB}" -maxdepth 1 -type f -name 'test_*', so "discovered automatically" holds in CI, not only on the authoring machine. test_gate_45_stylesheet_scope.sh13/13 andtest_gate_a11y_markup_scope.shall green — no regression from the rewrite.- The body's "Not run: shellcheck" caveat is closed: the ShellCheck workflow ran on this PR and passed.
- The one red at this SHA —
quality.ymlon thepushevent, 0 jobs — is a repo-wide startup failure that fails identically onmainand on every recent branch. Not this PR's, and not a reason to hold it. - The change earns more than the fixture it was built from. A reduced-motion block that names the selector but declares no motion property (
@media(reduce){ .btn { color: red } }) now FAILs where the pre-PR checker passed — a true positive the PR body doesn't claim.
Why the 🟡s don't block. All three are misses or noise on idioms the fleet does not currently write, and the fleet dry run's zero-new-findings result reproduces. The one I'd most like addressed is the first — an ancestor-scoped guard like .app-wrapper .btn over .btn goes PASS → FAIL across this PR while winning the cascade outright, and :root .btn on the identical fixture passes. Since gate 45 has no exclude hatch, an app that writes that idiom has no appeal. It is defensible conservatism (html/body/:root always match, an arbitrary class ancestor may not) — it just isn't written down as a choice.
The second 🟡 is a one-line fix in new code (gi is derived per rule, not per declaration). The third is the comment at :7741 asserting a source-order property that _covered never checks — the same "assert instead of observe" shape this PR exists to remove, one level down.
None of that outweighs the change. The gate now observes the property it is named after, the controls are pinned before the behaviour widened rather than after, and the anti-widening measurement is real. 👍
Round two of #788, all six from the review of the first round. Each one is a case where the checker that was written to stop asserting a property and start observing it was still asserting one. - Source order is carried through `_rules` instead of assumed. The comment said "the guard is later in the file, so equal specificity is enough"; `_covered` compared no positions, so a reduced-motion block placed ABOVE its motion at equal specificity and importance counted as an override. It does not — the later declaration wins, which is the thematiq#604 shape produced by ordering rather than by specificity. - An ancestor-scoped guard is a guard whatever the ancestor is named. `html`/`body`/`:root` were the only ones accepted, so `:root .btn` and `.app-wrapper .btn` — the same shape, the same specificity — disagreed, and the second was reported although it wins the cascade outright. `_descends` replaces the four special cases with one combinator-boundary test; `.xbtn` does not end on a boundary and stays unrelated to `.btn`. This credits a guard whose ancestor the checker cannot prove every element sits under. That is the deliberate trade: gate-45 has no `exclude` hatch, so a false positive on a working guard is unappealable. - Importance is read per declaration, and a guard covers only its own property family. `gi = '!important' in decls.lower()` was per RULE, so a block pairing a plain `transition: none` with an `animation: none !important` lent the transition an importance it did not have — and an animation guard was credited with stopping a transition it cannot touch. - A duration token counts as an override only when it is redefined to a duration that does not animate. Only the property NAME was read, so `--dur: 250ms` inside a reduced-motion block silenced its users. `0.01ms`, the universal reset's deliberate hair over zero, still counts. - A literal duration beside a tokenised one is not covered by the token: `transition: opacity var(--dur) ease, transform 0.4s ease` still animates the transform however `--dur` is redefined. - Guards are file-local and that is now written down, next to the universal reset that is the one thing crossing files. Behaviour is unchanged; a reader told the question is asked per selector will otherwise assume the selector is looked up repo-wide. The suite grows from 24 assertions to 35: P12/P12b, P13, P14/P14b, P15, P16/P16b, P17. Each new arm was mutation-tested against the runner at a8c8ba9 — exactly P12, P13, P14, P15, P16 and P17 go red there and every pre-existing arm stays green, so each control discriminates rather than decorating. test_gate_45_stylesheet_scope.sh 13/13 and test_gate_a11y_markup_scope.sh stay green. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
WilcoLouwerse
left a comment
There was a problem hiding this comment.
Verdict: APPROVE (Quick, re-review) — self-review posted as COMMENT (GitHub blocks self-APPROVE)
All six findings from the first round are addressed in 0c1c449a and their threads are resolved. No new findings in the delta.
Each fix is verified twice, because a gate fix that only makes its own new test pass is the failure mode this gate exists to catch:
- the fixture that misbehaved now matches the cascade — measured through the runner, not reasoned about; and
- a new suite arm discriminates: run the extended suite against
a8c8ba9cand exactly P12, P13, P14, P15, P16 and P17 go red while every pre-existing arm stays green. The controls that should not discriminate — P12b, P14b, P16b — are green in both directions.
| finding | fix | control |
|---|---|---|
| ancestor-scoped guard reported although it wins | _descends, one combinator-boundary test replacing four hardcoded prefixes |
P12 / P12b |
!important read per rule |
per-declaration importance and property-family matching | P13 / P17 |
| source order assumed | brace offset carried through _rules, applied where specificity ties |
P14 / P14b |
| literal duration beside a zeroed token | _nonzero_time on the motion value |
P15 |
| token redefinition treated as zeroing | _zeroish, 0.01ms still counting |
P16 / P16b |
| file-local guards undocumented | stated in the comment block; behaviour unchanged | — |
One of these needed more than the thread asked for. Finding 2's suggested one-line fix — read !important per declaration — still passed its fixture, because the !important sat on animation while the motion was a transition and nothing matched the two families. It closes only with family matching as well. Recording that here rather than quietly widening the fix.
Green: the new suite is 24 → 35 assertions; test_gate_45_stylesheet_scope.sh 13/13 and test_gate_a11y_markup_scope.sh unchanged; the full local run-helper-suites.sh reports 124 passed, 0 failed (1 pre-existing documented quarantine); CI on 0c1c449a is green on all four pull_request workflows including hydra-gates package and ShellCheck. The quality.yml push run is the repo-wide startup failure that fails identically on main — not this PR's.
Ready to merge as far as I'm concerned; I won't merge my own PR.
The defect
Gate-45 (
prefers-reduced-motion) asked one question per file: is there a@media (prefers-reduced-motion …)block in it? thematiq'scss/systems/nldesign/theme.csshad one, so the gate said PASS at6dcbbaf9— wherec495c99chad added:not(.action-button)to four!importantmotion selectors while the reset block kept the bare.button-vue, button, .button. Equal specificity before, so the reset won on source order; strictly less specific after, so it lost the cascade on every button in the app — with the block that was supposed to stop the motion sitting right there in the file, under a comment that says "Keep this block in step with the motion declarations above."The gate said PASS again at
67caeb85, after the selectors were mirrored back.Measured 2026-09-21 with the package runner at full scope, one variable:
theme.cssat6dcbbaf9(reset no longer names the motion selectors).button-vue:not(.action-button),.button-vue:not(.action-button) .button-vue__text,button:not(.action-button)67caeb85(reset names them again)Same verdict on the broken and the fixed file: the gate was not observing the property it is named after. It surfaced as a 🔴 in a human review three rounds in (thematiq#604), which is the wrong place for a mechanically decidable fact.
The change
The checker now flattens the stylesheet into rules (resolving SCSS / CSS-Nesting
&against the parent, skipping@keyframesbodies, keeping the comma inside:is(.a, .b)), and requires every selector carrying motion to be overridden by some rule inside a reduced-motion block under the cascade's actual rules:!importantwins regardless of specificity.Mvs guardGG == MMis!importantandGis notGextendsM(.btn:not(.x)over.btn,html .btnover.btn)Gis more specificMextendsG(.btn:hoverover.btn)Gis!importantandMis notGis*Gis!importantandMis not —*has specificity 0That last row is the one that changes the meaning of the repo-wide universal reset the pre-pass detects: it is now a
*guard fed into the same check instead of an early exit. It still silences every plain motion declaration in the repo (ARM 6 of the scope suite is unchanged) and silences no!importantone — which is exactly what thematiq's own comment above its reset says, and why that file repeats its selectors verbatim instead of writing*.Recognised as correct without a matching selector:
@media (prefers-reduced-motion: no-preference) { …motion… }(the motion itself is conditional), and a duration token —transition: opacity var(--dur)with--dur: 0msredefined inside the reduced-motion block (the atom-design convention). Brace-less.sassfalls back to the presence question.The finding names the selector and, for
!importantmotion, says what the fallback needs:Anti-widening — measured, not asserted
A per-selector question is exactly the kind of change that turns a gate into a noise generator, so the controls are pinned first and the fleet was run second.
scripts/lib/test_gate_45_selector_parity.sh(new, discovered byrun-helper-suites.shautomatically) — 24 assertions: the thematiq shape as the failing arm (the finding must name the three narrowed selectors and must not name.button, which the reset still covers), and a control for every correct idiom: mirrored selectors, an!importantguard over a plain motion (and the plain-over-plain case that genuinely loses),*over plain vs!importantmotion,no-preference, a zeroed duration token,&-nesting (resolved selector is what gets reported),:is(.a, .b), anhtml-prefixed guard, and the repo-wide reset in another file (covers plain motion there, not!importantmotion — and the finding lands on the motion file, not the reset).Existing suites stay green:
test_gate_45_stylesheet_scope.sh13/13,test_gate_a11y_markup_scope.sh,test_gate_45_to_55_acceptance.shALL GREEN (with ajv resolvable),tests/test-hydra-gates-bin.sh70/70.Fleet dry run — the full runner at default (full) scope on every local ConductionNL app clone with a
css/:7466a5a(development, pre-c495c99c)cabd4106aa6dc05291273fe045e9048motion-without-reduced-motion-fallback, and the pre-PR checker reports the identical 14fa02ce95c7bd7b3cc224f89Across eight apps the new rule fires on nothing but the fixture it was built from. The reds on pipelinq / procest / openconnector / softwarecatalog are
<style>blocks with motion and no reduced-motion block at all — the question the gate already asked, on stale local clones (May 2026 for pipelinq and procest).Also in this PR
scripts/test-fixtures/gate-acceptance/COVERED-ELSEWHERE.md— the gate-45 row names the new suite and its planted/clean arms (the ledger that enumerates a gate's tests has to move with the tests, which is the same rule this gate now enforces on stylesheets)._fail 45message now says "N finding(s): motion with no reduced-motion fallback, or a motion selector the fallback does not override under the cascade" — it used to count "stylesheet(s)", which is no longer what a line in the log is.Not run: shellcheck (not installed on the machine that produced this; the bash side of the diff is one message string, the rest is inside the
PYRMheredoc).The hydra-side docs (
hydra-gate-prefers-reduced-motion/SKILL.md, which still said the gate reads.vueonly, and the gate table inhydra-gates/SKILL.md) are updated in ConductionNL/hydra#688 to match.🤖 Generated with Claude Code