fix: distinguish WCAG violations from best practices in issue output by Hardonian · Pull Request #206 · github/accessibility-scanner

Hardonian · 2026-04-24T04:13:52Z

Summary

Addresses #34 — The scanner previously conflated best-practice recommendations with actual WCAG conformance failures in auto-generated issues and PRs. This caused Copilot to claim WCAG violations when only best-practice rules were triggered.

Changes

Core Fix

ruleType field added to Finding type — extracted from axe-core violation tags (wcag, best-practice, experimental)
Issue body now clearly labels each finding type with a badge and description:
- 🚨 WCAG Violation — conformance failure
- 💡 Best Practice — recommendation, not required for compliance
- 🧪 Experimental — non-standard rule
Acceptance criteria adjusted per type (WCAG requirements vs best-practice guidance)
Issue labels now include wcag-violation, best-practice, or experimental for easy filtering

Test Updates

Updated generateIssueBody test for new heading format
Added tests for best-practice and experimental label paths in openIssue

Maintenance

Fixed ESLint error in test plugin (unused vars)
Added missing esbuild devDependency
Included .js files in eslint config

Example Output

Before (misleading):

which violates WCAG 2.1 guidelines

After (accurate):

🚨 WCAG Violation — This is a WCAG conformance failure.
— or —
💡 Best Practice Recommendation — This is a best practice recommendation, not a WCAG conformance failure.

Addresses issue github#34 - The scanner previously conflated best-practice recommendations with actual WCAG conformance failures in auto-generated issue titles and bodies. This led to Copilot filing PRs claiming WCAG violations when only best-practice rules were triggered. Changes: - Add `ruleType` field to Finding type (wcag | best-practice | experimental) - Extract rule type from axe-core violation tags in findForUrl - Update issue body generator with clear type badge and description - Update issue labels: add wcag-violation / best-practice / experimental labels - Update tests with new label expectations - Fix ESLint error in test plugin (unused vars) - Add missing esbuild devDependency - Include .js files in eslint config

Copilot

Pull request overview

This PR updates the scanner’s finding model and issue generation so that best-practice and experimental axe rules are no longer presented as WCAG conformance failures in auto-filed issues/PRs (addresses #34).

Changes:

Add ruleType to Finding and infer it from axe-core violation tags (wcag*, best-practice, experimental).
Update issue body formatting + acceptance criteria to reflect the finding type, and add type-based GitHub labels.
Update/extend tests for new issue body headings and label selection; minor tooling/deps updates (eslint + esbuild).

Show a summary per file

File	Description
package.json	Adds `esbuild` devDependency.
package-lock.json	Locks `esbuild` and platform-specific optional deps.
eslint.config.js	Expands ESLint file matching to include `.js`.
.github/scanner-plugins/test-js-file-plugin-loading/index.js	Removes unused plugin parameters to satisfy linting.
.github/actions/find/src/types.d.ts	Adds optional `ruleType` to findings produced by the find action.
.github/actions/find/src/findForUrl.ts	Infers `ruleType` from axe violation tags and passes it into findings.
.github/actions/file/src/types.d.ts	Adds optional `ruleType` to findings consumed by the file action.
.github/actions/file/src/openIssue.ts	Adds type-based labels (`wcag-violation`, `best-practice`, `experimental`).
.github/actions/file/src/generateIssueBody.ts	Adds type-based heading/badge/acceptance-criteria generation.
.github/actions/file/tests/openIssue.test.ts	Adds coverage for best-practice and experimental label paths.
.github/actions/file/tests/generateIssueBody.test.ts	Updates expectations for new issue body heading/type text.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (2)

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

ruleTypeLabel.isWcag is used to choose between a WCAG-failure description and a best-practice description, but ruleType === 'experimental' sets isWcag: false, so experimental findings will incorrectly render the best-practice message. Consider branching the description on the actual rule type (wcag vs best-practice vs experimental) so experimental is described as non-standard/experimental rather than best-practice.

  const ruleTypeSection = `> **Type:** ${ruleTypeLabel.badge}
>
> ${ruleTypeLabel.isWcag
    ? 'This is a **WCAG conformance failure**. Fixing this issue helps meet WCAG 2.1 accessibility requirements.'
    : 'This is a **best practice recommendation**, not a WCAG conformance failure. Fixing it improves accessibility but is not required for WCAG compliance.'
  }`

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

The fallback in getRuleTypeLabel() treats any non-best-practice/experimental value (including undefined or a typo) as a WCAG violation. Given Finding.ruleType is optional and comes from external data (axe tags / plugins), this can silently misclassify findings and reintroduce the “claims WCAG violation” problem when ruleType is missing/unknown. Consider explicitly handling ruleType === 'wcag' vs undefined/unknown (e.g., an "Unclassified" type, or omit the compliance claim unless ruleType is wcag).

function getRuleTypeLabel(ruleType?: string): { heading: string; badge: string; isWcag: boolean } {
  if (ruleType === 'best-practice') {
    return {
      heading: 'Best Practice Recommendation',
      badge: '\U0001F4A1 Best Practice',
      isWcag: false,
    }
  }
  if (ruleType === 'experimental') {
    return {
      heading: 'Experimental Rule',
      badge: '\U0001F9EA Experimental',
      isWcag: false,
    }
  }
  // Default to WCAG violation
  return {
    heading: 'WCAG Violation',
    badge: '\U0001F6A8 WCAG Violation',
    isWcag: true,
  }

Files reviewed: 8/11 changed files
Comments generated: 5

Copilot · 2026-04-24T14:36:22Z

+/**
+ * Determines if a finding is a WCAG violation or a best practice recommendation.
+ */


The JSDoc for getRuleTypeLabel says it determines WCAG vs best-practice, but the function also handles experimental. Consider updating the comment to reflect all supported rule types (wcag / best-practice / experimental) to avoid confusion.

This issue also appears in the following locations of the same file:

line 6

line 49

Copilot · 2026-04-24T14:36:23Z

+  // Add rule type label to distinguish WCAG violations from best practices
+  if (finding.ruleType === 'best-practice') {
+    labels.push('best-practice')
+  } else if (finding.ruleType === 'experimental') {
+    labels.push('experimental')
+  } else {
+    // Default to wcag for any WCAG-tagged rule


This logic adds wcag-violation for any finding whose ruleType is not exactly best-practice or experimental (including undefined or unexpected values). Since Finding.ruleType is optional and can be omitted by plugins, this can incorrectly label non-WCAG findings as WCAG violations. Consider only adding wcag-violation when finding.ruleType === 'wcag', and either omit the type label or add an explicit "unknown" label when ruleType is missing.

Suggested change

// Add rule type label to distinguish WCAG violations from best practices

if (finding.ruleType === 'best-practice') {

labels.push('best-practice')

} else if (finding.ruleType === 'experimental') {

labels.push('experimental')

} else {

// Default to wcag for any WCAG-tagged rule

// Add a rule type label only for explicitly recognized rule types

if (finding.ruleType === 'best-practice') {

labels.push('best-practice')

} else if (finding.ruleType === 'experimental') {

labels.push('experimental')

} else if (finding.ruleType === 'wcag') {

+1 for this, as a default we don't want to have anything if ruleType is not passed in. One example is our custom reflow scan; this does map to a WCAG violation, however, if we add more built-in scans outside of Axe over time, this won't be relevant. Same with users who create their own custom plugins.

Copilot · 2026-04-24T14:36:23Z

+  /** Distinguishes WCAG violations from best practices. One of: "wcag", "best-practice", "experimental" */
+  ruleType?: string


ruleType is documented as having only three allowed values, but is typed as a generic string. Consider introducing a RuleType = 'wcag' | 'best-practice' | 'experimental' union (and using it for Finding.ruleType) so typos/unhandled values are caught at compile time and downstream logic (labels/body text) can be made exhaustive.

Copilot · 2026-04-24T14:36:23Z

 export type Finding = {
  scannerType: string
  ruleId?: string
+  /** Distinguishes WCAG violations from best practices. One of: "wcag", "best-practice", "experimental" */
+  ruleType?: string


ruleType is documented as having only three allowed values, but is typed as a generic string. Consider using a string-literal union (e.g., 'wcag' | 'best-practice' | 'experimental') to make openIssue() / generateIssueBody() logic exhaustive and prevent accidental mislabeling due to typos or unexpected values.

Suggested change

export type Finding = {

scannerType: string

ruleId?: string

/** Distinguishes WCAG violations from best practices. One of: "wcag", "best-practice", "experimental" */

ruleType?: string

export type FindingRuleType = 'wcag' | 'best-practice' | 'experimental'

export type Finding = {

scannerType: string

ruleId?: string

/** Distinguishes WCAG violations from best practices. One of: "wcag", "best-practice", "experimental" */

ruleType?: FindingRuleType

Copilot · 2026-04-24T14:36:24Z

+      // Determine rule type from axe-core tags
+      const tags = violation.tags ?? []
+      let ruleType: string | undefined
+      if (tags.some((tag: string) => tag.startsWith('wcag'))) {
+        ruleType = 'wcag'
+      } else if (tags.includes('best-practice')) {
+        ruleType = 'best-practice'
+      } else if (tags.includes('experimental')) {
+        ruleType = 'experimental'
+      }


ruleType is currently inferred into a string | undefined. Once Finding.ruleType is narrowed to a string-literal union, it would be good to type this local as the same union (or Finding['ruleType']) to avoid accidental values and keep the inference logic aligned with the Finding contract.

lindseywild

Thanks so much for contributing ✨ I've left a few comments, let us know if you have any questions!

lindseywild · 2026-04-24T15:12:40Z

+  if (ruleType === 'best-practice') {
+    return {
+      heading: 'Best Practice Recommendation',
+      badge: '\U0001F4A1 Best Practice',


I love the idea of adding an emoji, however it doesn't seem to render within the > in the Markdown.

I think we can also remove "Type: WCAG Violation/Best practice/experimental" line and just have "This is a X failure."

I agree, the emojis shouldn't be there. They can cause problems with different formats and add noise unnecessarily for screen reader users

lindseywild · 2026-04-24T15:15:08Z

+  // Add rule type label to distinguish WCAG violations from best practices
+  if (finding.ruleType === 'best-practice') {
+    labels.push('best-practice')
+  } else if (finding.ruleType === 'experimental') {
+    labels.push('experimental')
+  } else {
+    // Default to wcag for any WCAG-tagged rule


+1 for this, as a default we don't want to have anything if ruleType is not passed in. One example is our custom reflow scan; this does map to a WCAG violation, however, if we add more built-in scans outside of Axe over time, this won't be relevant. Same with users who create their own custom plugins.

lindseywild · 2026-04-24T15:15:38Z

 //   via js files still works and there are no regressions

-export default async function TestJsFilePluginLoad({ page, addFinding } = {}) {
+export default async function TestJsFilePluginLoad() {


Thanks for fixing!

abdulahmad307 · 2026-04-24T16:06:13Z

+      // Determine rule type from axe-core tags
+      const tags = violation.tags ?? []
+      let ruleType: string | undefined
+      if (tags.some((tag: string) => tag.startsWith('wcag'))) {


Minor (micro-optimization opportunity); we're iterating over tags in each condition here (I'm not sure how many tags there are in a violation in general - if the list is small, then this is prob not a big deal). We could run 1 loop, find all the info we need (stored in bools), then check against the bools in the conditions instead.

I suppose this can add up (even if the tags list is small) depending on how many pages we're scanning, how many violations are in each page, etc...

abdulahmad307 · 2026-04-24T16:12:48Z

    "@octokit/plugin-throttling": "^11.0.3",
    "@octokit/types": "^16.0.0",
    "@types/node": "^25.6.0",
+    "esbuild": "^0.28.0",


I'm curious as to why this was added here; it's in the package.json of the find action. I think the find, file, and fix actions are intentionally set up as isolated environments so they can be used individually - which is why esbuild was specifically installed there. For clarity, I don't think we are approaching this like a mono-repo (where you'd install packages at the root level). Have a look at the bootstrap.js file to get a sense of how this is set up.

let us know if you have any questions (or if you ran into an issue that caused you to add this here).

grace-snow · 2026-06-03T13:40:38Z

@lindseywild would any fails against GitHub's own accessibility requirements that are non-wcag be given a type of 'experimental'? I presume so.

I'm thinking about specific component requirements where you want an accessible name to be present on a section, that kind of scenario. It would still file an issue, but it's not a WCAG requirement, it's a requirement of the specific component-library.

lindseywild · 2026-06-03T14:59:28Z

@grace-snow I am thinking we wouldn't want a type at all for non-WCAG issues for the exact reason you say!

Hardonian requested a review from a team as a code owner April 24, 2026 04:13

Hardonian requested a review from accessibility-bot April 24, 2026 04:13

lindseywild requested a review from Copilot April 24, 2026 14:30

Copilot started reviewing on behalf of lindseywild April 24, 2026 14:31 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

lindseywild reviewed Apr 24, 2026

View reviewed changes

abdulahmad307 reviewed Apr 24, 2026

View reviewed changes

		/** Distinguishes WCAG violations from best practices. One of: "wcag", "best-practice", "experimental" */
		ruleType?: string

Conversation

Hardonian commented Apr 24, 2026

Summary

Changes

Core Fix

Test Updates

Maintenance

Example Output

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

lindseywild Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

lindseywild left a comment

Choose a reason for hiding this comment

Uh oh!

lindseywild Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

lindseywild Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

grace-snow Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

lindseywild Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

lindseywild Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

abdulahmad307 Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abdulahmad307 Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

grace-snow commented Jun 3, 2026

Uh oh!

lindseywild commented Jun 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

abdulahmad307 Apr 24, 2026 •

edited

Loading