feat(vibe-tests): ground the a11y score in runtime axe results (#4145)#4229
Open
AKnassa wants to merge 2 commits into
Open
feat(vibe-tests): ground the a11y score in runtime axe results (#4145)#4229AKnassa wants to merge 2 commits into
AKnassa wants to merge 2 commits into
Conversation
The accessibility dimension was a 5-rule static regex scan of consumer code that can only fire on raw-HTML footguns, so component-composed output scored ~100 by construction — it measured composition hygiene, not accessibility, and never moved during the APG hardening work. Implements proposals 1, 3, and 4 from the issue: - axe-previews.ts: headless-Chromium axe-core scan of built previews (light + dark), writing an axe-results.json sidecar per iteration — the same pattern as build-errors.json. Wired into vibe-screenshots.yml after the screenshot step; zero new dependencies (root already ships @axe-core/playwright + playwright). - universal-eval.ts folds sidecar violations into the score by impact (critical -15, serious -10, moderate -8, minor -3 per rule). Static rules axe verifies on the rendered DOM (image-alt, labels, button-name, heading-order) stop double-penalizing when runtime data exists; click-non-interactive stays static-only since React handlers are invisible to the DOM. No sidecar -> static-only, old iterations unaffected; the 'accessibility' key is unchanged (stored universal.json files key on it). - Honest labels: "A11y Hygiene (composition)" without runtime data, "Accessibility (runtime + hygiene)" with it — CLI tables, markdown compare footnote, and the report app. - A11yMetrics surfaces the ceiling: eligible-site counts per static rule, rules fired, runtime availability, axe stats. Zero eligible sites now reads as "100 by construction" instead of a perfect score. Proposal 2 (scoring the a11y-manifests diff) is documented as a non-goal: the astryx/baseline manifests use different component vocabularies and only 2 of 4 targets have one, so naive wiring would break the Fair Evaluators invariant. Verification: 32 new tests including a real-Chromium integration test (skipIf-guarded for runners without a browser); suppression set, unknown-impact default, and floor mutation-checked. Demo iteration: static-only scores two variants 100/100; with the axe pass the broken variant (aria-required-attr, color-contrast in both themes) drops to 75 while the accessible one stays 100. Pre-existing, untouched: analyzeMaintainability omits darkModeSupport from its metrics (strict tsc error; darkModeRate always computes 0). Closes facebook#4145
Edge-case pass over the axe fold-in, all found red-first: - A truncated or hand-edited axe-results.json entry (missing violations array, or a violation without a themes list) crashed evaluate() and took down the whole aggregate run. Malformed records now degrade to "no violations" / empty theme list. - A malformed previews/manifest.json entry (string instead of a target map) iterated as characters in enumeratePreviews and could leak the iteration directory itself as a preview path. Non-object entries are now skipped. Pins added while in there: clean runtime scans keep their metrics, explicitly-null axe results stay static-basis, rule-eligible sites that do not fire count as eligible only, missing previews directory returns an empty list, and the report app's hasRuntimeA11y/dimensionLabel handle old-format results with no a11y metrics. Part of facebook#4145
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
community
Contributor
PR Analysis ReportNo new or modified components detected. Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Generated by PR Enrichment workflow | View full report |
AKnassa
marked this pull request as ready for review
July 23, 2026 03:04
AKnassa
requested review from
cixzhang,
ejhammond and
imdreamrunner
as code owners
July 23, 2026 03:04
humbertovirtudes
left a comment
humbertovirtudes
left a comment
Contributor
There was a problem hiding this comment.
Really nice closing of the gap #4145 called out. A static scan of consumer code cannot see focus order, ARIA wiring, or contrast, so grounding the accessibility dimension in a real axe-core pass over the rendered DOM is the right instinct. A few things I appreciated on the read:
mergeAxeRunsunioning per-theme violations by rule id (max nodes, highest impact, themes attributed) is the correct shape, and taking the strictest passes / loosest incomplete across light and dark is a sensible conservative default.AXE_COVERED_STATIC_RULESkeeping a defect that axe already scores on the rendered DOM from being double-penalized by the static rule is exactly right. The note on whyclick-non-interactiveis deliberately NOT in that set (React attaches handlers synthetically, so there is no onClick in the DOM for axe to see) is a great comment to leave for the next reader.- Degrading a malformed or truncated sidecar to "no violations" instead of crashing the aggregate is the kind of robustness this harness needs.
- The honest
getA11yDimensionLabel("A11y Hygiene (composition)" when there is no runtime data) is a good call: it keeps the score from overclaiming.
One ask before this lands, and it is the same bar we hold other behavioral / agent-facing changes to: could you attach a before/after on a sample iteration or two showing how the accessibility scores actually move once axe grounds them? The penalty ladder (critical 15 / serious 10 / moderate 8 / minor 3) is reasonable on its face, but seeing it applied to real previews is what tells us the weights land where we want, and that a score that was previously 100 "by construction" now reflects reality.
Not blocking on my end, just want that evidence in the thread before we call the scoring calibrated. Solid contribution.
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.
Fixes #4145
The accessibility dimension was a static regex scan of consumer code, so component-composed output scores ~100 by construction. This adds a runtime basis and stops the score from overclaiming.
axe-previews.ts— axe-core over each built preview in headless Chromium (light + dark) →axe-results.jsonsidecar per iteration, same pattern asbuild-errors.json. Wired intovibe-screenshots.ymlafter screenshots; zero new deps (@axe-core/playwrightwas already a root devDep).universal-eval.tsfolds violations in by impact (critical −15 / serious −10 / moderate −8 / minor −3 per rule). The four static rules axe re-verifies on the rendered DOM (image-alt, labels, button-name, heading-order) stop double-penalizing;click-non-interactivestays static-only — React handlers are invisible to the DOM. No sidecar → static-only scoring as before; old iterations and the storedaccessibilitykey are untouched.A11yMetricsrecords eligible-site counts so a 100 earned on zero signal reads as "100 by construction".a11y-manifests/diff (proposal 2) — the manifests use different component vocabularies and only 2 of 4 targets have one, so naive wiring breaks the Fair Evaluators invariant. Documented as a non-goal in the README.Proof (demo iteration, two
html-target variants with identical static profiles):aria-required-attr+color-contrast)Testing: 49 new tests including a real-Chromium integration test (auto-skipped on browserless runners); scoring rules mutation-checked; malformed sidecar/manifest entries degrade instead of crashing the aggregate. Full suite 7260/7273 — the 13 are the known pre-existing
packages/cliload flakes, green in isolation.Pre-existing, noticed but untouched:
analyzeMaintainabilityomitsdarkModeSupportfrom its metrics (darkModeRatealways computes 0), and the baseline a11y manifest is hardcoded despite its header saying it fetches Radix docs.