Repository navigation
ci(a11y): the touched check audits every changed story no other audit covers - #10081
Conversation
… covers The touched check audited changed stories only when no component owned them, so a story added to an untouched component's file escaped: #9992 added core-resizable--collapsing-scroll-panel-fixture to Resizable's stories while touching LayoutPanel, and the scrollable-region-focusable violation surfaced only at main's release-check. The story audit now takes every story of a changed story file except those the touched components' audit already runs (--covered-components, the same filters), new violations only. A Lab-only pull request keeps its own lane (FR26). AST-030 FR12 says so.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
|
/approve-spec 0dd5900 |
PR Analysis Report🔎 Preview ·
|
| Before | After | Change | |
|---|---|---|---|
| Job-minutes per PR update | 81.4 | 80.7 | −0.7 |
| Runner pool, runner-minutes a day | 31,595 | 31,318 | −276 (−0.9%) |
| Average concurrent jobs | 21.9 | 21.7 | −0.2 |
| Time to green (median → this run) | 26.8 min | 15.3 min | −11.5 min |
| Runs a day | 388 | 388 | 0 |
Changed jobs: Touched components changed: standard
Not measured (the jobs that use these keep their recent minutes above): .github/scripts/accessibility-audit.js, .github/scripts/lib/a11y-changed-stories.js, .github/scripts/lib/a11y-changed-stories.test.mjs, .github/scripts/touched-components.mjs, .github/scripts/touched-components.test.mjs.
Assumptions: before is each job's average over the last 20 pull-request runs; after keeps that for unchanged jobs and uses this run's minutes for added or changed ones, weighted by how often each changed job ran in those runs; runs a day averages the seven whole days before today; time to green compares this run with the median of those runs that ran the same jobs.
Generated by PR Enrichment workflow | View full report
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved pending CI: the required checks are still running on this head.
The story audit now covers changed stories that an untouched component owns, and the spec:AST-030 FR12 change has an engineering owner's approval for this head. Two optional follow-ups:
- In the FR12 verification row, the "Mutation or failure expectation" column still ends with "a new violation in a changed story no component owns passes the touched check"; "no other audit covers" would match the new clause.
TOUCHED_AUDITin the story-audit step copies the touched audit step'sif:by hand; a contract-test assertion that the two match would stop a later edit to one from silently leaving stories unaudited.
[Automated review]
| | FR11 | surface owner, internal-job, and required-join contract tests | Build package browser behavior; CLI; theme-family broad fallback; mixed Core+CLI | a second owner is routed for one surface, an internal job masquerades as a lane, an owned failure misses its join, or an unrelated owner runs | | ||
| | FR12 | Storybook visual-plan, canonical workflow, and repository-owner guard tests | PR scope; full-plan maintenance; report-only publication; deployment/a11y/RTL/vibe artifacts | another workflow captures or compares pixels, a renamed or aliased owner escapes the guard, or legitimate non-visual artifacts are rejected | | ||
| | FR12 | `touched-components.test.mjs` changed-spec routing; `a11y-pr-contracts.test.mjs` `changedOnly` cases; `a11y-changed-stories.test.mjs` and the touched job's story-audit contract | a pull request that adds a browser accessibility spec; one that changes an `always` spec; one that changes a `release-only` or an `audit` spec; one that changes more spec files than the limit; a component change with no spec change; a pull request that adds an `a11y/` story no component owns | a pull request that adds a browser accessibility spec that fails is not caught on that pull request, another component's spec or an unchanged `audit` spec runs in the contract job, a not-run file reads as passed or fails the join, a failing contract leaves the `test` join green, or a new violation in a changed story no component owns passes the touched check | | ||
| | FR12 | `touched-components.test.mjs` changed-spec routing; `a11y-pr-contracts.test.mjs` `changedOnly` cases; `a11y-changed-stories.test.mjs` and the touched job's story-audit contract | a pull request that adds a browser accessibility spec; one that changes an `always` spec; one that changes a `release-only` or an `audit` spec; one that changes more spec files than the limit; a component change with no spec change; a pull request that adds an `a11y/` story no component owns; one that adds a story to an untouched component's story file beside a touched component | a pull request that adds a browser accessibility spec that fails is not caught on that pull request, another component's spec or an unchanged `audit` spec runs in the contract job, a not-run file reads as passed or fails the join, a failing contract leaves the `test` join green, or a new violation in a changed story no component owns passes the touched check | |
There was a problem hiding this comment.
Optional: the "Mutation or failure expectation" column still ends with "a new violation in a changed story no component owns passes the touched check". "…a changed story no other audit covers…" would match the new FR12 clause and the case this row now names.
| if: always() && steps.playwright.outcome == 'success' && steps.touched.outputs.changed_stories == 'true' | ||
| env: | ||
| FILES_PATH: ${{ steps.scope.outputs.changed_files_path }} | ||
| TOUCHED_AUDIT: ${{ steps.touched.outputs.has_components == 'true' && steps.touched.outputs.a11y_owners != '' }} |
There was a problem hiding this comment.
Optional: this copies the touched audit step's if: by hand. Asserting in touched-components.test.mjs that the two expressions are equal would stop a later edit to one from silently leaving stories unaudited.
Maintainer impact
A pull request that adds or changes a story owned by a component it does not touch has that story audited by nothing until
main's release-check. #9992 addedcore-resizable--collapsing-scroll-panel-fixtureto Resizable's stories while touching LayoutPanel. Its new [serious]scrollable-region-focusableviolation surfaced only atmain's release-check, where it turns the release gate red for everyone. The touched audit runs LayoutPanel's stories, and #10026's story audit covered only stories no component owns.Intended invariant
Every story file a pull request adds or changes is audited for accessibility, new violations only:
A Lab-only pull request keeps its own lane (FR26).
Change
.github/scripts/lib/a11y-changed-stories.js:changedUncoveredStoryIds(stories, changedFiles, coveredIds)replaceschangedUnownedStoryIds. It returns every story of a changed story file except those another audit runs.accessibility-audit.js: a new--covered-components <list>works only with--changed-story-files. The audit leaves out the stories those component filters route to, the same routes the touched audit uses.ci.yml: the story audit step (now "Accessibility audit for changed stories no other audit covers") passes--covered-componentswith the touched audit's own filters froma11y-pr-scope.mjs, when the touched audit runs. The step's condition, baseline and--fail-on-neware unchanged.Evidence
a11y-changed-stories.test.mjs):a11y/stories are still selected;core-resizable--collapsing-scroll-panel-fixtureis selected.main's selection code, 4 of the 7 selection cases fail.touched-components.test.mjs) pins the step's--covered-componentsand that it uses the same filters as the touched audit.a11y-changed-stories,touched-components,a11y-pr-contractsandworkflow-pathstests pass (141), andpnpm check:knowledgepasses.Writing rubric results
AST-030 FR12 (one clause changed) and its verification row:
Scope