From fb1d0e33d02f059f6ad34351b6754276e63e22c7 Mon Sep 17 00:00:00 2001 From: Cindy Zhang Date: Sat, 31 Oct 2026 11:09:36 -0700 Subject: [PATCH 1/7] ci: a default touched-components check runs visual, a11y and RTL for the components a pull request changes Every pull request without a ci:run- label runs one default check for only the components it touches, in every package, against its Storybook build. The visual part is pr-visual's new default mode, a focused check of the touched Core components that never escalates to the release plan, so pr-visual stays the one visual owner (AST-030 FR12). The accessibility and RTL parts run in pr-touched-components on one standard arm runner, which folds in pr-visual's verdict and writes touched-components.json: one row per component with each suite's pass/fail/changed/not-run result, or the wide flag and count. A wide change (more than five components, or shared styles: themes, tokens, shared .stylex.ts) skips it, and nothing starts a full suite on its own. AST-030 FR12 and DEC-7 record the check. --- .github/scripts/ci-heavy-lanes.test.mjs | 27 +- .github/scripts/ci-test-routing.test.mjs | 1 + .github/scripts/touched-components.mjs | 404 ++++++++++++++++++ .github/scripts/touched-components.test.mjs | 374 ++++++++++++++++ .../visual-maintenance-workflow.test.mjs | 6 +- .github/workflows/ci.yml | 213 ++++++++- CONTRIBUTING.md | 6 + docs/specs/AST-030/spec.md | 26 +- 8 files changed, 1047 insertions(+), 10 deletions(-) create mode 100644 .github/scripts/touched-components.mjs create mode 100644 .github/scripts/touched-components.test.mjs diff --git a/.github/scripts/ci-heavy-lanes.test.mjs b/.github/scripts/ci-heavy-lanes.test.mjs index 031b6bf7c3ce5..e29ac93bbcde6 100644 --- a/.github/scripts/ci-heavy-lanes.test.mjs +++ b/.github/scripts/ci-heavy-lanes.test.mjs @@ -26,7 +26,6 @@ const SUITE = { 'pr-a11y-specs': 'a11y', 'pr-a11y': 'a11y', 'visual-scout': 'visual', - 'pr-visual': 'visual', 'pr-rtl-shard': 'rtl', 'pr-rtl-merge': 'rtl', 'pr-rtl': 'rtl', @@ -57,6 +56,32 @@ describe('visual, accessibility and RTL owners', () => { }, ); + it('runs pr-visual on every pull request: its full check only with ci:run-visual, otherwise the default touched check', () => { + const visual = ci.jobs['pr-visual']; + const condition = ungated(visual.if); + expect(condition).toContain( + "(github.event_name == 'pull_request' &&\n needs.build-storybook.result == 'success')", + ); + expect(visual.needs).toContain('run-labels'); + const step = name => + visual.steps.find(candidate => candidate.name === name); + expect(step('Run the visual gate for the touched components').if).toContain( + "needs.run-labels.outputs.visual == 'true'", + ); + expect(step('Decide the default touched visual check').if).toContain( + "needs.run-labels.outputs.visual != 'true'", + ); + const touched = step('Visual check for the default touched components'); + expect(touched.if).toBe( + "steps.touched.outputs.has_components == 'true' && steps.touched.outputs.visual_components != ''", + ); + expect(touched.run).toContain('gate.mjs check'); + expect(touched.run).toContain('--tiers component'); + expect(touched.run).toContain('--no-scout'); + expect(touched.run).not.toContain('--dependents'); + expect(touched.run).not.toContain('gate.mjs release'); + }); + it('decides the suites in one gate job from the trusted decision script', () => { const gate = ci.jobs['run-labels']; expect(gate['runs-on']).toBe('ubuntu-slim'); diff --git a/.github/scripts/ci-test-routing.test.mjs b/.github/scripts/ci-test-routing.test.mjs index a8cacdfe436f1..2fa6d95fea313 100644 --- a/.github/scripts/ci-test-routing.test.mjs +++ b/.github/scripts/ci-test-routing.test.mjs @@ -428,6 +428,7 @@ describe('ci.yml RTL package sharding', () => { 'pr-rtl', 'pr-rtl-merge', 'pr-rtl-shard', + 'pr-touched-components', 'pr-visual', 'visual-scout', ].sort(), diff --git a/.github/scripts/touched-components.mjs b/.github/scripts/touched-components.mjs new file mode 100644 index 0000000000000..fb65cf9cba893 --- /dev/null +++ b/.github/scripts/touched-components.mjs @@ -0,0 +1,404 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file Touched-components check: scope and per-component results. + * @input The pull request's analysis.json, and the visual verdict, accessibility + * report and RTL reports a touched-components run wrote + * @output `scope`: GitHub step outputs naming what each suite audits. + * `report`: touched-components.json, one row per touched component with a + * pass/fail/changed/not-run result for each suite, for the pull-request + * comment; a step summary; exit 1 when a suite failed. + * @position Default pull-request check (spec:AST-030 FR12). It audits only the + * components a change touches and never widens to a full suite; full suites + * run behind their `ci:run-` labels and in the release check. + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import {pathToFileURL} from 'node:url'; +import {createRequire} from 'node:module'; + +const require = createRequire(import.meta.url); +const {diffAgainstBaseline} = require('./lib/a11y-baseline.js'); + +const SUITES = ['visual', 'a11y', 'rtl']; + +/** + * The most components the default check audits. Over the merged pull requests + * of the two weeks before this check, a pull request that touched components + * touched one at the median, two at the 90th percentile and three at the + * 95th; 2.1% touched more than five. A wider change is a wide change: the + * full suites behind the ci:run- labels are the check for it. + */ +export const MAX_TOUCHED_COMPONENTS = 5; + +/** + * Style-affecting foundations: a change here can move any component's pixels, + * so no touched set describes it. Behaviour-only changes in shared utilities, + * hooks, gestures or motion are not foundations. + */ +const FOUNDATION_PATTERNS = [ + /^packages\/themes\//, + /^packages\/[^/]+\/src\/theme\//, + /(^|\/)tokens\.stylex\.ts$/, + /^packages\/[^/]+\/src\/(utils|hooks|internal|theme|Layer)\/(.*\/)?[^/]+\.stylex\.ts$/, +]; + +/** The changed paths that are style-affecting foundations. */ +export function foundationChanges(files) { + return files.filter(file => + FOUNDATION_PATTERNS.some(pattern => pattern.test(file)), + ); +} + +/** + * Whether the default check runs: at most MAX_TOUCHED_COMPONENTS touched and + * no foundation changed. A wide change skips it; nothing starts a full suite + * on its own. + */ +export function decideWideChange(scope, files) { + const foundations = foundationChanges(files); + return { + componentCount: scope.touched.length, + foundations, + wideChange: + scope.touched.length > MAX_TOUCHED_COMPONENTS || foundations.length > 0, + }; +} + +/** `core/Button` → its package and name. */ +export function ownerIdentity(owner) { + const slash = owner.indexOf('/'); + const pkg = owner.slice(0, slash); + return { + key: owner, + package: `@astryxdesign/${pkg}`, + shortPackage: pkg, + name: owner.slice(slash + 1), + }; +} + +const strings = value => + Array.isArray(value) ? value.filter(item => typeof item === 'string') : []; + +/** + * What each suite audits for this pull request: the touched owners only. + * Reverse dependents stay with the opted-in owners (FR13): the analysis + * reports every component whose bundle reaches a change, which for a shared + * primitive is most of Core, so they would not fit this check's 15-minute limit. + * Nothing here selects a full suite either: a change the analysis cannot + * scope audits its touched owners, and the comment points to the labels for + * the rest. + */ +export function resolveTouchedScope(analysis) { + const touched = [ + ...new Set([ + ...strings(analysis.newComponentOwners), + ...strings(analysis.modifiedComponentOwners), + ]), + ].filter(owner => owner.includes('/')); + const packages = [ + ...new Set(touched.map(owner => ownerIdentity(owner).shortPackage)), + ].sort(); + return { + touched, + visualComponents: [ + ...new Set( + touched + .map(ownerIdentity) + .filter(identity => identity.shortPackage === 'core') + .map(identity => identity.name), + ), + ].sort(), + rtlOwnersByPackage: Object.fromEntries( + packages.map(pkg => [ + pkg, + touched.filter(owner => owner.startsWith(`${pkg}/`)), + ]), + ), + a11yOwners: touched, + broad: analysis.forceFullComponentAudits === true, + }; +} + +function readJson(file) { + if (file == null || !fs.existsSync(file)) return null; + try { + return JSON.parse(fs.readFileSync(file, 'utf8')); + } catch { + return null; + } +} + +/** Whether a component name or owner from a report names this owner. */ +function namesOwner(value, identity) { + if (typeof value !== 'string') return false; + return ( + value === identity.key || + value === identity.name || + value === `${identity.package}/${identity.name}` + ); +} + +/** + * The visual result for one owner, from the gate's verdict. Added and removed + * shots are listed by key; the run's manifest names their components. + */ +export function visualResult(identity, verdict, scope, shots = {}) { + if (verdict == null) return {result: 'not-run'}; + if (identity.shortPackage !== 'core') { + return {result: 'not-run', detail: 'no stable visual stories outside Core'}; + } + if ( + !scope.visualComponents.includes(identity.name) || + verdict.status === 'skipped' + ) { + return {result: 'not-run'}; + } + const mine = entry => + namesOwner( + typeof entry === 'string' ? shots[entry]?.component : entry?.component, + identity, + ); + if ((verdict.failures ?? []).some(mine)) return {result: 'fail'}; + const changed = + (verdict.changes ?? []).filter(mine).length + + (verdict.added ?? []).filter(mine).length + + (verdict.removed ?? []).filter(mine).length; + if (changed > 0) { + return {result: 'changed', detail: `${changed} story image(s) changed`}; + } + return {result: 'pass'}; +} + +/** The accessibility result for one owner: new violations fail it. */ +export function a11yResult(identity, report, newViolations) { + if (report == null) return {result: 'not-run'}; + const audited = Object.keys(report.components ?? {}).some(component => + namesOwner(component, identity), + ); + if (!audited) return {result: 'not-run'}; + const mine = newViolations.filter(violation => + namesOwner(violation.component, identity), + ); + return mine.length > 0 + ? { + result: 'fail', + detail: [...new Set(mine.map(violation => violation.ruleId))].join( + ', ', + ), + } + : {result: 'pass'}; +} + +const RTL_SECTIONS = [ + 'autoDiscovery', + 'positionalMirror', + 'directionalDecorations', +]; + +/** The RTL result for one owner, across every package report. */ +export function rtlResult(identity, reports) { + const results = reports.flatMap(report => + RTL_SECTIONS.flatMap(section => report?.[section]?.results ?? []), + ); + const mine = results.filter(result => namesOwner(result.component, identity)); + if (mine.length === 0) return {result: 'not-run'}; + if (mine.every(result => result.verdict === 'N-A')) { + return {result: 'not-run', detail: 'no RTL check applies'}; + } + const failed = mine.filter( + result => result.verdict !== 'pass' && result.verdict !== 'N-A', + ); + if (failed.length > 0) { + return {result: 'fail', detail: `${failed.length} RTL check(s) failed`}; + } + return {result: 'pass'}; +} + +/** touched-components.json for the pull-request comment. */ +export function buildTouchedReport({ + headSha, + scope, + verdict, + visualShots = {}, + a11yReport, + a11yBaseline, + rtlReports, +}) { + const newViolations = + a11yReport != null && a11yBaseline != null + ? diffAgainstBaseline(a11yReport, a11yBaseline).newViolations + : []; + const row = owner => { + const identity = ownerIdentity(owner); + const visual = visualResult(identity, verdict, scope, visualShots); + const a11y = a11yResult(identity, a11yReport, newViolations); + const rtl = rtlResult(identity, rtlReports); + const details = Object.entries({visual, a11y, rtl}) + .filter(([, value]) => value.detail) + .map(([suite, value]) => `${suite}: ${value.detail}`); + return { + key: identity.key, + package: identity.package, + name: identity.name, + results: {visual: visual.result, a11y: a11y.result, rtl: rtl.result}, + ...(details.length > 0 ? {detail: details.join('; ')} : {}), + }; + }; + return { + headSha, + wideChange: false, + componentCount: scope.touched.length, + broad: scope.broad, + components: scope.touched.map(row), + }; +} + +export function hasFailure(report) { + return report.components.some(component => + SUITES.some(suite => component.results[suite] === 'fail'), + ); +} + +/** The record a wide change writes instead of results. */ +export function wideReport({headSha, scope, decision}) { + return { + headSha, + wideChange: true, + componentCount: decision.componentCount, + foundations: decision.foundations, + components: scope.touched.map(owner => { + const identity = ownerIdentity(owner); + return { + key: identity.key, + package: identity.package, + name: identity.name, + }; + }), + }; +} + +export function summaryMarkdown(report) { + if (report.wideChange) { + const reason = + report.foundations?.length > 0 + ? `it changes shared styles (${report.foundations.join(', ')})` + : `it touches ${report.componentCount} components (more than ${MAX_TOUCHED_COMPONENTS})`; + return `### Touched components\n\nWide change (${report.componentCount} components): the touched-components check did not run because ${reason}. Add \`ci:run-visual\`, \`ci:run-a11y\` or \`ci:run-rtl\` to run the full suite.\n`; + } + if (report.components.length === 0) { + return '### Touched components\n\nNo component changed.\n'; + } + const lines = [ + '### Touched components', + '', + '| Component | Visual | Accessibility | RTL |', + '| --- | --- | --- | --- |', + ...report.components.map( + component => + `| ${component.key} | ${component.results.visual} | ${component.results.a11y} | ${component.results.rtl} |`, + ), + ]; + if (report.broad) { + lines.push( + '', + 'This change reaches shared code the analysis could not scope; only the touched components were audited. Add `ci:run-visual`, `ci:run-a11y` or `ci:run-rtl` to audit everything it reaches.', + ); + } + return `${lines.join('\n')}\n`; +} + +function arg(argv, name) { + const index = argv.indexOf(`--${name}`); + return index === -1 ? undefined : argv[index + 1]; +} + +function writeOutputs(entries) { + const lines = Object.entries(entries).map( + ([key, value]) => `${key}=${value}`, + ); + if (process.env.GITHUB_OUTPUT) { + fs.appendFileSync(process.env.GITHUB_OUTPUT, `${lines.join('\n')}\n`); + } else { + console.log(lines.join('\n')); + } +} + +export function main(argv = process.argv.slice(2)) { + const [command] = argv; + const analysis = readJson(arg(argv, 'analysis')) ?? {}; + const scope = resolveTouchedScope(analysis); + if (command === 'scope') { + const filesPath = arg(argv, 'files'); + const files = + filesPath != null && fs.existsSync(filesPath) + ? fs.readFileSync(filesPath, 'utf8').split('\n').filter(Boolean) + : []; + const decision = decideWideChange(scope, files); + if (decision.wideChange) { + const report = wideReport({ + headSha: arg(argv, 'head-sha') ?? null, + scope, + decision, + }); + fs.writeFileSync( + arg(argv, 'out') ?? 'touched-components.json', + `${JSON.stringify(report, null, 2)}\n`, + ); + const summary = summaryMarkdown(report); + if (process.env.GITHUB_STEP_SUMMARY) { + fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, summary); + } + console.log(summary); + } + writeOutputs({ + wide_change: String(decision.wideChange), + component_count: String(decision.componentCount), + has_components: String(scope.touched.length > 0 && !decision.wideChange), + visual_components: scope.visualComponents.join(','), + a11y_owners: scope.a11yOwners.join(','), + rtl_packages: Object.keys(scope.rtlOwnersByPackage).join(','), + rtl_owners: scope.touched.join(','), + }); + return 0; + } + if (command === 'report') { + const rtlDir = arg(argv, 'rtl-dir'); + const rtlReports = + rtlDir != null && fs.existsSync(rtlDir) + ? fs + .readdirSync(rtlDir) + .filter(file => file.endsWith('.json')) + .map(file => readJson(path.join(rtlDir, file))) + .filter(Boolean) + : []; + const report = buildTouchedReport({ + headSha: arg(argv, 'head-sha') ?? null, + scope, + verdict: readJson(arg(argv, 'visual')), + visualShots: readJson(arg(argv, 'visual-manifest'))?.shots ?? {}, + a11yReport: readJson(arg(argv, 'a11y')), + a11yBaseline: readJson(arg(argv, 'a11y-baseline')), + rtlReports, + }); + fs.writeFileSync( + arg(argv, 'out') ?? 'touched-components.json', + `${JSON.stringify(report, null, 2)}\n`, + ); + const summary = summaryMarkdown(report); + if (process.env.GITHUB_STEP_SUMMARY) { + fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, summary); + } + console.log(summary); + return hasFailure(report) ? 1 : 0; + } + console.error( + 'usage: touched-components.mjs scope|report --analysis analysis.json [...]', + ); + return 2; +} + +if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) { + process.exitCode = main(); +} diff --git a/.github/scripts/touched-components.test.mjs b/.github/scripts/touched-components.test.mjs new file mode 100644 index 0000000000000..6946c263d96af --- /dev/null +++ b/.github/scripts/touched-components.test.mjs @@ -0,0 +1,374 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file Touched-components check: scope, wide changes, per-component results + * and routing. + * @input touched-components.mjs with fixture reports, and ci.yml + * @output Only touched components are audited (never reverse dependents), a + * wide change skips the check, each suite's result is read per component, + * and the job runs by default on a pull request without becoming a required + * context + * @position Node contracts for AST-030 FR12 + */ + +import fs from 'node:fs'; +import {describe, expect, it} from 'vitest'; +import yaml from 'yaml'; +import {ungated} from './ci-pr-gate.mjs'; +import { + MAX_TOUCHED_COMPONENTS, + a11yResult, + buildTouchedReport, + decideWideChange, + foundationChanges, + hasFailure, + ownerIdentity, + resolveTouchedScope, + rtlResult, + summaryMarkdown, + visualResult, + wideReport, +} from './touched-components.mjs'; + +const ci = yaml.parse( + fs.readFileSync(new URL('../workflows/ci.yml', import.meta.url), 'utf8'), +); +const job = ci.jobs['pr-touched-components']; +const step = name => job.steps.find(candidate => candidate.name === name); + +const analysis = { + newComponentOwners: ['core/Badge'], + modifiedComponentOwners: ['core/Button', 'lab/MobileTokenizer'], + dependentComponentOwners: ['core/Toolbar', 'core/Button'], + dependentComponentFolders: ['core/Toolbar'], + forceFullComponentAudits: false, +}; + +describe('touched scope', () => { + it('audits the touched owners of every package, never their reverse dependents', () => { + const scope = resolveTouchedScope(analysis); + expect(scope.touched).toEqual([ + 'core/Badge', + 'core/Button', + 'lab/MobileTokenizer', + ]); + expect(scope.visualComponents).toEqual(['Badge', 'Button']); + expect(scope.a11yOwners).toEqual(scope.touched); + expect(scope.rtlOwnersByPackage).toEqual({ + core: ['core/Badge', 'core/Button'], + lab: ['lab/MobileTokenizer'], + }); + expect(JSON.stringify(scope)).not.toContain('Toolbar'); + }); + + it('never widens to a full suite, even for a change the analysis cannot scope', () => { + const scope = resolveTouchedScope({ + ...analysis, + forceFullComponentAudits: true, + }); + expect(scope.broad).toBe(true); + expect(scope.touched).toHaveLength(3); + expect( + resolveTouchedScope({forceFullComponentAudits: true}).touched, + ).toEqual([]); + }); + + it('names a component by package and name', () => { + expect(ownerIdentity('lab/MobileTokenizer')).toEqual({ + key: 'lab/MobileTokenizer', + package: '@astryxdesign/lab', + shortPackage: 'lab', + name: 'MobileTokenizer', + }); + }); +}); + +describe('wide changes', () => { + const owners = count => + resolveTouchedScope({ + modifiedComponentOwners: Array.from( + {length: count}, + (_, index) => `core/C${index}`, + ), + }); + + it('runs for up to five touched components and skips above that', () => { + expect(MAX_TOUCHED_COMPONENTS).toBe(5); + expect(decideWideChange(owners(5), [])).toEqual({ + componentCount: 5, + foundations: [], + wideChange: false, + }); + expect(decideWideChange(owners(6), []).wideChange).toBe(true); + expect(decideWideChange(owners(21), []).componentCount).toBe(21); + }); + + it('treats style-affecting foundations as wide, and behaviour-only shared code as not', () => { + const foundations = [ + 'packages/themes/neutral/src/neutralTheme.ts', + 'packages/core/src/theme/tokens.stylex.ts', + 'packages/core/src/utils/focusOutline.stylex.ts', + 'packages/core/src/Layer/layer.stylex.ts', + 'packages/lab/src/theme/labTokens.ts', + ]; + const behaviour = [ + 'packages/core/src/utils/pressFeedback.ts', + 'packages/core/src/hooks/useMenuPress.ts', + 'packages/core/src/gestures/GestureHost.ts', + 'packages/core/src/motion/animator.ts', + 'packages/core/src/Button/Button.stylex.ts', + 'packages/core/src/Button/Button.tsx', + ]; + expect(foundationChanges([...foundations, ...behaviour])).toEqual( + foundations, + ); + expect( + decideWideChange(owners(1), foundations.slice(0, 1)).wideChange, + ).toBe(true); + expect(decideWideChange(owners(1), behaviour).wideChange).toBe(false); + }); + + it('writes the wide record and points to the full-suite labels', () => { + const scope = owners(6); + const report = wideReport({ + headSha: 'abc', + scope, + decision: decideWideChange(scope, []), + }); + expect(report).toMatchObject({ + headSha: 'abc', + wideChange: true, + componentCount: 6, + }); + expect(report.components[0]).toEqual({ + key: 'core/C0', + package: '@astryxdesign/core', + name: 'C0', + }); + expect(summaryMarkdown(report)).toContain('Wide change (6 components)'); + expect(summaryMarkdown(report)).toContain('`ci:run-visual`'); + }); +}); + +describe('per-component results', () => { + const scope = resolveTouchedScope(analysis); + const button = ownerIdentity('core/Button'); + const badge = ownerIdentity('core/Badge'); + const tokenizer = ownerIdentity('lab/MobileTokenizer'); + + it('reads visual changes, additions and failures by component', () => { + const verdict = { + status: 'changed', + changes: [{key: 'a', component: 'Button'}], + added: ['b'], + removed: [], + failures: [], + }; + const shots = {b: {component: 'Badge'}}; + expect(visualResult(button, verdict, scope, shots).result).toBe('changed'); + expect(visualResult(badge, verdict, scope, shots).result).toBe('changed'); + expect( + visualResult(ownerIdentity('core/Toolbar'), verdict, scope, shots).result, + ).toBe('not-run'); + expect(visualResult(tokenizer, verdict, scope, shots)).toEqual({ + result: 'not-run', + detail: 'no stable visual stories outside Core', + }); + expect( + visualResult( + button, + {...verdict, failures: [{component: 'Button'}]}, + scope, + ).result, + ).toBe('fail'); + expect(visualResult(button, null, scope).result).toBe('not-run'); + expect(visualResult(button, {status: 'skipped'}, scope).result).toBe( + 'not-run', + ); + }); + + it('fails accessibility only for new violations, and only where it audited', () => { + const report = {components: {'core/Button': {}, 'core/Badge': {}}}; + const newViolations = [ + {component: 'core/Button', ruleId: 'color-contrast'}, + ]; + expect(a11yResult(button, report, newViolations)).toEqual({ + result: 'fail', + detail: 'color-contrast', + }); + expect(a11yResult(badge, report, newViolations).result).toBe('pass'); + expect(a11yResult(tokenizer, report, newViolations).result).toBe('not-run'); + expect(a11yResult(button, null, []).result).toBe('not-run'); + }); + + it('reads RTL verdicts across every package report', () => { + const reports = [ + { + autoDiscovery: { + results: [ + {component: 'core/Button', verdict: 'pass'}, + {component: 'core/Badge', verdict: 'N-A'}, + ], + }, + positionalMirror: { + results: [{component: 'core/Badge', verdict: 'fail'}], + }, + }, + { + autoDiscovery: { + results: [{component: 'lab/MobileTokenizer', verdict: 'ERROR'}], + }, + }, + ]; + expect(rtlResult(button, reports).result).toBe('pass'); + expect(rtlResult(badge, reports)).toEqual({ + result: 'fail', + detail: '1 RTL check(s) failed', + }); + expect(rtlResult(tokenizer, reports).result).toBe('fail'); + expect(rtlResult(ownerIdentity('core/Toolbar'), reports).result).toBe( + 'not-run', + ); + // Every check not applicable is not a pass. + expect( + rtlResult(ownerIdentity('core/Toast'), [ + {autoDiscovery: {results: [{component: 'core/Toast', verdict: 'N-A'}]}}, + ]), + ).toEqual({result: 'not-run', detail: 'no RTL check applies'}); + }); + + it('writes one row per component in the shape the comment reads', () => { + const report = buildTouchedReport({ + headSha: 'abc', + scope, + verdict: { + status: 'pass', + changes: [], + added: [], + removed: [], + failures: [], + }, + a11yReport: {components: {'core/Button': {storyDetails: []}}}, + a11yBaseline: {version: 1, entries: []}, + rtlReports: [ + { + autoDiscovery: { + results: [{component: 'core/Button', verdict: 'pass'}], + }, + }, + ], + }); + expect(report).toMatchObject({ + headSha: 'abc', + wideChange: false, + componentCount: 3, + }); + expect(report.components.map(row => row.key)).toEqual([ + 'core/Badge', + 'core/Button', + 'lab/MobileTokenizer', + ]); + expect(report.components[1]).toEqual({ + key: 'core/Button', + package: '@astryxdesign/core', + name: 'Button', + results: {visual: 'pass', a11y: 'pass', rtl: 'pass'}, + }); + expect(report.components[2].results).toEqual({ + visual: 'not-run', + a11y: 'not-run', + rtl: 'not-run', + }); + expect(hasFailure(report)).toBe(false); + expect(summaryMarkdown(report)).toContain( + '| core/Button | pass | pass | pass |', + ); + }); + + it('points to the full-suite labels when the change was broad', () => { + const report = { + broad: true, + components: [ + { + key: 'core/Button', + results: {visual: 'pass', a11y: 'pass', rtl: 'pass'}, + }, + ], + }; + expect(summaryMarkdown(report)).toContain('`ci:run-visual`'); + }); +}); + +describe('pr-touched-components routing', () => { + it('runs on every pull request after the Storybook build and pr-visual, on one standard arm runner', () => { + expect(job.needs).toEqual(['build-storybook', 'pr-visual']); + expect(ungated(job.if)).toBe( + "!cancelled() && github.event_name == 'pull_request' && needs.build-storybook.result == 'success'", + ); + expect(job['runs-on']).toBe('ubuntu-24.04-arm'); + expect(job['timeout-minutes']).toBeLessThanOrEqual(15); + expect(job.permissions).toEqual({ + contents: 'read', + 'pull-requests': 'read', + }); + }); + + it('exposes the wide-change flag and count', () => { + expect(job.outputs).toEqual({ + wide_change: '${{ steps.touched.outputs.wide_change }}', + component_count: '${{ steps.touched.outputs.component_count }}', + }); + expect(step('Decide the touched components').run).toContain( + '--files "$FILES_PATH"', + ); + }); + + it('is no required context and no release-check owner', () => { + for (const join of ['test', 'build', 'docsite-test', 'release-check']) { + expect([ci.jobs[join].needs].flat(), join).not.toContain( + 'pr-touched-components', + ); + } + }); + + it('audits accessibility and RTL for touched components, and takes the visual verdict from pr-visual', () => { + // pr-visual is the one visual owner (AST-030 FR12): this job runs no + // visual gate of its own. + expect(JSON.stringify(job)).not.toContain('gate.mjs'); + const verdict = step("Download pr-visual's verdict"); + expect(verdict.with.name).toBe('visual-pr-report'); + expect(verdict['continue-on-error']).toBe(true); + expect( + step('Accessibility audit for the touched components').run, + ).toContain('a11y-pr-scope.mjs analysis.json'); + const rtl = step('RTL audit for the touched components').run; + expect(rtl).toContain('--filter "$FILTER"'); + // A finding never stops the job before the report judges it. + expect(rtl).toMatch(/--filter "\$FILTER" \|\|/); + expect(verdict.if).toMatch(/^always\(\) && /); + expect(step('Report per-component results').run).toContain( + '--visual .visual-run/verdict.json', + ); + }); + + it('skips visibly when no component changed or the change is wide, and gates every audit on the scope', () => { + expect( + step('Skip when no component changed or the change is wide').if, + ).toBe("steps.touched.outputs.has_components != 'true'"); + const scopeIndex = job.steps.findIndex( + candidate => candidate.id === 'touched', + ); + for (const candidate of job.steps.slice(scopeIndex + 2)) { + expect(candidate.if, candidate.name).toContain( + "steps.touched.outputs.has_components == 'true'", + ); + } + }); + + it('uploads touched-components.json for the comment, for a wide change too', () => { + const upload = step('Upload touched-components results'); + expect(upload.with.name).toBe('touched-components'); + expect(upload.with.path).toContain('touched-components.json'); + expect(upload.if).toContain("steps.touched.outputs.wide_change == 'true'"); + }); +}); diff --git a/.github/scripts/visual-maintenance-workflow.test.mjs b/.github/scripts/visual-maintenance-workflow.test.mjs index 10cae5b6bb1de..2872e9c4c31e6 100644 --- a/.github/scripts/visual-maintenance-workflow.test.mjs +++ b/.github/scripts/visual-maintenance-workflow.test.mjs @@ -127,10 +127,10 @@ describe('CI baseline maintenance routing', () => { const optedIn = {event: 'pull_request', needs}; // The job decides its stable visual scope in its own steps. expect(runs(visual, optedIn)).toBe(true); - // Without the opt-in a pull request update runs no visual owner, and - // another suite's label does not opt it in. + // Without the opt-in pr-visual still runs, for its default touched check + // (its full check's step needs ci:run-visual; ci-heavy-lanes.test.mjs). needs['run-labels'].outputs = {visual: 'false', a11y: 'true', rtl: 'true'}; - expect(runs(visual, {event: 'pull_request', needs})).toBe(false); + expect(runs(visual, {event: 'pull_request', needs})).toBe(true); needs['run-labels'].outputs = {visual: 'true', a11y: 'false', rtl: 'false'}; needs['build-storybook'].result = 'failure'; expect(runs(visual, optedIn)).toBe(false); diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8f5db03721062..1f9e57361409e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2283,7 +2283,7 @@ jobs: needs: [build-storybook, maintenance-request, visual-scout, run-labels] if: >- (github.event_name != 'pull_request' || !github.event.pull_request.draft && (github.event.pull_request.head.repo.full_name == github.repository || contains(github.event.pull_request.labels.*.name, 'ci:run'))) && (!cancelled() && - (((github.event_name == 'pull_request' && needs.run-labels.outputs.visual == 'true') && + ((github.event_name == 'pull_request' && needs.build-storybook.result == 'success') || (github.event_name == 'workflow_dispatch' && ((inputs.operation == 'release-check' && @@ -2349,6 +2349,21 @@ jobs: with: name: pr-analysis + # Without ci:run-visual a pull request gets the default touched check: + # only the Core components it touches, never their reverse dependents + # and never the release plan; a wide change skips it (spec:AST-030 + # FR12). + - name: Decide the default touched visual check + id: touched + if: github.event_name == 'pull_request' && needs.run-labels.outputs.visual != 'true' && steps.visual-scope.outputs.has_stable_visual == 'true' + env: + FILES_PATH: ${{ steps.scope.outputs.changed_files_path }} + run: | + node .github/scripts/touched-components.mjs scope \ + --analysis analysis.json \ + --files "$FILES_PATH" \ + --out touched-scope.json + - name: Download Storybook artifact if: (github.event_name == 'pull_request' || inputs.operation == 'release-check') && (github.event_name != 'pull_request' || steps.visual-scope.outputs.has_stable_visual == 'true') uses: actions/download-artifact@v8 @@ -2389,7 +2404,7 @@ jobs: - name: Run the visual gate for the touched components id: visual - if: (github.event_name == 'pull_request') && steps.visual-scope.outputs.has_stable_visual == 'true' + if: (github.event_name == 'pull_request' && needs.run-labels.outputs.visual == 'true') && steps.visual-scope.outputs.has_stable_visual == 'true' env: BROAD_STABLE_VISUAL: ${{ steps.visual-scope.outputs.broad_stable_visual }} ASTRYX_PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} @@ -2477,6 +2492,32 @@ jobs: *) echo "Visual evidence is incomplete: $STATUS"; exit 1 ;; esac + # Exit 0 is clean, 2 is a change for review and 3 means nothing touched + # has a stable story (no verdict); anything else is an infrastructure + # failure. + - name: Visual check for the default touched components + if: steps.touched.outputs.has_components == 'true' && steps.touched.outputs.visual_components != '' + env: + COMPONENTS: ${{ steps.touched.outputs.visual_components }} + run: | + set +e + node .github/scripts/visual-gate/gate.mjs check \ + --storybook-dir apps/storybook/dist \ + --baseline .github/visual-baseline \ + --out .visual-run \ + --tiers component \ + --components "$COMPONENTS" \ + --no-scout \ + --summary-output visual-summary.md + code=$? + set -e + case "$code" in + 0|2) ;; + 3) rm -f .visual-run/verdict.json ;; + *) exit "$code" ;; + esac + if [ -f visual-summary.md ]; then cat visual-summary.md >> "$GITHUB_STEP_SUMMARY"; fi + - name: Upload visual verdict and report if: (always() && github.event_name == 'pull_request') && steps.visual-scope.outputs.has_stable_visual == 'true' uses: actions/upload-artifact@v7 @@ -2954,6 +2995,174 @@ jobs: # Release-time projection only: the existing owners above do all measurement. # A skipped owner or main drift must never look like passing release evidence. + # The default pull-request check for components: accessibility and RTL for + # only the components the change touches, on one standard runner, against + # this pull request's Storybook build, with pr-visual's verdict for the same + # components folded in (pr-visual stays the one visual owner, spec:AST-030 + # FR12). It never widens to reverse dependents or a full suite: those run + # behind the ci:run- labels and in the release check. It writes + # touched-components.json, one row per component with each suite's + # pass/fail/changed/not-run result, or the wide flag and count, for the + # pull-request comment. No required context depends on it. + pr-touched-components: + name: Touched components (visual, accessibility, RTL) + needs: [build-storybook, pr-visual] + if: ${{ (github.event_name != 'pull_request' || !github.event.pull_request.draft && (github.event.pull_request.head.repo.full_name == github.repository || contains(github.event.pull_request.labels.*.name, 'ci:run'))) && (!cancelled() && github.event_name == 'pull_request' && needs.build-storybook.result == 'success') }} + runs-on: ubuntu-24.04-arm + timeout-minutes: 15 + permissions: + contents: read + pull-requests: read + outputs: + wide_change: ${{ steps.touched.outputs.wide_change }} + component_count: ${{ steps.touched.outputs.component_count }} + steps: + # The shared scope action lists the pull request's changed files, which + # decide a wide change. + - name: Check out the scope action + uses: actions/checkout@v7 + with: + sparse-checkout: .github/actions/test-scope + path: .test-scope + persist-credentials: false + + - name: List the changed files + id: scope + uses: ./.test-scope/.github/actions/test-scope + with: + component-audit: 'true' + + - name: Checkout + uses: actions/checkout@v7 + with: + ref: ${{ github.event.pull_request.head.sha }} + persist-credentials: false + + - name: Download analysis artifact + uses: actions/download-artifact@v8 + with: + name: pr-analysis + + # More than five touched components, or a change to shared styles + # (themes, tokens, shared .stylex.ts), is a wide change: this check + # skips and the comment points to the ci:run- labels. + - name: Decide the touched components + id: touched + env: + FILES_PATH: ${{ steps.scope.outputs.changed_files_path }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + node .github/scripts/touched-components.mjs scope \ + --analysis analysis.json \ + --files "$FILES_PATH" \ + --head-sha "$HEAD_SHA" \ + --out touched-components.json + + - name: Skip when no component changed or the change is wide + if: steps.touched.outputs.has_components != 'true' + env: + WIDE: ${{ steps.touched.outputs.wide_change }} + COUNT: ${{ steps.touched.outputs.component_count }} + run: | + if [ "$WIDE" = true ]; then + echo "Wide change ($COUNT components): add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suite." + else + echo "No component changed; nothing to audit." + fi + + - name: Setup Node and pnpm + if: steps.touched.outputs.has_components == 'true' + uses: ./.github/actions/setup + with: + install: 'false' + + - name: Download Storybook artifact + if: steps.touched.outputs.has_components == 'true' + uses: actions/download-artifact@v8 + with: + name: storybook-${{ needs.build-storybook.outputs.short_hash }} + path: apps/storybook/dist + + - name: Install Playwright + if: steps.touched.outputs.has_components == 'true' + run: pnpm install --frozen-lockfile && npx playwright install chromium + + # The same changed-component filters an opted-in pull request uses. + - name: Accessibility audit for the touched components + if: steps.touched.outputs.has_components == 'true' && steps.touched.outputs.a11y_owners != '' + run: | + COMPONENTS=$(node .github/scripts/a11y-pr-scope.mjs analysis.json) + if [ -z "$COMPONENTS" ]; then + echo "No touched component has accessibility stories." + exit 0 + fi + node .github/scripts/accessibility-audit.js \ + --storybook-dir apps/storybook/dist \ + --output a11y-report.json \ + --concurrency 4 \ + --components "$COMPONENTS" + + - name: RTL audit for the touched components + if: steps.touched.outputs.has_components == 'true' && steps.touched.outputs.rtl_packages != '' + env: + PACKAGES: ${{ steps.touched.outputs.rtl_packages }} + OWNERS: ${{ steps.touched.outputs.rtl_owners }} + run: | + mkdir -p rtl-reports + IFS=',' read -ra PACKAGE_LIST <<< "$PACKAGES" + for PACKAGE in "${PACKAGE_LIST[@]}"; do + FILTER=$(tr ',' '\n' <<< "$OWNERS" | grep "^$PACKAGE/" | paste -sd, -) + [ -n "$FILTER" ] || continue + # Findings are the report's to judge, as in pr-rtl-shard: a + # nonzero exit (a failed check or a coverage gap) still writes the + # package report. + node apps/storybook/rtl-audit/rtl-audit.mjs \ + --storybook-dir apps/storybook/dist \ + --output "rtl-reports/$PACKAGE.json" \ + --packages "$PACKAGE" \ + --concurrency 8 \ + --filter "$FILTER" || + echo "RTL audit for $PACKAGE exited $?; the per-component report judges it." + test -s "rtl-reports/$PACKAGE.json" + done + + # The visual result comes from pr-visual, the one visual owner: its + # default touched check, or its full check under ci:run-visual. + - name: Download pr-visual's verdict + if: always() && steps.touched.outputs.has_components == 'true' && needs.pr-visual.result == 'success' + continue-on-error: true + uses: actions/download-artifact@v8 + with: + name: visual-pr-report + path: .visual-run + + - name: Report per-component results + if: always() && steps.touched.outputs.has_components == 'true' + env: + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + node .github/scripts/touched-components.mjs report \ + --analysis analysis.json \ + --visual .visual-run/verdict.json \ + --visual-manifest .visual-run/manifest.json \ + --a11y a11y-report.json \ + --a11y-baseline .github/a11y-baseline.json \ + --rtl-dir rtl-reports \ + --head-sha "$HEAD_SHA" \ + --out touched-components.json + + - name: Upload touched-components results + if: always() && (steps.touched.outputs.has_components == 'true' || steps.touched.outputs.wide_change == 'true') + uses: actions/upload-artifact@v7 + with: + name: touched-components + path: | + touched-components.json + a11y-report.json + rtl-reports/ + if-no-files-found: ignore + retention-days: 7 + release-check: needs: [ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 05c65e6fdb36e..efe0134fec7aa 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -932,6 +932,12 @@ Storybook story for anything you added or altered. takes effect on the next push or a close and reopen, and removing it stops that suite from the next run. When the accessibility audit did not run, the pull-request comment names `ci:run-a11y`. +- **Every pull request runs a touched-components check** (visual, + accessibility and RTL for only the components it changes) on one standard + runner. It skips a wide change, one that touches more than five components + or changes shared styles (themes, tokens, shared `.stylex.ts` files); add + the `ci:run-*` labels for those. It is advisory: no required check waits for + it. - **Pull requests from a branch in this repository run CI without the label**, since pushing a branch here needs write access. Maintainers who work from a fork either push their branches here or apply `ci:run` themselves. diff --git a/docs/specs/AST-030/spec.md b/docs/specs/AST-030/spec.md index e659ea2bf176d..029d4f34e468e 100644 --- a/docs/specs/AST-030/spec.md +++ b/docs/specs/AST-030/spec.md @@ -195,9 +195,24 @@ they are separately named. run only when the pull request carries that suite's label at the event: `ci:run-visual`, `ci:run-a11y` or `ci:run-rtl`, in each case last applied by someone with write, maintain or admin access, and a lookup that cannot - confirm that runs nothing. A label runs only its own suite; a pull request - update without one runs none of them, and no required context depends on - them. An opted-in PR's a11y MUST remain scoped to changed components + confirm that runs nothing. A label runs only its own suite. Without a label, a pull request update + runs none of these owners at their opted-in scope, and no required context + depends on them; it runs one default touched-components check instead: + visual, accessibility and RTL for only the components the change touches, + never their reverse dependents, against the pull request's Storybook build + and within the 15-minute budget. Its visual part is `pr-visual`'s default + mode, a focused check of the touched Core components that never escalates + to the release plan, so `pr-visual` stays the one visual owner; its + accessibility and RTL parts and the per-component record are + `pr-touched-components`, on one standard runner. A wide change skips it: one + that touches more than five components, or changes shared styles (themes, + tokens, or a `.stylex.ts` file in a package's shared `theme`, `utils`, + `hooks`, `internal` or `Layer` folder); behaviour-only shared code is not a + wide change. Nothing starts a full suite on its own; a wide change's check + is the full suites behind the labels. No required context depends on the + touched check, and it records each touched component's pass, fail, changed + or not-run result per suite, or that the change was wide and how many + components it touched, for the pull-request comment. An opted-in PR's a11y MUST remain scoped to changed components plus fast interaction guards; full a11y sweeps, whole-repository spec-test contracts, and their evidence uploads MUST run only for `release-check` in this workflow. Missing, @@ -479,7 +494,10 @@ the same exact-commit `release-check` the release gate uses, and before a cut; a pull request runs each only when it opts in with that suite's `ci:run-visual`, `ci:run-a11y` or `ci:run-rtl` label. Main-push checks are throttled: one runs at a time, a burst of pushes collapses to its newest commit, and a running check is never cancelled, so every check that starts finishes. -Pull request updates keep the fast checks and every required context; the +Pull request updates keep the fast checks and every required context, and add +the touched-components check (FR12), which audits only what the change touches +on one standard runner, so each update spends few runner-minutes while the full +suites stay behind their labels; the modal-close, theme-var and story-play guards are steps of the accessibility owner, so they run with it, after merge or on a labelled pull request. A failure on `main` is repaired forward with an atomic pull request, not reverted From 2fc5b26ba2d634fe4f7957005087afe0a7b2846f Mon Sep 17 00:00:00 2001 From: Cindy Zhang Date: Sat, 31 Oct 2026 18:32:43 -0700 Subject: [PATCH 2/7] ci: touched-components review fixes pr-visual's default step sets the PR head and base and marks its verdict scope: touched, so the PR comment accepts it and names ci:run-visual; a wide change writes a skipped verdict that does the same. Only touched components with an accepted baseline shot are compared. pr-touched-components skips the analysis for spec-only, docsite-only and tooling-only changes, reads a failed pr-visual's verdict (and reports a failed visual job as fail), counts curated RTL cards, and expands a changed multi-component folder to its members, counted as one component. AST-030 FR11/DEC-4 record the advisory job; FR12, FR13, DEC-6, the DEC-7 title and rtl-audit-evidence INV7 match. --- .github/scripts/ci-heavy-lanes.test.mjs | 3 +- .github/scripts/lib/visual-format.js | 14 + .github/scripts/lib/visual-format.test.mjs | 19 ++ .github/scripts/run-labels.test.mjs | 4 +- .github/scripts/touched-components.mjs | 288 +++++++++++++++--- .github/scripts/touched-components.test.mjs | 137 +++++++++ .github/scripts/visual-gate/gate.mjs | 3 + .../scripts/visual-gate/publish-pr-report.mjs | 2 + .../visual-gate/publish-pr-report.test.mjs | 9 + .github/workflows/ci.yml | 37 ++- docs/architecture/rtl-audit-evidence.md | 6 +- docs/specs/AST-030/spec.md | 34 ++- 12 files changed, 494 insertions(+), 62 deletions(-) diff --git a/.github/scripts/ci-heavy-lanes.test.mjs b/.github/scripts/ci-heavy-lanes.test.mjs index e29ac93bbcde6..b9d02bb6a91b1 100644 --- a/.github/scripts/ci-heavy-lanes.test.mjs +++ b/.github/scripts/ci-heavy-lanes.test.mjs @@ -1,7 +1,8 @@ // Copyright (c) Meta Platforms, Inc. and affiliates. /** - * @file Visual, accessibility and RTL run on main, not on pull request updates. + * @file Full visual, accessibility and RTL suites run on main; pull request + * updates check only touched components. * @input CI, Main and Main heavy lanes workflows, and a mocked GitHub API * @output Routing for the heavy owners, the required joins, and Main's * release-check dispatch diff --git a/.github/scripts/lib/visual-format.js b/.github/scripts/lib/visual-format.js index 54501059c4094..212f311b0e7b9 100644 --- a/.github/scripts/lib/visual-format.js +++ b/.github/scripts/lib/visual-format.js @@ -20,6 +20,20 @@ const {inline, num, safeUrl} = require('./report-text'); * @returns {string} markdown */ function buildVisualSection(verdict, reportUrl, imageUrl) { + const section = buildVisualSectionBody(verdict, reportUrl, imageUrl); + // pr-visual's default check compares only the touched components (spec: + // AST-030 FR12); say so, and name the label for the full suite. + if ( + section && + verdict?.context?.scope === 'touched' && + verdict.status !== 'skipped' + ) { + return `${section}Only the touched components were checked; add \`ci:run-visual\` to run the full suite.\n\n`; + } + return section; +} + +function buildVisualSectionBody(verdict, reportUrl, imageUrl) { if (!verdict) return ''; // Verdict fields are report data: render them as literal inline text (and diff --git a/.github/scripts/lib/visual-format.test.mjs b/.github/scripts/lib/visual-format.test.mjs index d5ba7f1719aaa..0842d715fcd65 100644 --- a/.github/scripts/lib/visual-format.test.mjs +++ b/.github/scripts/lib/visual-format.test.mjs @@ -26,6 +26,25 @@ describe('buildVisualSection', () => { expect(buildVisualSection(null)).toBe(''); }); + it("names ci:run-visual under pr-visual's default touched check", () => { + const touched = verdict({context: {scope: 'touched'}}); + expect(buildVisualSection(touched)).toContain( + 'Only the touched components were checked; add `ci:run-visual` to run the full suite.', + ); + expect(buildVisualSection(verdict())).not.toContain('ci:run-visual'); + // A wide change's skipped verdict carries its own reason. + expect( + buildVisualSection( + verdict({ + status: 'skipped', + reason: + 'Wide change (7 components): add ci:run-visual to run the full suite.', + context: {scope: 'touched'}, + }), + ), + ).toContain('Skipped — Wide change (7 components)'); + }); + it('says so plainly when nothing moved', () => { expect(buildVisualSection(verdict())).toContain( 'No visual change across 16 compared shot(s)', diff --git a/.github/scripts/run-labels.test.mjs b/.github/scripts/run-labels.test.mjs index 87cccc66e8a75..4e7a0d180c21a 100644 --- a/.github/scripts/run-labels.test.mjs +++ b/.github/scripts/run-labels.test.mjs @@ -199,8 +199,10 @@ describe('run-labels decision', () => { // Replay: each label's decision through every heavy owner's condition on a // pull request whose other needs succeeded. describe('each label runs only its own suite', () => { + // pr-visual runs on every pull request for its default touched check; its + // full check's step needs ci:run-visual (ci-heavy-lanes.test.mjs). const OWNERS = { - visual: ['visual-scout', 'pr-visual'], + visual: ['visual-scout'], a11y: ['pr-a11y-guards', 'pr-a11y-specs', 'pr-a11y'], rtl: ['pr-rtl-shard', 'pr-rtl-merge', 'pr-rtl'], }; diff --git a/.github/scripts/touched-components.mjs b/.github/scripts/touched-components.mjs index fb65cf9cba893..5d488dd7e069a 100644 --- a/.github/scripts/touched-components.mjs +++ b/.github/scripts/touched-components.mjs @@ -17,9 +17,17 @@ import fs from 'node:fs'; import path from 'node:path'; import {pathToFileURL} from 'node:url'; import {createRequire} from 'node:module'; +import componentPackages from '../../scripts/component-packages.cjs'; const require = createRequire(import.meta.url); const {diffAgainstBaseline} = require('./lib/a11y-baseline.js'); +const { + componentPackage, + componentExportsFromBarrel, + documentedComponentNames, + flatPackageComponentNames, + nestedPackageComponentNames, +} = componentPackages; const SUITES = ['visual', 'a11y', 'rtl']; @@ -52,17 +60,20 @@ export function foundationChanges(files) { } /** - * Whether the default check runs: at most MAX_TOUCHED_COMPONENTS touched and - * no foundation changed. A wide change skips it; nothing starts a full suite - * on its own. + * Whether the default check runs: at most MAX_TOUCHED_COMPONENTS touched + * component folders and no foundation changed. A wide change skips it; + * nothing starts a full suite on its own. Folders are counted, as the + * threshold's data counted them, so one change inside a multi-component + * folder such as Chat is one component. */ export function decideWideChange(scope, files) { const foundations = foundationChanges(files); + const componentCount = scope.folders.length; return { - componentCount: scope.touched.length, + componentCount, foundations, wideChange: - scope.touched.length > MAX_TOUCHED_COMPONENTS || foundations.length > 0, + componentCount > MAX_TOUCHED_COMPONENTS || foundations.length > 0, }; } @@ -82,34 +93,108 @@ const strings = value => Array.isArray(value) ? value.filter(item => typeof item === 'string') : []; /** - * What each suite audits for this pull request: the touched owners only. - * Reverse dependents stay with the opted-in owners (FR13): the analysis - * reports every component whose bundle reaches a change, which for a shared - * primitive is most of Core, so they would not fit this check's 15-minute limit. - * Nothing here selects a full suite either: a change the analysis cannot - * scope audits its touched owners, and the comment points to the labels for - * the rest. + * The public components a changed multi-component folder (such as Chat or + * Table) implements. The analysis leaves those folders unresolved; this + * expands only the changed folder, as the accessibility scope does, without + * reading Storybook. */ -export function resolveTouchedScope(analysis) { - const touched = [ - ...new Set([ - ...strings(analysis.newComponentOwners), - ...strings(analysis.modifiedComponentOwners), - ]), - ].filter(owner => owner.includes('/')); +export function expandFolderOwners(sources, repoRoot = process.cwd()) { + const members = []; + const publicByPackage = new Map(); + for (const source of sources) { + const [shortPackage, directory, extra] = source.split('/'); + const pkg = componentPackage(shortPackage); + if ( + !pkg || + extra !== undefined || + !/^[A-Za-z0-9_.-]+$/.test(directory ?? '') + ) { + continue; + } + const sourceRoot = path.join(repoRoot, pkg.src); + const changedDirectory = path.join(sourceRoot, directory); + if (!fs.existsSync(changedDirectory)) continue; + const names = documentedComponentNames(changedDirectory); + if (names.length === 0) continue; + if (!publicByPackage.has(shortPackage)) { + publicByPackage.set( + shortPackage, + new Set( + pkg.layout === 'flat' + ? flatPackageComponentNames(repoRoot, pkg) + : nestedPackageComponentNames(repoRoot, pkg), + ), + ); + } + const exports = componentExportsFromBarrel( + path.join(sourceRoot, 'index.ts'), + sourceRoot, + ); + for (const name of names) { + if (!publicByPackage.get(shortPackage).has(name)) continue; + if (exports.get(name)?.startsWith(`${changedDirectory}${path.sep}`)) { + members.push({owner: `${shortPackage}/${name}`, folder: directory}); + } + } + } + return members; +} + +/** + * What each suite audits for this pull request: the touched components only, + * in every package, with a changed multi-component folder expanded to its + * members. Reverse dependents stay with the opted-in owners (FR13): the + * analysis reports every component whose bundle reaches a change, which for a + * shared primitive is most of Core, so they would not fit this check's + * 15-minute limit. Nothing here selects a full suite either: a change the + * analysis cannot scope audits its touched components, and the comment + * points to the labels for the rest. Visual stories are keyed by folder, so + * a folder member's visual result is its folder's. + */ +export function resolveTouchedScope( + analysis, + {repoRoot = process.cwd(), expand = expandFolderOwners} = {}, +) { + const entries = new Map(); + for (const owner of [ + ...strings(analysis.newComponentOwners), + ...strings(analysis.modifiedComponentOwners), + ]) { + if (owner.includes('/') && !entries.has(owner)) { + entries.set(owner, ownerIdentity(owner).name); + } + } + for (const {owner, folder} of expand( + strings(analysis.unresolvedComponentSources), + repoRoot, + )) { + if (!entries.has(owner)) entries.set(owner, folder); + } + const touched = [...entries.keys()].sort(); + const visualKeyByOwner = Object.fromEntries(entries); + const folders = [ + ...new Set( + touched.map( + owner => + `${ownerIdentity(owner).shortPackage}/${visualKeyByOwner[owner]}`, + ), + ), + ].sort(); const packages = [ ...new Set(touched.map(owner => ownerIdentity(owner).shortPackage)), ].sort(); return { touched, + folders, + visualKeyByOwner, visualComponents: [ ...new Set( touched - .map(ownerIdentity) - .filter(identity => identity.shortPackage === 'core') - .map(identity => identity.name), + .filter(owner => ownerIdentity(owner).shortPackage === 'core') + .map(owner => visualKeyByOwner[owner]), ), ].sort(), + noVisualBaseline: [], rtlOwnersByPackage: Object.fromEntries( packages.map(pkg => [ pkg, @@ -121,6 +206,29 @@ export function resolveTouchedScope(analysis) { }; } +/** + * Keep only the visual components the accepted baseline has a shot for: a + * component that has none yet (one a pull request adds) has nothing to + * compare, so it is reported not-run instead of failing the check. Baseline + * story ids are `core---<story>`, ending in the component. + */ +export function withVisualBaseline(scope, baselineShots) { + const storyIds = Object.values(baselineShots ?? {}) + .map(shot => shot?.storyId) + .filter(id => typeof id === 'string'); + const hasShot = name => { + const lower = name.toLowerCase(); + return storyIds.some(id => + new RegExp(`^core-(?:[a-z0-9-]+-)?${lower}--`).test(id), + ); + }; + return { + ...scope, + visualComponents: scope.visualComponents.filter(hasShot), + noVisualBaseline: scope.visualComponents.filter(name => !hasShot(name)), + }; +} + function readJson(file) { if (file == null || !fs.existsSync(file)) return null; try { @@ -141,26 +249,46 @@ function namesOwner(value, identity) { } /** - * The visual result for one owner, from the gate's verdict. Added and removed - * shots are listed by key; the run's manifest names their components. + * The visual result for one owner, from pr-visual's verdict for its folder + * (visual stories are keyed by folder). Added, removed and failed shots are + * listed by key; the run's manifest names their components. `jobResult` is + * pr-visual's job result: a failed job that left no verdict is a failure, + * never a not-run. */ -export function visualResult(identity, verdict, scope, shots = {}) { - if (verdict == null) return {result: 'not-run'}; +export function visualResult( + identity, + verdict, + scope, + shots = {}, + jobResult = null, +) { if (identity.shortPackage !== 'core') { return {result: 'not-run', detail: 'no stable visual stories outside Core'}; } + const key = scope.visualKeyByOwner?.[identity.key] ?? identity.name; + if ((scope.noVisualBaseline ?? []).includes(key)) { + return {result: 'not-run', detail: 'no accepted visual baseline yet'}; + } + if (!scope.visualComponents.includes(key)) return {result: 'not-run'}; + if (verdict == null) { + return jobResult === 'failure' + ? {result: 'fail', detail: 'the visual check failed before its verdict'} + : {result: 'not-run'}; + } + if (verdict.status === 'skipped') return {result: 'not-run'}; + const componentOf = entry => + typeof entry === 'string' + ? shots[entry]?.component + : (entry?.component ?? shots[entry?.key]?.component); + const mine = entry => componentOf(entry) === key; + const failures = verdict.failures ?? []; + // A failure that names no component may be this one's. if ( - !scope.visualComponents.includes(identity.name) || - verdict.status === 'skipped' + failures.some(mine) || + failures.some(entry => componentOf(entry) == null) ) { - return {result: 'not-run'}; + return {result: 'fail', detail: 'a story image could not be captured'}; } - const mine = entry => - namesOwner( - typeof entry === 'string' ? shots[entry]?.component : entry?.component, - identity, - ); - if ((verdict.failures ?? []).some(mine)) return {result: 'fail'}; const changed = (verdict.changes ?? []).filter(mine).length + (verdict.added ?? []).filter(mine).length + @@ -197,11 +325,26 @@ const RTL_SECTIONS = [ 'directionalDecorations', ]; -/** The RTL result for one owner, across every package report. */ +/** A curated card's rollup, as one of the section verdicts. */ +const CURATED_VERDICT = { + 'RTL-ready': 'pass', + 'not-RTL': 'fail', + ERROR: 'fail', + 'MISSING-STORY': 'fail', +}; + +/** + * The RTL result for one owner, across every package report: the automatic + * sections and the curated targets' cards. + */ export function rtlResult(identity, reports) { - const results = reports.flatMap(report => - RTL_SECTIONS.flatMap(section => report?.[section]?.results ?? []), - ); + const results = reports.flatMap(report => [ + ...RTL_SECTIONS.flatMap(section => report?.[section]?.results ?? []), + ...(report?.curated?.results ?? []).map(card => ({ + component: card.component, + verdict: CURATED_VERDICT[card.rollup] ?? 'N-A', + })), + ]); const mine = results.filter(result => namesOwner(result.component, identity)); if (mine.length === 0) return {result: 'not-run'}; if (mine.every(result => result.verdict === 'N-A')) { @@ -222,6 +365,7 @@ export function buildTouchedReport({ scope, verdict, visualShots = {}, + visualJobResult = null, a11yReport, a11yBaseline, rtlReports, @@ -232,7 +376,13 @@ export function buildTouchedReport({ : []; const row = owner => { const identity = ownerIdentity(owner); - const visual = visualResult(identity, verdict, scope, visualShots); + const visual = visualResult( + identity, + verdict, + scope, + visualShots, + visualJobResult, + ); const a11y = a11yResult(identity, a11yReport, newViolations); const rtl = rtlResult(identity, rtlReports); const details = Object.entries({visual, a11y, rtl}) @@ -249,7 +399,7 @@ export function buildTouchedReport({ return { headSha, wideChange: false, - componentCount: scope.touched.length, + componentCount: scope.folders.length, broad: scope.broad, components: scope.touched.map(row), }; @@ -261,6 +411,46 @@ export function hasFailure(report) { ); } +/** + * The skipped verdict pr-visual writes for a wide change, so the pull-request + * comment names the label for the full suite. It carries the run's identity, + * as a captured verdict does, so the report publisher accepts it. + */ +export function wideVisualVerdict(decision, env = process.env) { + return { + version: 1, + status: 'skipped', + generatedAt: new Date().toISOString(), + reason: `Wide change (${decision.componentCount} components): add ci:run-visual to run the full suite.`, + context: { + sha: env.GITHUB_SHA ?? null, + runId: env.GITHUB_RUN_ID ?? null, + runAttempt: env.GITHUB_RUN_ATTEMPT ?? null, + headSha: env.ASTRYX_PR_HEAD_SHA ?? null, + baseSha: env.ASTRYX_PR_BASE_SHA ?? null, + ref: env.GITHUB_REF ?? null, + scope: 'touched', + }, + counts: { + total: 0, + unchanged: 0, + changed: 0, + added: 0, + removed: 0, + failed: 0, + }, + changes: [], + added: [], + removed: [], + failures: [], + targeting: { + unexercisedOverrides: [], + uncoveredTargets: [], + undeclaredTargets: [], + }, + }; +} + /** The record a wide change writes instead of results. */ export function wideReport({headSha, scope, decision}) { return { @@ -328,7 +518,12 @@ function writeOutputs(entries) { export function main(argv = process.argv.slice(2)) { const [command] = argv; const analysis = readJson(arg(argv, 'analysis')) ?? {}; - const scope = resolveTouchedScope(analysis); + const baselinePath = arg(argv, 'baseline-manifest'); + const resolved = resolveTouchedScope(analysis); + const scope = + baselinePath == null + ? resolved + : withVisualBaseline(resolved, readJson(baselinePath)?.shots); if (command === 'scope') { const filesPath = arg(argv, 'files'); const files = @@ -351,6 +546,14 @@ export function main(argv = process.argv.slice(2)) { fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, summary); } console.log(summary); + const wideVerdict = arg(argv, 'wide-verdict'); + if (wideVerdict != null) { + fs.mkdirSync(path.dirname(wideVerdict), {recursive: true}); + fs.writeFileSync( + wideVerdict, + `${JSON.stringify(wideVisualVerdict(decision), null, 2)}\n`, + ); + } } writeOutputs({ wide_change: String(decision.wideChange), @@ -378,6 +581,7 @@ export function main(argv = process.argv.slice(2)) { scope, verdict: readJson(arg(argv, 'visual')), visualShots: readJson(arg(argv, 'visual-manifest'))?.shots ?? {}, + visualJobResult: arg(argv, 'visual-job') ?? null, a11yReport: readJson(arg(argv, 'a11y')), a11yBaseline: readJson(arg(argv, 'a11y-baseline')), rtlReports, diff --git a/.github/scripts/touched-components.test.mjs b/.github/scripts/touched-components.test.mjs index 6946c263d96af..d9ccd524fb8e0 100644 --- a/.github/scripts/touched-components.test.mjs +++ b/.github/scripts/touched-components.test.mjs @@ -17,6 +17,8 @@ import yaml from 'yaml'; import {ungated} from './ci-pr-gate.mjs'; import { MAX_TOUCHED_COMPONENTS, + withVisualBaseline, + wideVisualVerdict, a11yResult, buildTouchedReport, decideWideChange, @@ -73,6 +75,38 @@ describe('touched scope', () => { ).toEqual([]); }); + it('expands a changed multi-component folder to its members, counted as one component', () => { + const scope = resolveTouchedScope({ + unresolvedComponentSources: ['core/Chat'], + modifiedComponentOwners: ['core/Toast'], + }); + expect(scope.touched).toContain('core/ChatComposer'); + expect(scope.touched).toContain('core/Toast'); + expect(scope.folders).toEqual(['core/Chat', 'core/Toast']); + // Visual stories are keyed by folder. + expect(scope.visualComponents).toEqual(['Chat', 'Toast']); + expect(scope.visualKeyByOwner['core/ChatComposer']).toBe('Chat'); + expect(decideWideChange(scope, []).componentCount).toBe(2); + }); + + it('compares only visual components that have an accepted baseline shot', () => { + const scope = withVisualBaseline( + resolveTouchedScope({ + modifiedComponentOwners: ['core/Toast', 'core/Brand'], + }), + { + a: {storyId: 'core-toast--default'}, + b: {storyId: 'core-forms-textarea--default'}, + }, + ); + expect(scope.visualComponents).toEqual(['Toast']); + expect(scope.noVisualBaseline).toEqual(['Brand']); + expect(visualResult(ownerIdentity('core/Brand'), null, scope)).toEqual({ + result: 'not-run', + detail: 'no accepted visual baseline yet', + }); + }); + it('names a component by package and name', () => { expect(ownerIdentity('lab/MobileTokenizer')).toEqual({ key: 'lab/MobileTokenizer', @@ -128,6 +162,34 @@ describe('wide changes', () => { expect(decideWideChange(owners(1), behaviour).wideChange).toBe(false); }); + it('writes a skipped visual verdict for a wide change, carrying the run identity', () => { + const verdict = wideVisualVerdict( + {componentCount: 7}, + { + GITHUB_SHA: 'a'.repeat(40), + GITHUB_RUN_ID: '42', + GITHUB_RUN_ATTEMPT: '1', + ASTRYX_PR_HEAD_SHA: 'b'.repeat(40), + ASTRYX_PR_BASE_SHA: 'c'.repeat(40), + }, + ); + expect(verdict).toMatchObject({ + version: 1, + status: 'skipped', + context: { + sha: 'a'.repeat(40), + runId: '42', + runAttempt: '1', + headSha: 'b'.repeat(40), + baseSha: 'c'.repeat(40), + scope: 'touched', + }, + counts: {total: 0, failed: 0}, + }); + expect(verdict.reason).toContain('Wide change (7 components)'); + expect(verdict.reason).toContain('ci:run-visual'); + }); + it('writes the wide record and points to the full-suite labels', () => { const scope = owners(6); const report = wideReport({ @@ -182,6 +244,26 @@ describe('per-component results', () => { ).result, ).toBe('fail'); expect(visualResult(button, null, scope).result).toBe('not-run'); + // A visual job that failed without a verdict is a failure, not a not-run. + expect(visualResult(button, null, scope, {}, 'failure').result).toBe( + 'fail', + ); + // A failed capture is matched to its component through the manifest, and + // a failure that names no component fails every audited one. + const failed = { + status: 'failed', + changes: [], + added: [], + removed: [], + failures: [{key: 'f1', error: 'timeout'}], + }; + expect( + visualResult(button, failed, scope, {f1: {component: 'Button'}}).result, + ).toBe('fail'); + expect( + visualResult(badge, failed, scope, {f1: {component: 'Button'}}).result, + ).toBe('pass'); + expect(visualResult(badge, failed, scope, {}).result).toBe('fail'); expect(visualResult(button, {status: 'skipped'}, scope).result).toBe( 'not-run', ); @@ -229,6 +311,22 @@ describe('per-component results', () => { expect(rtlResult(ownerIdentity('core/Toolbar'), reports).result).toBe( 'not-run', ); + // A curated target's card counts: not-RTL fails, RTL-ready passes. + expect( + rtlResult(button, [ + { + autoDiscovery: { + results: [{component: 'core/Button', verdict: 'pass'}], + }, + curated: {results: [{component: 'core/Button', rollup: 'not-RTL'}]}, + }, + ]).result, + ).toBe('fail'); + expect( + rtlResult(button, [ + {curated: {results: [{component: 'core/Button', rollup: 'RTL-ready'}]}}, + ]).result, + ).toBe('pass'); // Every check not applicable is not a pass. expect( rtlResult(ownerIdentity('core/Toast'), [ @@ -313,6 +411,45 @@ describe('pr-touched-components routing', () => { }); }); + it('skips the analysis download for a change with no components, and reads a failed pr-visual too', () => { + expect(step('Download analysis artifact').if).toBe( + "steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.docsite_only != 'true' && steps.scope.outputs.tooling_only != 'true'", + ); + expect(step("Download pr-visual's verdict").if).toContain( + "needs.pr-visual.result == 'failure'", + ); + const report = step('Report per-component results'); + expect(report.env.VISUAL_JOB).toBe('${{ needs.pr-visual.result }}'); + expect(report.run).toContain('--visual-job "$VISUAL_JOB"'); + expect(report.run).toContain( + '--baseline-manifest .github/visual-baseline/manifest.json', + ); + }); + + it("marks pr-visual's default verdict with its run identity and touched scope", () => { + const visual = ci.jobs['pr-visual']; + const visualStep = name => + visual.steps.find(candidate => candidate.name === name); + const decide = visualStep('Decide the default touched visual check'); + expect(decide.run).toContain('--wide-verdict .visual-run/verdict.json'); + expect(decide.run).toContain( + '--baseline-manifest .github/visual-baseline/manifest.json', + ); + expect(decide.env.ASTRYX_PR_HEAD_SHA).toBe( + '${{ github.event.pull_request.head.sha }}', + ); + const check = visualStep('Visual check for the default touched components'); + expect(check.env).toMatchObject({ + ASTRYX_PR_HEAD_SHA: '${{ github.event.pull_request.head.sha }}', + ASTRYX_PR_BASE_SHA: '${{ github.event.pull_request.base.sha }}', + ASTRYX_VISUAL_SCOPE: 'touched', + }); + // A scope with nothing to compare leaves no verdict instead of failing. + expect(check.run).toContain( + 'planned no shots|no stable visual representative', + ); + }); + it('exposes the wide-change flag and count', () => { expect(job.outputs).toEqual({ wide_change: '${{ steps.touched.outputs.wide_change }}', diff --git a/.github/scripts/visual-gate/gate.mjs b/.github/scripts/visual-gate/gate.mjs index ce7f47905fbf8..d1ba318473d3a 100644 --- a/.github/scripts/visual-gate/gate.mjs +++ b/.github/scripts/visual-gate/gate.mjs @@ -82,6 +82,9 @@ const captureIdentity = () => ({ runId: process.env.ASTRYX_VISUAL_RUN_ID ?? process.env.GITHUB_RUN_ID ?? null, runAttempt: process.env.ASTRYX_VISUAL_RUN_ATTEMPT ?? process.env.GITHUB_RUN_ATTEMPT ?? null, + // pr-visual's default touched check (spec:AST-030 FR12) says so, so the + // pull-request comment can name the label for the full suite. + ...(process.env.ASTRYX_VISUAL_SCOPE === 'touched' ? {scope: 'touched'} : {}), }); const config = loadConfig(REPO_ROOT); diff --git a/.github/scripts/visual-gate/publish-pr-report.mjs b/.github/scripts/visual-gate/publish-pr-report.mjs index ec2676a4d9d18..7a486854570bb 100644 --- a/.github/scripts/visual-gate/publish-pr-report.mjs +++ b/.github/scripts/visual-gate/publish-pr-report.mjs @@ -177,6 +177,8 @@ export function publishPrReport({ // The attempt that produced this evidence, which can precede the // reported attempt after a rerun of only the failed jobs. runAttempt: String(raw.context.runAttempt), + // pr-visual's default touched check; any other value is dropped. + ...(raw.context.scope === 'touched' ? {scope: 'touched'} : {}), }, baseline: {sha: text(raw.baseline?.sha ?? '')}, counts, diff --git a/.github/scripts/visual-gate/publish-pr-report.test.mjs b/.github/scripts/visual-gate/publish-pr-report.test.mjs index 74389791afec7..bef67c0173425 100644 --- a/.github/scripts/visual-gate/publish-pr-report.test.mjs +++ b/.github/scripts/visual-gate/publish-pr-report.test.mjs @@ -132,6 +132,15 @@ describe('canonical visual report publication', () => { ]); }); + it("keeps the default touched check's scope marker and drops any other", () => { + verdict.context.scope = 'touched'; + json(); + expect(run().context.scope).toBe('touched'); + verdict.context.scope = '<b>full</b>'; + json(); + expect(run().context).not.toHaveProperty('scope'); + }); + it('accepts a report from an earlier attempt of the same run, never a later one', () => { // Rerunning only the failed jobs keeps the passing visual job's artifact. verdict.context.runAttempt = '1'; diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1f9e57361409e..4515f7c04a11b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2353,16 +2353,22 @@ jobs: # only the Core components it touches, never their reverse dependents # and never the release plan; a wide change skips it (spec:AST-030 # FR12). + # A wide change writes a skipped verdict naming ci:run-visual, so the + # pull-request comment says how to run the full suite. - name: Decide the default touched visual check id: touched if: github.event_name == 'pull_request' && needs.run-labels.outputs.visual != 'true' && steps.visual-scope.outputs.has_stable_visual == 'true' env: FILES_PATH: ${{ steps.scope.outputs.changed_files_path }} + ASTRYX_PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + ASTRYX_PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} run: | node .github/scripts/touched-components.mjs scope \ --analysis analysis.json \ --files "$FILES_PATH" \ - --out touched-scope.json + --baseline-manifest .github/visual-baseline/manifest.json \ + --out touched-scope.json \ + --wide-verdict .visual-run/verdict.json - name: Download Storybook artifact if: (github.event_name == 'pull_request' || inputs.operation == 'release-check') && (github.event_name != 'pull_request' || steps.visual-scope.outputs.has_stable_visual == 'true') @@ -2492,13 +2498,18 @@ jobs: *) echo "Visual evidence is incomplete: $STATUS"; exit 1 ;; esac - # Exit 0 is clean, 2 is a change for review and 3 means nothing touched - # has a stable story (no verdict); anything else is an infrastructure - # failure. + # Only touched components with an accepted baseline shot are compared + # (one a pull request adds has nothing to compare yet). Exit 0 is clean + # and 2 a change for review. A scope that plans no shot or has no stable + # story leaves no verdict, which reads as not-run; anything else is an + # infrastructure failure. - name: Visual check for the default touched components if: steps.touched.outputs.has_components == 'true' && steps.touched.outputs.visual_components != '' env: COMPONENTS: ${{ steps.touched.outputs.visual_components }} + ASTRYX_PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + ASTRYX_PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + ASTRYX_VISUAL_SCOPE: touched run: | set +e node .github/scripts/visual-gate/gate.mjs check \ @@ -2508,11 +2519,18 @@ jobs: --tiers component \ --components "$COMPONENTS" \ --no-scout \ - --summary-output visual-summary.md + --summary-output visual-summary.md 2> >(tee visual-gate.err >&2) code=$? set -e case "$code" in 0|2) ;; + 1) + if grep -qE 'planned no shots|no stable visual representative' visual-gate.err; then + rm -f .visual-run/verdict.json + else + exit 1 + fi + ;; 3) rm -f .visual-run/verdict.json ;; *) exit "$code" ;; esac @@ -3038,7 +3056,10 @@ jobs: ref: ${{ github.event.pull_request.head.sha }} persist-credentials: false + # A spec-only, docsite-only or tooling-only change has no analysis; it + # has no component either. - name: Download analysis artifact + if: steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.docsite_only != 'true' && steps.scope.outputs.tooling_only != 'true' uses: actions/download-artifact@v8 with: name: pr-analysis @@ -3055,6 +3076,7 @@ jobs: node .github/scripts/touched-components.mjs scope \ --analysis analysis.json \ --files "$FILES_PATH" \ + --baseline-manifest .github/visual-baseline/manifest.json \ --head-sha "$HEAD_SHA" \ --out touched-components.json @@ -3129,7 +3151,7 @@ jobs: # The visual result comes from pr-visual, the one visual owner: its # default touched check, or its full check under ci:run-visual. - name: Download pr-visual's verdict - if: always() && steps.touched.outputs.has_components == 'true' && needs.pr-visual.result == 'success' + if: always() && steps.touched.outputs.has_components == 'true' && (needs.pr-visual.result == 'success' || needs.pr-visual.result == 'failure') continue-on-error: true uses: actions/download-artifact@v8 with: @@ -3140,11 +3162,14 @@ jobs: if: always() && steps.touched.outputs.has_components == 'true' env: HEAD_SHA: ${{ github.event.pull_request.head.sha }} + VISUAL_JOB: ${{ needs.pr-visual.result }} run: | node .github/scripts/touched-components.mjs report \ --analysis analysis.json \ --visual .visual-run/verdict.json \ --visual-manifest .visual-run/manifest.json \ + --visual-job "$VISUAL_JOB" \ + --baseline-manifest .github/visual-baseline/manifest.json \ --a11y a11y-report.json \ --a11y-baseline .github/a11y-baseline.json \ --rtl-dir rtl-reports \ diff --git a/docs/architecture/rtl-audit-evidence.md b/docs/architecture/rtl-audit-evidence.md index 70124d51c800e..d89954dd807a3 100644 --- a/docs/architecture/rtl-audit-evidence.md +++ b/docs/architecture/rtl-audit-evidence.md @@ -152,8 +152,10 @@ Lint and rendered audit are complementary: - **INV7 — PR scope and full-roster scope answer different questions.** The RTL audit runs after merges to `main` (a burst of pushes is checked once, at its newest commit) and before each release, at full scope; a pull request runs it - only with the `ci:run-rtl` label (spec:AST-030 FR12, DEC-7). When a pull - request runs it, it checks components affected by the change; it may run per + at its affected scope only with the `ci:run-rtl` label (spec:AST-030 FR12, + DEC-7), and otherwise audits only the components the change touches, in the + default touched-components check. When a pull request runs it with the + label, it checks components affected by the change; it may run per package, but the PR result is the join of every package scope and cannot pass with one scope missing. The full-scope run checks the live roster and shared regressions. Neither silently replaces the other. diff --git a/docs/specs/AST-030/spec.md b/docs/specs/AST-030/spec.md index 029d4f34e468e..1211128b0494c 100644 --- a/docs/specs/AST-030/spec.md +++ b/docs/specs/AST-030/spec.md @@ -166,7 +166,12 @@ they are separately named. independently routed test lane and, when applicable, one independently routed build lane MUST own them. New coverage inside an existing surface MUST join those owners and MUST NOT create a separately routed lane or required context. - Existing historical joins remain stable. A selected surface whose dedicated + Existing historical joins remain stable. The default touched-components + check (FR12) is the visual, accessibility and RTL owners' pull-request + coverage for the components a change touches: its visual part runs inside + `pr-visual`, and its accessibility and RTL part runs in one advisory job + beside them that reports no required context, so it routes no new surface + lane. A selected surface whose dedicated owner is not yet admitted continues through broad CI. The Build package's test owner includes its production CSS-layer cascade behavior. Theme-family compilation remains `theme-build` and does not become Build merely because both @@ -192,7 +197,8 @@ they are separately named. focused, smoke, and no-op paths MUST NOT narrow release checks. On a pull request, the visual (`visual-scout`, `pr-visual`), accessibility (`pr-a11y` with its guards and specs) and RTL (`pr-rtl` with its legs) owners MUST each - run only when the pull request carries that suite's label at the event: + run at their opted-in scope (FR13) only when the pull request carries that + suite's label at the event: `ci:run-visual`, `ci:run-a11y` or `ci:run-rtl`, in each case last applied by someone with write, maintain or admin access, and a lookup that cannot confirm that runs nothing. A label runs only its own suite. Without a label, a pull request update @@ -250,17 +256,20 @@ they are separately named. reads sources for it (the TypeScript compiler, a bundler) counts as reading sources; and every test that loads a changed workspace package from its build, by the package's name or a path into its `dist/`. When a pull request runs - them (FR12: only with their `ci:run-visual` or `ci:run-rtl` label), the visual and RTL owners - audit one affected set of component folders: the changed components + them at their opted-in scope (FR12: only with their `ci:run-visual` or + `ci:run-rtl` label), the visual and RTL owners audit one affected set of + component folders: the changed components and their reverse dependents, the folders of the components, in any package, whose built bundle, `import()` targets included, reaches a changed module. Visual audits each dependent folder that has a stable story, and runs the whole plan when the dependents exceed its focused budget or none has one; RTL audits every component each of those folders implements. A Changeset renders nothing and widens no audit. A changed module that no component bundle holds adds none, and without a readable build the audits run in full. PR accessibility, when it runs, stays - scoped to changed components (FR12). Shared infrastructure runs the full suite of - the owners the pull request runs (visual, accessibility and RTL only with - their `ci:run-*` labels, FR12): root + scoped to changed components (FR12). Without a label, the default + touched-components check audits only the touched components and never + widens (FR12). Shared infrastructure runs the full suite of the owners the + pull request runs (visual, accessibility and RTL only with their `ci:run-*` + labels, FR12): root dependency manifests and lockfile, root configuration, a package's manifest, build or test configuration and test setup, `internal/` (the test harness included), `theme-build` paths (tokens and themes), Storybook configuration, @@ -443,6 +452,10 @@ registers its cases with the one shared Storybook-backed PR visual owner in visual lanes; a second workflow that captures or compares those pixels is. The single-workflow wording clarifies the existing shared-system requirement; it does not admit a new owner. +The default touched-components check (FR12) is the visual, accessibility and +RTL owners' own pull-request coverage of touched components: its visual part +is `pr-visual`'s default mode, and its advisory accessibility and RTL job feeds +no required context, so it is not a new lane. Build's production CSS-layer cascade is a Build package integration contract, so it belongs to Build's test owner. Theme-family compilation remains owned by @@ -484,7 +497,7 @@ pushes to `main`), and treating a maintenance capture, skipped check, a checked SHA no longer in current `main` history, or retroactive status as release evidence. -### DEC-7 — Visual, accessibility and RTL run on main, not on pull request updates +### DEC-7 — Full visual, accessibility and RTL suites run on main; pull request updates check only touched components **Reference:** `spec:AST-030/DEC-7` **Decider:** `cixzhang`, `2026-10-28` @@ -521,8 +534,9 @@ the module graph for tests and built-bundle reach for components. Tests that read sources through the file system run whole, because no import reaches what they read. Shared infrastructure, uncertainty, `main`, merge-group entries and release checks run everything, so no landing or release gate narrows; a pull -request runs the visual, accessibility and RTL owners only with the -suite's `ci:run-*` label (DEC-7). +request runs the visual, accessibility and RTL owners at their opted-in scope +only with the suite's `ci:run-*` label, and otherwise only the +touched-components check for the components it touches (DEC-7). Rejected: a full run triggered by approval, a label or a draft-to-ready change, which tests the pull request's head rather than the merged result. From 0c8f7cf8701c063bf6d5625bca2b919a2148a0df Mon Sep 17 00:00:00 2001 From: Cindy Zhang <cindyxz@meta.com> Date: Sat, 31 Oct 2026 19:27:50 -0700 Subject: [PATCH 3/7] ci: the PR status table learns the touched visual scope; RTL runs after a failed a11y audit Main's status table (pr-status.js) replaced visual-format.js. A touched-only visual verdict now gives a result only to the components it compared (its published context carries them) and the table names ci:run-visual, with its own line for a wide change. The touched job's RTL audit runs even when the accessibility audit failed. CONTRIBUTING and AST-030 DEC-7 say where each part runs; DEC-7's opening and DEC-5's guard sentence match the check. --- .github/scripts/lib/pr-status.js | 22 +++++++-- .github/scripts/lib/pr-status.test.mjs | 49 +++++++++++++++++++ .github/scripts/touched-components.test.mjs | 4 ++ .../scripts/visual-gate/publish-pr-report.mjs | 7 ++- .../visual-gate/publish-pr-report.test.mjs | 7 ++- .github/workflows/ci.yml | 5 +- CONTRIBUTING.md | 3 +- docs/specs/AST-030/spec.md | 18 ++++--- 8 files changed, 101 insertions(+), 14 deletions(-) diff --git a/.github/scripts/lib/pr-status.js b/.github/scripts/lib/pr-status.js index ba4c358b16606..5cb935457a4ac 100644 --- a/.github/scripts/lib/pr-status.js +++ b/.github/scripts/lib/pr-status.js @@ -187,6 +187,12 @@ function componentRows({analysis, a11yReport, visualVerdict}) { keys.add(`${pkg}/${name}`); } const visualChanges = visualVerdict?.changes || []; + // pr-visual's default check compares only the touched components (spec: + // AST-030 FR12): any other component has no visual result. + const touchedOnly = visualVerdict?.context?.scope === 'touched'; + const visuallyCompared = new Set( + touchedOnly ? visualVerdict.context.components || [] : [], + ); // Main's sizes of the merge base were read (a base record or build). const baseMeasured = Boolean(analysis.baseSha) || @@ -202,7 +208,9 @@ function componentRows({analysis, a11yReport, visualVerdict}) { : 'pass' : null; const visual = - visualVerdict && visualVerdict.status !== 'skipped' + visualVerdict && + visualVerdict.status !== 'skipped' && + (!touchedOnly || visuallyCompared.has(name)) ? // A run that failed compared nothing: never a pass. visualVerdict.status === 'failed' ? 'fail' @@ -443,13 +451,21 @@ function notRunLine({jobs, a11yReport, visualVerdict}) { if (suite.id === 'a11y') return !a11yReport; return false; }); - if (notRun.length === 0) return ''; + // pr-visual's default check ran, but compared only the touched components + // (or nothing, for a wide change). + const touchedVisual = + visualVerdict?.context?.scope !== 'touched' + ? '' + : visualVerdict.status === 'skipped' + ? 'Visual did not run on this wide change: add `ci:run-visual` to run the full suite.\n\n' + : 'Visual compared only the touched components: add `ci:run-visual` to run the full suite.\n\n'; + if (notRun.length === 0) return touchedVisual; const labels = notRun.map(suite => `\`ci:run-${suite.id}\``); const list = labels.length === 1 ? labels[0] : `${labels.slice(0, -1).join(', ')} or ${labels.at(-1)}`; - return `Not run on this PR: add ${list} to run ${labels.length === 1 ? 'it' : 'them'}.\n\n`; + return `Not run on this PR: add ${list} to run ${labels.length === 1 ? 'it' : 'them'}.\n\n${touchedVisual}`; } /** The compact report body, between the heading and the footer. */ diff --git a/.github/scripts/lib/pr-status.test.mjs b/.github/scripts/lib/pr-status.test.mjs index 636d9c03659de..2cf3d132b02b4 100644 --- a/.github/scripts/lib/pr-status.test.mjs +++ b/.github/scripts/lib/pr-status.test.mjs @@ -193,6 +193,28 @@ describe('buildComponentTable', () => { expect(table).toContain('| ✗ · — · — |'); }); + it("gives a visual result only to the components pr-visual's default check compared", () => { + const sizes = { + 'core/Card': {importGzipBytes: 2048, cssGzipBytes: null}, + 'core/Menu': {importGzipBytes: 1024, cssGzipBytes: null}, + }; + const table = buildComponentTable({ + analysis: { + baseSha: 'a'.repeat(40), + componentSizes: sizes, + baseComponentSizes: sizes, + }, + origin: '', + visualVerdict: { + status: 'pass', + changes: [], + context: {scope: 'touched', components: ['Card']}, + }, + }); + expect(table).toMatch(/Card[^\n]*\| ✓ · — · — \|/); + expect(table).toMatch(/Menu[^\n]*\| — · — · — \|/); + }); + it('renders nothing when no component changed', () => { expect(buildComponentTable({analysis: {}, origin: ORIGIN})).toBe(''); }); @@ -390,6 +412,33 @@ describe('notRunLine', () => { ); }); + it("names ci:run-visual after pr-visual's default touched check, and on a wide change", () => { + const jobs = [ + {name: 'pr-visual', conclusion: 'success'}, + {name: 'pr-a11y', conclusion: 'success'}, + {name: 'pr-rtl', conclusion: 'success'}, + ]; + expect( + notRunLine({ + jobs, + visualVerdict: {status: 'pass', context: {scope: 'touched'}}, + }), + ).toBe( + 'Visual compared only the touched components: add `ci:run-visual` to run the full suite.\n\n', + ); + expect( + notRunLine({ + jobs, + visualVerdict: {status: 'skipped', context: {scope: 'touched'}}, + }), + ).toBe( + 'Visual did not run on this wide change: add `ci:run-visual` to run the full suite.\n\n', + ); + expect( + notRunLine({jobs, visualVerdict: {status: 'pass', context: {}}}), + ).toBe(''); + }); + it('leaves the line out when the Storybook build failed', () => { expect( notRunLine({ diff --git a/.github/scripts/touched-components.test.mjs b/.github/scripts/touched-components.test.mjs index d9ccd524fb8e0..b1958586ff481 100644 --- a/.github/scripts/touched-components.test.mjs +++ b/.github/scripts/touched-components.test.mjs @@ -418,6 +418,10 @@ describe('pr-touched-components routing', () => { expect(step("Download pr-visual's verdict").if).toContain( "needs.pr-visual.result == 'failure'", ); + // A failed accessibility audit does not skip RTL. + expect(step('RTL audit for the touched components').if).toMatch( + /^always\(\) && steps\.playwright\.outcome == 'success' && /, + ); const report = step('Report per-component results'); expect(report.env.VISUAL_JOB).toBe('${{ needs.pr-visual.result }}'); expect(report.run).toContain('--visual-job "$VISUAL_JOB"'); diff --git a/.github/scripts/visual-gate/publish-pr-report.mjs b/.github/scripts/visual-gate/publish-pr-report.mjs index 7a486854570bb..7ff6a69d85fb5 100644 --- a/.github/scripts/visual-gate/publish-pr-report.mjs +++ b/.github/scripts/visual-gate/publish-pr-report.mjs @@ -177,8 +177,11 @@ export function publishPrReport({ // The attempt that produced this evidence, which can precede the // reported attempt after a rerun of only the failed jobs. runAttempt: String(raw.context.runAttempt), - // pr-visual's default touched check; any other value is dropped. - ...(raw.context.scope === 'touched' ? {scope: 'touched'} : {}), + // pr-visual's default touched check and the components it compared; + // any other scope is dropped. + ...(raw.context.scope === 'touched' + ? {scope: 'touched', components: list(raw.context.components).map(text)} + : {}), }, baseline: {sha: text(raw.baseline?.sha ?? '')}, counts, diff --git a/.github/scripts/visual-gate/publish-pr-report.test.mjs b/.github/scripts/visual-gate/publish-pr-report.test.mjs index bef67c0173425..4fb83642b836d 100644 --- a/.github/scripts/visual-gate/publish-pr-report.test.mjs +++ b/.github/scripts/visual-gate/publish-pr-report.test.mjs @@ -134,11 +134,16 @@ describe('canonical visual report publication', () => { it("keeps the default touched check's scope marker and drops any other", () => { verdict.context.scope = 'touched'; + verdict.context.components = ['Button']; json(); - expect(run().context.scope).toBe('touched'); + expect(run().context).toMatchObject({ + scope: 'touched', + components: ['Button'], + }); verdict.context.scope = '<b>full</b>'; json(); expect(run().context).not.toHaveProperty('scope'); + expect(run().context).not.toHaveProperty('components'); }); it('accepts a report from an earlier attempt of the same run, never a later one', () => { diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fc6012390af85..9a9bf51ca4a28 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3069,6 +3069,7 @@ jobs: path: apps/storybook/dist - name: Install Playwright + id: playwright if: steps.touched.outputs.has_components == 'true' run: pnpm install --frozen-lockfile && npx playwright install chromium @@ -3087,8 +3088,10 @@ jobs: --concurrency 4 \ --components "$COMPONENTS" + # A failed accessibility audit must not skip RTL: each suite reports + # its own result. - name: RTL audit for the touched components - if: steps.touched.outputs.has_components == 'true' && steps.touched.outputs.rtl_packages != '' + if: always() && steps.playwright.outcome == 'success' && steps.touched.outputs.has_components == 'true' && steps.touched.outputs.rtl_packages != '' env: PACKAGES: ${{ steps.touched.outputs.rtl_packages }} OWNERS: ${{ steps.touched.outputs.rtl_owners }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index efe0134fec7aa..51242475ecc7a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -933,7 +933,8 @@ Storybook story for anything you added or altered. that suite from the next run. When the accessibility audit did not run, the pull-request comment names `ci:run-a11y`. - **Every pull request runs a touched-components check** (visual, - accessibility and RTL for only the components it changes) on one standard + accessibility and RTL for only the components it changes): its visual part + runs in the visual job, and accessibility and RTL in one job on a standard runner. It skips a wide change, one that touches more than five components or changes shared styles (themes, tokens, shared `.stylex.ts` files); add the `ci:run-*` labels for those. It is advisory: no required check waits for diff --git a/docs/specs/AST-030/spec.md b/docs/specs/AST-030/spec.md index 14c69cdac9c4d..cbddc2f344c91 100644 --- a/docs/specs/AST-030/spec.md +++ b/docs/specs/AST-030/spec.md @@ -484,9 +484,11 @@ history. Baseline capture and the reviewed pull request that versions an accepted candidate remain separate operations. -PR accessibility, when a pull request runs it (DEC-7), is a scoped, fast check: -audit only explicitly resolved changed component owners and retain the fast -modal-close, theme-var, and story-play browser guards. Never widen a PR to the exhaustive accessibility sweep because a +PR accessibility, when a pull request runs it at its opted-in scope (DEC-7), is +a scoped, fast check: audit only explicitly resolved changed component owners +and retain the fast modal-close, theme-var, and story-play browser guards. The +default touched-components check (FR12) audits the touched components' stories +without the guards, which run with the accessibility owner. Never widen a PR to the exhaustive accessibility sweep because a shared path changed or the component set is empty. Missing or approximate analysis fails the scoped check rather than silently claiming no work. Multi-component folders expand only to their own canonical component owners or existing umbrella @@ -509,12 +511,16 @@ not on every push) Run the visual, accessibility and RTL owners on every push to `main`, through the same exact-commit `release-check` the release gate uses, and before a cut; -a pull request runs each only when it opts in with that suite's `ci:run-visual`, `ci:run-a11y` or `ci:run-rtl` label. Main-push checks are throttled: one runs at a time, a +a pull request runs each at its opted-in scope only when it opts in with that +suite's `ci:run-visual`, `ci:run-a11y` or `ci:run-rtl` label, and otherwise +only the touched-components check (FR12). Main-push checks are throttled: one runs at a time, a burst of pushes collapses to its newest commit, and a running check is never cancelled, so every check that starts finishes. Pull request updates keep the fast checks and every required context, and add -the touched-components check (FR12), which audits only what the change touches -on one standard runner, so each update spends few runner-minutes while the full +the touched-components check (FR12), which audits only what the change touches, +its visual part in `pr-visual`'s default mode and its accessibility and RTL +parts in one standard-runner job, so each update spends few runner-minutes +while the full suites stay behind their labels; the modal-close, theme-var and story-play guards are steps of the accessibility owner, so they run with it, after merge or on a labelled pull request. A From 4e5ecfdd0063f265bb29e966f3120d9eba434f03 Mon Sep 17 00:00:00 2001 From: Cindy Zhang <cindyxz@meta.com> Date: Sat, 31 Oct 2026 20:39:27 -0700 Subject: [PATCH 4/7] ci: touched-components reports crashed audits and RTL gaps as failures; one folder expansion An accessibility or RTL audit that failed before writing its report now fails its owners instead of reading not-run, and an RTL coverage gap fails rather than reading "no RTL check applies". The PR status table says visual did not run (and names ci:run-visual) when pr-visual's default mode compared nothing. The changed-folder expansion is one helper, changedFolderMembers in a11y-pr-scope.mjs, used by both the accessibility scope and this check. --- .github/scripts/a11y-pr-scope.mjs | 122 ++++++++++++-------- .github/scripts/lib/pr-status.js | 18 +++ .github/scripts/lib/pr-status.test.mjs | 26 +++++ .github/scripts/touched-components.mjs | 105 ++++++++--------- .github/scripts/touched-components.test.mjs | 26 +++++ .github/workflows/ci.yml | 6 + 6 files changed, 196 insertions(+), 107 deletions(-) diff --git a/.github/scripts/a11y-pr-scope.mjs b/.github/scripts/a11y-pr-scope.mjs index 013791bc5ad90..1eaca3278ad56 100644 --- a/.github/scripts/a11y-pr-scope.mjs +++ b/.github/scripts/a11y-pr-scope.mjs @@ -66,6 +66,65 @@ export function readPrA11yRoutes(repoRoot) { })); } +/** + * The public components each changed multi-component folder (such as Chat or + * Table) implements, from the analysis's unresolved component sources. Only + * the changed folder expands, never the whole package roster. Shared by the + * accessibility scope and the touched-components check. + * + * @param {string[]} sources - `package/Folder` entries + * @param {string} [repoRoot] + * @returns {{packageName: string, directory: string, name: string, owner: string}[]} + */ +export function changedFolderMembers(sources, repoRoot = process.cwd()) { + const members = []; + const publicComponentsByPackage = new Map(); + for (const source of sources) { + const [packageName, directory, extra] = source.split('/'); + const pkg = componentPackage(packageName); + if ( + !pkg || + extra !== undefined || + !/^[A-Za-z0-9_.-]+$/.test(directory ?? '') || + ['.', '..'].includes(directory) + ) { + throw new Error(`Invalid unresolved component source: ${source}`); + } + const sourceRoot = path.join(repoRoot, pkg.src); + const changedDirectory = path.join(sourceRoot, directory); + const names = documentedComponentNames(changedDirectory); + if (names.length === 0) continue; + let publicComponents = publicComponentsByPackage.get(packageName); + if (publicComponents == null) { + publicComponents = new Set( + pkg.layout === 'flat' + ? flatPackageComponentNames(repoRoot, pkg) + : nestedPackageComponentNames(repoRoot, pkg), + ); + publicComponentsByPackage.set(packageName, publicComponents); + } + const exports = componentExportsFromBarrel( + path.join(sourceRoot, 'index.ts'), + sourceRoot, + ); + for (const name of names) { + // Utility docs in shared source directories do not create component owners. + if (!publicComponents.has(name)) continue; + // A doc can mention a dependency such as Avatar; it does not make that + // component part of the changed folder's scope. + if (exports.get(name)?.startsWith(`${changedDirectory}${path.sep}`)) { + members.push({ + packageName, + directory, + name, + owner: `${packageName}/${name}`, + }); + } + } + } + return members; +} + export function resolvePrA11yComponents( analysis, repoRoot = process.cwd(), @@ -104,56 +163,19 @@ export function resolvePrA11yComponents( } // Analysis deliberately leaves multi-component folders (such as Chat) // unresolved. Expand only that changed folder, not the whole package roster. - const publicComponentsByPackage = new Map(); - for (const source of analysis.unresolvedComponentSources) { - const [packageName, directory, extra] = source.split('/'); - const pkg = componentPackage(packageName); - if ( - !pkg || - extra !== undefined || - !/^[A-Za-z0-9_.-]+$/.test(directory ?? '') || - ['.', '..'].includes(directory) - ) { - throw new Error(`Invalid unresolved component source: ${source}`); - } - const sourceRoot = path.join(repoRoot, pkg.src); - const changedDirectory = path.join(sourceRoot, directory); - const names = documentedComponentNames(changedDirectory); - if (names.length === 0) continue; - let publicComponents = publicComponentsByPackage.get(packageName); - if (publicComponents == null) { - publicComponents = new Set( - pkg.layout === 'flat' - ? flatPackageComponentNames(repoRoot, pkg) - : nestedPackageComponentNames(repoRoot, pkg), - ); - publicComponentsByPackage.set(packageName, publicComponents); - } - const exports = componentExportsFromBarrel( - path.join(sourceRoot, 'index.ts'), - sourceRoot, - ); - for (const name of names) { - // Utility docs in shared source directories do not create component owners. - if (!publicComponents.has(name)) continue; - const implementation = exports.get(name); - // A doc can mention a dependency such as Avatar; it does not make that - // component part of the changed folder's scope. - if (implementation?.startsWith(`${changedDirectory}${path.sep}`)) { - routes ??= readPrA11yRoutes(repoRoot); - const owner = `${packageName}/${name}`; - const umbrella = `${packageName}/${directory}`; - if (componentRoutesForFilters(routes, [owner]).length > 0) { - owners.add(owner); - } else if (componentRoutesForFilters(routes, [umbrella]).length > 0) { - // Some grouped exports are exercised only by their family's stories. - owners.add(umbrella); - } else { - throw new Error( - `No scoped Storybook route for ${owner} or ${umbrella}`, - ); - } - } + for (const {packageName, directory, owner} of changedFolderMembers( + analysis.unresolvedComponentSources, + repoRoot, + )) { + routes ??= readPrA11yRoutes(repoRoot); + const umbrella = `${packageName}/${directory}`; + if (componentRoutesForFilters(routes, [owner]).length > 0) { + owners.add(owner); + } else if (componentRoutesForFilters(routes, [umbrella]).length > 0) { + // Some grouped exports are exercised only by their family's stories. + owners.add(umbrella); + } else { + throw new Error(`No scoped Storybook route for ${owner} or ${umbrella}`); } } return [...owners].sort(); diff --git a/.github/scripts/lib/pr-status.js b/.github/scripts/lib/pr-status.js index 5cb935457a4ac..ca4e325b33b05 100644 --- a/.github/scripts/lib/pr-status.js +++ b/.github/scripts/lib/pr-status.js @@ -63,6 +63,13 @@ function baseJobName(name) { * jobs was skipped by its run condition (the label was absent). A suite with * no jobs in the run is unknown (null). */ +/** Whether every job of the suite that ran succeeded (none failed). */ +function suiteSucceeded(suite, jobs) { + return (jobs || []) + .filter(job => suite.jobs.test(baseJobName(job.name))) + .every(job => ['success', 'skipped'].includes(job.conclusion)); +} + function suiteRunState(suite, jobs) { const own = (jobs || []).filter(job => suite.jobs.test(baseJobName(job.name)), @@ -445,6 +452,17 @@ function notRunLine({jobs, a11yReport, visualVerdict}) { if (build && ['failure', 'cancelled'].includes(build.conclusion)) return ''; const notRun = SUITES.filter(suite => { const state = suiteRunState(suite, jobs); + // pr-visual's default check runs on every pull request; when it compared + // nothing (no touched component has a baseline shot) it leaves no + // verdict, and visual did not run. + if ( + suite.id === 'visual' && + state === 'ran' && + !visualVerdict && + suiteSucceeded(suite, jobs) + ) { + return true; + } if (state !== null) return state === 'not-run'; if (jobs) return false; if (suite.id === 'visual') return !visualVerdict; diff --git a/.github/scripts/lib/pr-status.test.mjs b/.github/scripts/lib/pr-status.test.mjs index 2cf3d132b02b4..540bb3fdcb285 100644 --- a/.github/scripts/lib/pr-status.test.mjs +++ b/.github/scripts/lib/pr-status.test.mjs @@ -392,6 +392,7 @@ describe('notRunLine', () => { {name: 'Stable visual regression', conclusion: 'success'}, {name: 'pr-a11y', conclusion: 'success'}, ], + visualVerdict: {status: 'pass'}, }), ).toBe('Not run on this PR: add `ci:run-rtl` to run it.\n\n'); }); @@ -439,6 +440,31 @@ describe('notRunLine', () => { ).toBe(''); }); + it('names ci:run-visual when pr-visual succeeded but compared nothing', () => { + expect( + notRunLine({ + jobs: [ + {name: 'pr-visual', conclusion: 'success'}, + skipped('Stable visual scout (1/4)'), + {name: 'pr-a11y', conclusion: 'success'}, + {name: 'pr-rtl', conclusion: 'success'}, + ], + visualVerdict: null, + }), + ).toBe('Not run on this PR: add `ci:run-visual` to run it.\n\n'); + // A failed pr-visual is GitHub's to show, not a not-run. + expect( + notRunLine({ + jobs: [ + {name: 'pr-visual', conclusion: 'failure'}, + {name: 'pr-a11y', conclusion: 'success'}, + {name: 'pr-rtl', conclusion: 'success'}, + ], + visualVerdict: null, + }), + ).toBe(''); + }); + it('leaves the line out when the Storybook build failed', () => { expect( notRunLine({ diff --git a/.github/scripts/touched-components.mjs b/.github/scripts/touched-components.mjs index 5d488dd7e069a..4c5e1c2fc9ded 100644 --- a/.github/scripts/touched-components.mjs +++ b/.github/scripts/touched-components.mjs @@ -17,17 +17,10 @@ import fs from 'node:fs'; import path from 'node:path'; import {pathToFileURL} from 'node:url'; import {createRequire} from 'node:module'; -import componentPackages from '../../scripts/component-packages.cjs'; +import {changedFolderMembers} from './a11y-pr-scope.mjs'; const require = createRequire(import.meta.url); const {diffAgainstBaseline} = require('./lib/a11y-baseline.js'); -const { - componentPackage, - componentExportsFromBarrel, - documentedComponentNames, - flatPackageComponentNames, - nestedPackageComponentNames, -} = componentPackages; const SUITES = ['visual', 'a11y', 'rtl']; @@ -94,50 +87,14 @@ const strings = value => /** * The public components a changed multi-component folder (such as Chat or - * Table) implements. The analysis leaves those folders unresolved; this - * expands only the changed folder, as the accessibility scope does, without - * reading Storybook. + * Table) implements, through the accessibility scope's own expansion, with + * the folder each one belongs to. */ export function expandFolderOwners(sources, repoRoot = process.cwd()) { - const members = []; - const publicByPackage = new Map(); - for (const source of sources) { - const [shortPackage, directory, extra] = source.split('/'); - const pkg = componentPackage(shortPackage); - if ( - !pkg || - extra !== undefined || - !/^[A-Za-z0-9_.-]+$/.test(directory ?? '') - ) { - continue; - } - const sourceRoot = path.join(repoRoot, pkg.src); - const changedDirectory = path.join(sourceRoot, directory); - if (!fs.existsSync(changedDirectory)) continue; - const names = documentedComponentNames(changedDirectory); - if (names.length === 0) continue; - if (!publicByPackage.has(shortPackage)) { - publicByPackage.set( - shortPackage, - new Set( - pkg.layout === 'flat' - ? flatPackageComponentNames(repoRoot, pkg) - : nestedPackageComponentNames(repoRoot, pkg), - ), - ); - } - const exports = componentExportsFromBarrel( - path.join(sourceRoot, 'index.ts'), - sourceRoot, - ); - for (const name of names) { - if (!publicByPackage.get(shortPackage).has(name)) continue; - if (exports.get(name)?.startsWith(`${changedDirectory}${path.sep}`)) { - members.push({owner: `${shortPackage}/${name}`, folder: directory}); - } - } - } - return members; + return changedFolderMembers(sources, repoRoot).map(member => ({ + owner: member.owner, + folder: member.directory, + })); } /** @@ -299,9 +256,24 @@ export function visualResult( return {result: 'pass'}; } -/** The accessibility result for one owner: new violations fail it. */ -export function a11yResult(identity, report, newViolations) { - if (report == null) return {result: 'not-run'}; +/** + * The accessibility result for one owner: new violations fail it, and an + * audit that failed before writing its report fails every owner. + */ +export function a11yResult( + identity, + report, + newViolations, + stepOutcome = null, +) { + if (report == null) { + return stepOutcome === 'failure' + ? { + result: 'fail', + detail: 'the accessibility audit failed before its report', + } + : {result: 'not-run'}; + } const audited = Object.keys(report.components ?? {}).some(component => namesOwner(component, identity), ); @@ -337,7 +309,7 @@ const CURATED_VERDICT = { * The RTL result for one owner, across every package report: the automatic * sections and the curated targets' cards. */ -export function rtlResult(identity, reports) { +export function rtlResult(identity, reports, stepOutcome = null) { const results = reports.flatMap(report => [ ...RTL_SECTIONS.flatMap(section => report?.[section]?.results ?? []), ...(report?.curated?.results ?? []).map(card => ({ @@ -346,7 +318,22 @@ export function rtlResult(identity, reports) { })), ]); const mine = results.filter(result => namesOwner(result.component, identity)); - if (mine.length === 0) return {result: 'not-run'}; + // The audit's own coverage: no applicable dimension and no verified-N/A + // reason is a gap, which the audit signals as a failure. + const coverage = reports + .flatMap(report => report?.coverage?.results ?? []) + .find(entry => namesOwner(entry.component, identity)); + if ( + coverage?.status === 'coverage-gap' || + coverage?.status === 'stale-verified-na' + ) { + return {result: 'fail', detail: `RTL ${coverage.status}`}; + } + if (mine.length === 0) { + return stepOutcome === 'failure' + ? {result: 'fail', detail: 'the RTL audit failed before its report'} + : {result: 'not-run'}; + } if (mine.every(result => result.verdict === 'N-A')) { return {result: 'not-run', detail: 'no RTL check applies'}; } @@ -367,6 +354,8 @@ export function buildTouchedReport({ visualShots = {}, visualJobResult = null, a11yReport, + a11yStep = null, + rtlStep = null, a11yBaseline, rtlReports, }) { @@ -383,8 +372,8 @@ export function buildTouchedReport({ visualShots, visualJobResult, ); - const a11y = a11yResult(identity, a11yReport, newViolations); - const rtl = rtlResult(identity, rtlReports); + const a11y = a11yResult(identity, a11yReport, newViolations, a11yStep); + const rtl = rtlResult(identity, rtlReports, rtlStep); const details = Object.entries({visual, a11y, rtl}) .filter(([, value]) => value.detail) .map(([suite, value]) => `${suite}: ${value.detail}`); @@ -583,6 +572,8 @@ export function main(argv = process.argv.slice(2)) { visualShots: readJson(arg(argv, 'visual-manifest'))?.shots ?? {}, visualJobResult: arg(argv, 'visual-job') ?? null, a11yReport: readJson(arg(argv, 'a11y')), + a11yStep: arg(argv, 'a11y-step') ?? null, + rtlStep: arg(argv, 'rtl-step') ?? null, a11yBaseline: readJson(arg(argv, 'a11y-baseline')), rtlReports, }); diff --git a/.github/scripts/touched-components.test.mjs b/.github/scripts/touched-components.test.mjs index b1958586ff481..7e93c9951e57b 100644 --- a/.github/scripts/touched-components.test.mjs +++ b/.github/scripts/touched-components.test.mjs @@ -280,6 +280,12 @@ describe('per-component results', () => { }); expect(a11yResult(badge, report, newViolations).result).toBe('pass'); expect(a11yResult(tokenizer, report, newViolations).result).toBe('not-run'); + // An audit that failed before writing its report fails every owner. + expect(a11yResult(button, null, [], 'failure')).toEqual({ + result: 'fail', + detail: 'the accessibility audit failed before its report', + }); + expect(a11yResult(button, null, [], 'success').result).toBe('not-run'); expect(a11yResult(button, null, []).result).toBe('not-run'); }); @@ -311,6 +317,22 @@ describe('per-component results', () => { expect(rtlResult(ownerIdentity('core/Toolbar'), reports).result).toBe( 'not-run', ); + // The audit's coverage gap fails, and an audit that crashed before its + // report fails rather than reading not-run. + expect( + rtlResult(button, [ + { + autoDiscovery: { + results: [{component: 'core/Button', verdict: 'N-A'}], + }, + coverage: { + results: [{component: 'core/Button', status: 'coverage-gap'}], + }, + }, + ]), + ).toEqual({result: 'fail', detail: 'RTL coverage-gap'}); + expect(rtlResult(button, [], 'failure').result).toBe('fail'); + expect(rtlResult(button, [], 'success').result).toBe('not-run'); // A curated target's card counts: not-RTL fails, RTL-ready passes. expect( rtlResult(button, [ @@ -425,6 +447,10 @@ describe('pr-touched-components routing', () => { const report = step('Report per-component results'); expect(report.env.VISUAL_JOB).toBe('${{ needs.pr-visual.result }}'); expect(report.run).toContain('--visual-job "$VISUAL_JOB"'); + expect(report.env.A11Y_STEP).toBe('${{ steps.a11y.outcome }}'); + expect(report.env.RTL_STEP).toBe('${{ steps.rtl.outcome }}'); + expect(report.run).toContain('--a11y-step "$A11Y_STEP"'); + expect(report.run).toContain('--rtl-step "$RTL_STEP"'); expect(report.run).toContain( '--baseline-manifest .github/visual-baseline/manifest.json', ); diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9a9bf51ca4a28..45c49f9676f47 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3075,6 +3075,7 @@ jobs: # The same changed-component filters an opted-in pull request uses. - name: Accessibility audit for the touched components + id: a11y if: steps.touched.outputs.has_components == 'true' && steps.touched.outputs.a11y_owners != '' run: | COMPONENTS=$(node .github/scripts/a11y-pr-scope.mjs analysis.json) @@ -3091,6 +3092,7 @@ jobs: # A failed accessibility audit must not skip RTL: each suite reports # its own result. - name: RTL audit for the touched components + id: rtl if: always() && steps.playwright.outcome == 'success' && steps.touched.outputs.has_components == 'true' && steps.touched.outputs.rtl_packages != '' env: PACKAGES: ${{ steps.touched.outputs.rtl_packages }} @@ -3129,6 +3131,8 @@ jobs: env: HEAD_SHA: ${{ github.event.pull_request.head.sha }} VISUAL_JOB: ${{ needs.pr-visual.result }} + A11Y_STEP: ${{ steps.a11y.outcome }} + RTL_STEP: ${{ steps.rtl.outcome }} run: | node .github/scripts/touched-components.mjs report \ --analysis analysis.json \ @@ -3138,7 +3142,9 @@ jobs: --baseline-manifest .github/visual-baseline/manifest.json \ --a11y a11y-report.json \ --a11y-baseline .github/a11y-baseline.json \ + --a11y-step "$A11Y_STEP" \ --rtl-dir rtl-reports \ + --rtl-step "$RTL_STEP" \ --head-sha "$HEAD_SHA" \ --out touched-components.json From 2beb9a34bb78b4a9124f2376c055467b95778dab Mon Sep 17 00:00:00 2001 From: Cindy Zhang <cindyxz@meta.com> Date: Sat, 31 Oct 2026 21:31:25 -0700 Subject: [PATCH 5/7] ci: the published visual verdict keeps only the touched scope marker Main's PR report (#9217) reads the touched-components record for per-component results; this branch's own status-table edits were dropped in the merge, so the verdict's compared-components list has no reader. --- .github/scripts/visual-gate/publish-pr-report.mjs | 7 ++----- .github/scripts/visual-gate/publish-pr-report.test.mjs | 7 +------ 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/.github/scripts/visual-gate/publish-pr-report.mjs b/.github/scripts/visual-gate/publish-pr-report.mjs index 7ff6a69d85fb5..52d4fe9803617 100644 --- a/.github/scripts/visual-gate/publish-pr-report.mjs +++ b/.github/scripts/visual-gate/publish-pr-report.mjs @@ -177,11 +177,8 @@ export function publishPrReport({ // The attempt that produced this evidence, which can precede the // reported attempt after a rerun of only the failed jobs. runAttempt: String(raw.context.runAttempt), - // pr-visual's default touched check and the components it compared; - // any other scope is dropped. - ...(raw.context.scope === 'touched' - ? {scope: 'touched', components: list(raw.context.components).map(text)} - : {}), + // pr-visual's default touched check; any other scope is dropped. + ...(raw.context.scope === 'touched' ? {scope: 'touched'} : {}), }, baseline: {sha: text(raw.baseline?.sha ?? '')}, counts, diff --git a/.github/scripts/visual-gate/publish-pr-report.test.mjs b/.github/scripts/visual-gate/publish-pr-report.test.mjs index 4fb83642b836d..bef67c0173425 100644 --- a/.github/scripts/visual-gate/publish-pr-report.test.mjs +++ b/.github/scripts/visual-gate/publish-pr-report.test.mjs @@ -134,16 +134,11 @@ describe('canonical visual report publication', () => { it("keeps the default touched check's scope marker and drops any other", () => { verdict.context.scope = 'touched'; - verdict.context.components = ['Button']; json(); - expect(run().context).toMatchObject({ - scope: 'touched', - components: ['Button'], - }); + expect(run().context.scope).toBe('touched'); verdict.context.scope = '<b>full</b>'; json(); expect(run().context).not.toHaveProperty('scope'); - expect(run().context).not.toHaveProperty('components'); }); it('accepts a report from an earlier attempt of the same run, never a later one', () => { From 555e6f3b52cf5048ea6100f6e804f76fcbe72328 Mon Sep 17 00:00:00 2001 From: Cindy Zhang <cindyxz@meta.com> Date: Sat, 31 Oct 2026 22:04:48 -0700 Subject: [PATCH 6/7] ci: touched accessibility counts a folder's umbrella and pattern-story violations The audit files a multi-component folder's violations under its umbrella story owner (core/Chat) or a member's pattern story owner (core/ChatSendButton). Audited owners now come from the report's auditedStories; an umbrella violation fails every touched member of its folder, and an audited owner outside the touched set gets its own row, so a new violation can never read pass or not-run. Tested with the audit's real report shape through diffAgainstBaseline. --- .github/scripts/touched-components.mjs | 57 +++++++++++++++---- .github/scripts/touched-components.test.mjs | 61 +++++++++++++++++++++ 2 files changed, 108 insertions(+), 10 deletions(-) diff --git a/.github/scripts/touched-components.mjs b/.github/scripts/touched-components.mjs index 4c5e1c2fc9ded..561f6526d4bf2 100644 --- a/.github/scripts/touched-components.mjs +++ b/.github/scripts/touched-components.mjs @@ -257,14 +257,31 @@ export function visualResult( } /** - * The accessibility result for one owner: new violations fail it, and an - * audit that failed before writing its report fails every owner. + * The owners the accessibility audit checked, from its report's audited + * stories: a touched folder member, a multi-component folder's umbrella + * (`core/Chat`) or a member's pattern story owner (`core/ChatSendButton`). + */ +export function auditedA11yOwners(report) { + return [ + ...new Set( + (Array.isArray(report?.auditedStories) ? report.auditedStories : []) + .map(entry => entry?.owner) + .filter(owner => typeof owner === 'string' && owner.includes('/')), + ), + ].sort(); +} + +/** + * The accessibility result for one owner. Its own stories and its folder's + * umbrella stories count: a new violation in either fails it, and an audit + * that failed before writing its report fails every owner. */ export function a11yResult( identity, report, newViolations, stepOutcome = null, + scope = null, ) { if (report == null) { return stepOutcome === 'failure' @@ -274,13 +291,19 @@ export function a11yResult( } : {result: 'not-run'}; } - const audited = Object.keys(report.components ?? {}).some(component => - namesOwner(component, identity), - ); + const folder = scope?.visualKeyByOwner?.[identity.key]; + const umbrella = + folder != null && folder !== identity.name + ? `${identity.shortPackage}/${folder}` + : null; + const counts = owner => namesOwner(owner, identity) || owner === umbrella; + const audited = + auditedA11yOwners(report).some(counts) || + Object.keys(report.components ?? {}).some(component => + namesOwner(component, identity), + ); if (!audited) return {result: 'not-run'}; - const mine = newViolations.filter(violation => - namesOwner(violation.component, identity), - ); + const mine = newViolations.filter(violation => counts(violation.component)); return mine.length > 0 ? { result: 'fail', @@ -372,7 +395,13 @@ export function buildTouchedReport({ visualShots, visualJobResult, ); - const a11y = a11yResult(identity, a11yReport, newViolations, a11yStep); + const a11y = a11yResult( + identity, + a11yReport, + newViolations, + a11yStep, + scope, + ); const rtl = rtlResult(identity, rtlReports, rtlStep); const details = Object.entries({visual, a11y, rtl}) .filter(([, value]) => value.detail) @@ -390,7 +419,15 @@ export function buildTouchedReport({ wideChange: false, componentCount: scope.folders.length, broad: scope.broad, - components: scope.touched.map(row), + // An owner the audit checked outside the touched set (a folder's + // umbrella or a member's pattern story) gets its own row, so none of its + // violations is dropped. + components: [ + ...scope.touched, + ...auditedA11yOwners(a11yReport).filter( + owner => !scope.touched.includes(owner), + ), + ].map(row), }; } diff --git a/.github/scripts/touched-components.test.mjs b/.github/scripts/touched-components.test.mjs index 7e93c9951e57b..11723bd4c1c08 100644 --- a/.github/scripts/touched-components.test.mjs +++ b/.github/scripts/touched-components.test.mjs @@ -17,6 +17,7 @@ import yaml from 'yaml'; import {ungated} from './ci-pr-gate.mjs'; import { MAX_TOUCHED_COMPONENTS, + auditedA11yOwners, withVisualBaseline, wideVisualVerdict, a11yResult, @@ -89,6 +90,66 @@ describe('touched scope', () => { expect(decideWideChange(scope, []).componentCount).toBe(2); }); + it("counts a folder's umbrella and pattern-story violations, from the audit's real report shape", () => { + const scope = resolveTouchedScope({ + unresolvedComponentSources: ['core/Chat'], + }); + // The audit files a Chat change under the folder's umbrella story and a + // member's pattern story, neither of which is a documented member. + const a11yReport = { + auditedStories: [ + {owner: 'core/Chat', storyId: 'core-chat--default'}, + {owner: 'core/ChatSendButton', storyId: 'core-chat-patterns--send'}, + ], + components: { + Chat: { + storiesAudited: 2, + violations: [], + storyDetails: [ + { + story: 'Default', + storyId: 'core-chat--default', + violations: [ + {id: 'button-name', impact: 'critical', nodes: [{}]}, + ], + }, + { + story: 'Send', + storyId: 'core-chat-patterns--send', + violations: [ + {id: 'color-contrast', impact: 'serious', nodes: [{}]}, + ], + }, + ], + }, + }, + }; + expect(auditedA11yOwners(a11yReport)).toEqual([ + 'core/Chat', + 'core/ChatSendButton', + ]); + const report = buildTouchedReport({ + headSha: 'a'.repeat(40), + scope, + verdict: null, + a11yReport, + a11yBaseline: {}, + rtlReports: [], + }); + const byKey = Object.fromEntries( + report.components.map(component => [component.key, component]), + ); + // The umbrella's violation fails every touched member of the folder. + expect(byKey['core/ChatComposer'].results.a11y).toBe('fail'); + expect(byKey['core/ChatComposer'].detail).toContain('button-name'); + // A pattern-story owner outside the touched set gets its own row. + expect(byKey['core/ChatSendButton'].results.a11y).toBe('fail'); + expect(byKey['core/ChatSendButton'].detail).toContain('color-contrast'); + expect(hasFailure(report)).toBe(true); + // The wide rule still counts the folder once. + expect(report.componentCount).toBe(1); + }); + it('compares only visual components that have an accepted baseline shot', () => { const scope = withVisualBaseline( resolveTouchedScope({ From 1b51112b3aa4309d170b593d4efbea0fc75ac8e6 Mon Sep 17 00:00:00 2001 From: Cindy Zhang <cindyxz@meta.com> Date: Sat, 31 Oct 2026 23:52:06 -0700 Subject: [PATCH 7/7] ci: a partial curated RTL card fails; the visual-gate README and release-check comment match A curated card whose rollup is partial (some dimensions hold, others do not) now fails its owner instead of reading N/A. The visual-gate README says a pull request runs pr-visual's full check with ci:run-visual and its default touched check otherwise, and that the touched accessibility audit runs without the guards. The release-check comment moves back above its job. --- .github/scripts/touched-components.mjs | 3 +++ .github/scripts/touched-components.test.mjs | 6 ++++++ .github/scripts/visual-gate/README.md | 14 +++++++++----- .github/workflows/ci.yml | 4 ++-- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/.github/scripts/touched-components.mjs b/.github/scripts/touched-components.mjs index 561f6526d4bf2..982c277327743 100644 --- a/.github/scripts/touched-components.mjs +++ b/.github/scripts/touched-components.mjs @@ -323,6 +323,9 @@ const RTL_SECTIONS = [ /** A curated card's rollup, as one of the section verdicts. */ const CURATED_VERDICT = { 'RTL-ready': 'pass', + // Some dimensions hold and others do not (prev and next ordered in LTR but + // not mirrored in RTL): a failure. + partial: 'fail', 'not-RTL': 'fail', ERROR: 'fail', 'MISSING-STORY': 'fail', diff --git a/.github/scripts/touched-components.test.mjs b/.github/scripts/touched-components.test.mjs index 11723bd4c1c08..5288a75a69e00 100644 --- a/.github/scripts/touched-components.test.mjs +++ b/.github/scripts/touched-components.test.mjs @@ -394,6 +394,12 @@ describe('per-component results', () => { ).toEqual({result: 'fail', detail: 'RTL coverage-gap'}); expect(rtlResult(button, [], 'failure').result).toBe('fail'); expect(rtlResult(button, [], 'success').result).toBe('not-run'); + // A partial curated card (some dimensions hold, others do not) fails. + expect( + rtlResult(button, [ + {curated: {results: [{component: 'core/Button', rollup: 'partial'}]}}, + ]).result, + ).toBe('fail'); // A curated target's card counts: not-RTL fails, RTL-ready passes. expect( rtlResult(button, [ diff --git a/.github/scripts/visual-gate/README.md b/.github/scripts/visual-gate/README.md index a70b86b278d6b..acec25158d2c3 100644 --- a/.github/scripts/visual-gate/README.md +++ b/.github/scripts/visual-gate/README.md @@ -3,8 +3,10 @@ Every visual-regression test uses the shared Storybook framework and `.github/workflows/ci.yml` → `pr-visual` (**Stable visual regression**). It runs in `release-check`, which `main-heavy-lanes.yml` dispatches after merges to -`main` (a burst is checked once, at its newest commit) and the release gate dispatches before a cut; a pull request runs it only -with the `ci:run-visual` label (AST-030 FR12, DEC-7). +`main` (a burst is checked once, at its newest commit) and the release gate dispatches before a cut. A pull request runs its full +check only with the `ci:run-visual` label; without it, `pr-visual` runs its +default touched check, which compares only the touched Core components that +have an accepted baseline shot and never widens (AST-030 FR12, DEC-7). [AST-030 FR12](../../../docs/specs/AST-030/spec.md) owns this requirement. Add coverage to that owner, not to a new component, package, theme, scheduled, post-CI, or post-merge workflow. @@ -30,9 +32,11 @@ findings remain soft while report completeness is mandatory. The dispatch does not capture baseline candidates, promote baselines, or publish previews. `visual-release-report` holds the visual evidence for that exact run/attempt. -PR accessibility is intentionally smaller: explicit changed-component axe scope -(including grouped-folder owners), plus the fast modal-close, theme-var, and -story-play guards. Empty/shared scope never falls back to all stories, and +PR accessibility is intentionally smaller. With `ci:run-a11y` it runs an +explicit changed-component axe scope (including grouped-folder owners), plus +the fast modal-close, theme-var, and story-play guards. Without the label, the +default touched-components check audits only the touched components' stories, +without the guards. Empty/shared scope never falls back to all stories, and missing or approximate analysis fails rather than granting an empty scope. The full axe roster, whole-repository accessibility spec-test contracts and their pixel/evidence uploads run only during `release-check`. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 39c99c2455a49..20cf315c2d106 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2999,8 +2999,6 @@ jobs: --force-full "${{ steps.scope.outputs.force_full_component_audits }}" \ --reports-dir rtl-shard-reports - # Release-time projection only: the existing owners above do all measurement. - # A skipped owner or main drift must never look like passing release evidence. # The default pull-request check for components: accessibility and RTL for # only the components the change touches, on one standard runner, against # this pull request's Storybook build, with pr-visual's verdict for the same @@ -3185,6 +3183,8 @@ jobs: if-no-files-found: ignore retention-days: 7 + # Release-time projection only: the existing owners above do all measurement. + # A skipped owner or main drift must never look like passing release evidence. release-check: needs: [