Distinguish wcag vs best practice (#233)

Closes #34 (which closes github/accessibility#10749)

## Problem
Every finding was filed as if it were a hard WCAG failure, with no way to differentiate definitive violations and best-practice or experimental
checks. There was also no way to stop filing the non-WCAG ones.

## Changes
- Classify each Axe finding as `wcag`, `best-practice`, or `experimental`
- Surface non-WCAG findings in the filed issue: a "**Note:** …not a hard WCAG failure" line in the body and a `best-practice`/`experimental`label.
- Add two action inputs, `file_best_practice_issues` and`file_experimental_issues` (default `true`), to skip filing those categories

Author: kzhou314

.github/actions/file/action.yml

@@ -28,6 +28,14 @@ inputs:
     description: "When true, log the issues that would be filed without opening, closing, or reopening any issues."
     required: false
     default: "false"
+  file_best_practice_issues:
+    description: "File issues for best-practice findings (accessibility recommendations that are not hard WCAG failures). Disabling only suppresses new issues; existing ones are left untouched."
+    required: false
+    default: "true"
+  file_experimental_issues:
+    description: "File issues for experimental findings (checks that are not yet stable). Disabling only suppresses new issues; existing ones are left untouched."
+    required: false
+    default: "true"
 
 outputs:
   filings_file:

.github/actions/file/src/generateIssueBody.ts

@@ -18,13 +18,25 @@ export function generateIssueBody(finding: Finding, screenshotRepo: string): str
 `
   }
 
+  const categoryNotice =
+    finding.category && finding.category !== 'wcag'
+      ? `> [!NOTE]\n> This is ${
+          finding.category === 'experimental' ? 'an experimental check' : 'a best-practice recommendation'
+        }, not a definite WCAG failure.\n\n`
+      : ''
+
+  const standardsLine =
+    finding.category && finding.category !== 'wcag'
+      ? '- [ ] The fix MUST meet the accessibility standards specified by the repository or organization (WCAG 2.2 if applicable).'
+      : '- [ ] The fix MUST meet WCAG 2.2 guidelines OR the accessibility standards specified by the repository or organization.'
+
   const acceptanceCriteria = `## Acceptance Criteria
 - [ ] The specific violation reported in this issue is no longer reproducible.
-- [ ] The fix MUST meet WCAG 2.1 guidelines OR the accessibility standards specified by the repository or organization.
+${standardsLine}
 - [ ] A test SHOULD be added to ensure this specific violation does not regress.
 - [ ] This PR MUST NOT introduce any new accessibility issues or regressions.`
 
-  const body = `## What
+  const body = `${categoryNotice}## What
 An accessibility scan ${finding.html ? `flagged the element \`${finding.html}\`` : `found an issue on ${finding.url}`} because ${finding.problemShort}. Learn more about why this was flagged by visiting ${finding.problemUrl}.
 
 ${screenshotSection ?? ''}

.github/actions/file/src/index.ts

@@ -17,6 +17,12 @@ import {updateFilingsWithNewFindings} from './updateFilingsWithNewFindings.js'
 import {OctokitResponse} from '@octokit/types'
 const OctokitWithThrottling = Octokit.plugin(throttling)
 
+// core.getBooleanInput throws on unset inputs, so apply the default first.
+function getBooleanInputWithDefault(name: string, defaultValue: boolean): boolean {
+  if (!core.getInput(name)) return defaultValue
+  return core.getBooleanInput(name)
+}
+
 export default async function () {
   core.info("Started 'file' action")
   const findingsFile = core.getInput('findings_file', {required: true})
@@ -31,13 +37,17 @@ export default async function () {
     : []
   const shouldOpenGroupedIssues = core.getBooleanInput('open_grouped_issues')
   const dryRun = core.getBooleanInput('dry_run')
+  const fileBestPracticeIssues = getBooleanInputWithDefault('file_best_practice_issues', true)
+  const fileExperimentalIssues = getBooleanInputWithDefault('file_experimental_issues', true)
   core.debug(`Input: 'findings_file: ${findingsFile}'`)
   core.debug(`Input: 'repository: ${repoWithOwner}'`)
   core.debug(`Input: 'base_url: ${baseUrl ?? '(default)'}'`)
   core.debug(`Input: 'screenshot_repository: ${screenshotRepo}'`)
   core.debug(`Input: 'cached_filings_file: ${cachedFilingsFile}'`)
   core.debug(`Input: 'open_grouped_issues: ${shouldOpenGroupedIssues}'`)
   core.debug(`Input: 'dry_run: ${dryRun}'`)
+  core.debug(`Input: 'file_best_practice_issues: ${fileBestPracticeIssues}'`)
+  core.debug(`Input: 'file_experimental_issues: ${fileExperimentalIssues}'`)
 
   const octokit = new OctokitWithThrottling({
     auth: token,
@@ -61,6 +71,9 @@ export default async function () {
   })
   const filings = updateFilingsWithNewFindings(cachedFilings, findings)
 
+  // Suppressed new filings are kept out of the cache
+  const suppressedFilings = new Set<Filing>()
+
   // Fetch closed wontfix issues once up front; a failed fetch reopens as usual
   let wontfixIssueNumbers = new Set<number>()
   if (!dryRun) {
@@ -80,6 +93,21 @@ export default async function () {
   for (const filing of filings) {
     let response: OctokitResponse<IssueResponse> | undefined
     try {
+      // Category switches gate only new issues
+      if (isNewFiling(filing)) {
+        const category = filing.findings[0].category ?? 'wcag'
+        if (
+          (category === 'best-practice' && !fileBestPracticeIssues) ||
+          (category === 'experimental' && !fileExperimentalIssues)
+        ) {
+          core.info(
+            `Skipping new ${category} issue (filing disabled for this category): ${filing.findings[0].problemShort}`,
+          )
+          suppressedFilings.add(filing)
+          continue
+        }
+      }
+
       if (dryRun) {
         if (isResolvedFiling(filing)) {
           dryRunCounts.close++
@@ -182,7 +210,8 @@ export default async function () {
   }
 
   const filingsPath = path.join(process.env.RUNNER_TEMP || '/tmp', `filings-${crypto.randomUUID()}.json`)
-  fs.writeFileSync(filingsPath, JSON.stringify(filings))
+  const outputFilings = suppressedFilings.size > 0 ? filings.filter(f => !suppressedFilings.has(f)) : filings
+  fs.writeFileSync(filingsPath, JSON.stringify(outputFilings))
   core.setOutput('filings_file', filingsPath)
 
   core.debug(`Output: 'filings_file: ${filingsPath}'`)

.github/actions/file/src/openIssue.ts

@@ -26,6 +26,10 @@ export async function openIssue(octokit: Octokit, repoWithOwner: string, finding
   if (finding.ruleId) {
     labels.push(`${finding.scannerType} rule: ${finding.ruleId}`)
   }
+  // Flag non-WCAG findings so they can be filtered or triaged separately
+  if (finding.category && finding.category !== 'wcag') {
+    labels.push(finding.category)
+  }
 
   const title = truncateWithEllipsis(
     `Accessibility issue: ${finding.problemShort[0].toUpperCase() + finding.problemShort.slice(1)} on ${new URL(finding.url).pathname}`,

.github/actions/file/src/types.d.ts

@@ -1,5 +1,8 @@
+export type FindingCategory = 'wcag' | 'best-practice' | 'experimental'
+
 export type Finding = {
   scannerType: string
+  category?: FindingCategory
   ruleId?: string
   url: string
   html?: string

.github/actions/file/tests/generateIssueBody.test.ts

@@ -26,6 +26,7 @@ describe('generateIssueBody', () => {
     expect(body).toContain('## What')
     expect(body).toContain('## Acceptance Criteria')
     expect(body).toContain('The specific violation reported in this issue is no longer reproducible.')
+    expect(body).toContain('The fix MUST meet WCAG 2.2 guidelines OR')
     expect(body).not.toContain('Specifically:')
   })
 
@@ -76,4 +77,31 @@ describe('generateIssueBody', () => {
     expect(body).toContain(`found an issue on ${findingWithEmptyOptionalFields.url}`)
     expect(body).not.toContain('flagged the element')
   })
+
+  it('omits the category notice for WCAG findings', () => {
+    expect(generateIssueBody(baseFinding, 'github/accessibility-scanner')).not.toContain('> [!NOTE]')
+    expect(generateIssueBody({...baseFinding, category: 'wcag'}, 'github/accessibility-scanner')).not.toContain(
+      '> [!NOTE]',
+    )
+  })
+
+  it('includes a best-practice notice for best-practice findings', () => {
+    const body = generateIssueBody({...baseFinding, category: 'best-practice'}, 'github/accessibility-scanner')
+
+    expect(body).toContain('> [!NOTE]')
+    expect(body).toContain('best-practice recommendation')
+    expect(body).toContain('not a definite WCAG failure')
+    expect(body).toContain('WCAG 2.2 if applicable')
+    expect(body).not.toContain('The fix MUST meet WCAG 2.2 guidelines OR')
+  })
+
+  it('includes an experimental notice for experimental findings', () => {
+    const body = generateIssueBody({...baseFinding, category: 'experimental'}, 'github/accessibility-scanner')
+
+    expect(body).toContain('> [!NOTE]')
+    expect(body).toContain('an experimental check')
+    expect(body).toContain('not a definite WCAG failure')
+    expect(body).toContain('WCAG 2.2 if applicable')
+    expect(body).not.toContain('The fix MUST meet WCAG 2.2 guidelines OR')
+  })
 })

.github/actions/file/tests/openIssue.test.ts

@@ -65,6 +65,28 @@ describe('openIssue', () => {
     )
   })
 
+  it('adds a category label for non-WCAG findings', async () => {
+    const octokit = mockOctokit()
+    await openIssue(octokit, 'org/repo', {...baseFinding, category: 'best-practice'})
+
+    expect(octokit.request).toHaveBeenCalledWith(
+      expect.any(String),
+      expect.objectContaining({
+        labels: ['axe-scanning-issue', 'axe rule: color-contrast', 'best-practice'],
+      }),
+    )
+  })
+
+  it('does not add a category label for WCAG findings', async () => {
+    const octokit = mockOctokit()
+    await openIssue(octokit, 'org/repo', {...baseFinding, category: 'wcag'})
+
+    const labels = octokit.request.mock.calls[0][1].labels
+    expect(labels).not.toContain('wcag')
+    expect(labels).not.toContain('best-practice')
+    expect(labels).not.toContain('experimental')
+  })
+
   it('truncates long titles with ellipsis', async () => {
     const octokit = mockOctokit()
     const longFinding = {

.github/actions/find/src/findForUrl.ts

@@ -1,4 +1,4 @@
-import type {ColorSchemePreference, Finding, ReducedMotionPreference, UrlConfig} from './types.d.js'
+import type {ColorSchemePreference, Finding, FindingCategory, ReducedMotionPreference, UrlConfig} from './types.d.js'
 import {AxeBuilder} from '@axe-core/playwright'
 import playwright from 'playwright'
 import {AuthContext} from './AuthContext.js'
@@ -87,6 +87,7 @@ async function runAxeScan({
     for (const violation of rawFindings.violations) {
       await addFinding({
         scannerType: 'axe',
+        category: categorizeAxeViolation(violation.tags),
         url,
         html: violation.nodes[0].html.replace(/'/g, '''),
         problemShort: violation.help.toLowerCase().replace(/'/g, '''),
@@ -98,3 +99,11 @@ async function runAxeScan({
     }
   }
 }
+
+// Maps an Axe violation's tags to a conformance tier. Experimental is checked
+// first because some experimental rules also carry a wcag* tag.
+function categorizeAxeViolation(tags: string[]): FindingCategory {
+  if (tags.includes('experimental')) return 'experimental'
+  if (tags.includes('best-practice')) return 'best-practice'
+  return 'wcag'
+}

.github/actions/find/src/types.d.ts

@@ -1,5 +1,8 @@
+export type FindingCategory = 'wcag' | 'best-practice' | 'experimental'
+
 export type Finding = {
   scannerType: string
+  category?: FindingCategory
   url: string
   html?: string
   problemShort: string

.github/actions/find/tests/findForUrl.test.ts

@@ -117,4 +117,40 @@ describe('findForUrl', () => {
       expect(loadedPlugins[1].default).toHaveBeenCalledTimes(0)
     })
   })
+
+  describe('axe finding categorization', () => {
+    function axeViolation(tags: string[]) {
+      return {
+        id: 'some-rule',
+        help: 'Help',
+        helpUrl: 'https://example.com',
+        description: 'Description',
+        tags,
+        nodes: [{html: '<div></div>', failureSummary: 'summary'}],
+      }
+    }
+
+    async function categoryFor(tags: string[]) {
+      clearAll()
+      actionInput = JSON.stringify(['axe'])
+      vi.mocked(AxeBuilder.prototype.analyze).mockResolvedValueOnce({
+        violations: [axeViolation(tags)],
+      } as unknown as axe.AxeResults)
+
+      const findings = await findForUrl('test.com')
+      return findings[0].category
+    }
+
+    it('categorizes a violation with only wcag tags as wcag', async () => {
+      expect(await categoryFor(['wcag2a', 'wcag111'])).toBe('wcag')
+    })
+
+    it('categorizes a violation with a best-practice tag as best-practice', async () => {
+      expect(await categoryFor(['cat.semantics', 'best-practice'])).toBe('best-practice')
+    })
+
+    it('categorizes a violation with an experimental tag as experimental, even alongside wcag tags', async () => {
+      expect(await categoryFor(['wcag2a', 'experimental'])).toBe('experimental')
+    })
+  })
 })