Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -68,25 +68,25 @@ GITTENSORY_REVIEW_ENRICHMENT=false
# commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot
# blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
# pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber
# conflictMarker,debugLeftover,sizeSmell,commitLint
# conflictMarker,debugLeftover,sizeSmell,commitLint,a11y
#
# Profile defaults:
# fast: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency
# hardcodedUrl,actionPin,eol,redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild
# testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker
# debugLeftover,sizeSmell
# debugLeftover,sizeSmell,a11y
# balanced (default): dependency,dependencyDiff,lockfileDrift,secret,license,installScript
# heavyDependency,hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight
# typosquat,commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication
# churnHotspot,blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch
# commitHygiene,pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology
# todoMarker,magicNumber,conflictMarker,debugLeftover,sizeSmell,commitLint
# todoMarker,magicNumber,conflictMarker,debugLeftover,sizeSmell,commitLint,a11y
# deep: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency
# hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat
# commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot
# blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
# pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber
# conflictMarker,debugLeftover,sizeSmell,commitLint
# conflictMarker,debugLeftover,sizeSmell,commitLint,a11y
# END GENERATED REES ANALYZERS

# Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep
Expand Down
23 changes: 23 additions & 0 deletions apps/gittensory-ui/src/lib/rees-analyzers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,29 @@ export const REES_ANALYZERS = [
"Structured-fields-only: reads commit.message subjects, linted independently, never cross-line state. Fail-safe on missing token/fetch error.",
},
},
{
name: "a11y",
title: "Accessibility regressions",
category: "quality",
cost: "local",
defaultEnabled: true,
profiles: ["fast", "balanced", "deep"],
requires: ["files"],
limits: {
maxFindings: 25,
maxLineChars: 2000,
},
docs: {
summary:
"Flags common accessibility regressions in added JSX/HTML markup: an <img> without alt, a clickable non-interactive element with no keyboard handler or role, a form control with no label association, and a positive tabindex.",
looksAt:
"Self-contained added tags (open through close on one line) in .jsx, .tsx, .html, and .vue files.",
reports: "File, line, and public-safe rule kind — never markup content.",
network: "Pure local analyzer. No external network call.",
notes:
"Only matches tags whose full opening tag appears on a single added line; multi-line attribute lists are not scanned.",
},
},
] as const satisfies readonly ReesAnalyzerDoc[];

export const REES_ANALYZER_NAMES = REES_ANALYZERS.map((analyzer) => analyzer.name);
26 changes: 26 additions & 0 deletions review-enrichment/analyzer-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -1131,6 +1131,32 @@
"network": "Calls the GitHub PR-commits API once, bounded to one page.",
"notes": "Structured-fields-only: reads commit.message subjects, linted independently, never cross-line state. Fail-safe on missing token/fetch error."
}
},
{
"name": "a11y",
"title": "Accessibility regressions",
"category": "quality",
"cost": "local",
"defaultEnabled": true,
"profiles": [
"fast",
"balanced",
"deep"
],
"requires": [
"files"
],
"limits": {
"maxFindings": 25,
"maxLineChars": 2000
},
"docs": {
"summary": "Flags common accessibility regressions in added JSX/HTML markup: an <img> without alt, a clickable non-interactive element with no keyboard handler or role, a form control with no label association, and a positive tabindex.",
"looksAt": "Self-contained added tags (open through close on one line) in .jsx, .tsx, .html, and .vue files.",
"reports": "File, line, and public-safe rule kind — never markup content.",
"network": "Pure local analyzer. No external network call.",
"notes": "Only matches tags whose full opening tag appears on a single added line; multi-line attribute lists are not scanned."
}
}
]
}
160 changes: 160 additions & 0 deletions review-enrichment/src/analyzers/a11y-regression.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
// Accessibility-regression analyzer (#2026, the a11y half of the #1499 epic 'accessibility and i18n regression'
// idea; the i18n half is a separate bounty). Flags common accessibility regressions in added JSX/HTML markup:
// an <img> without alt, an onClick handler added to a non-interactive element without a keyboard handler or
// role, a form control with no way to associate a label, and a positive tabindex (which breaks natural tab
// order). Pure compute over added diff lines, no network. Only self-contained tags (opening `<` through closing
// `>` on the same added line) are matched, so a tag whose attributes wrap across lines is not scanned — this
// keeps the analyzer diff-local and free of false positives from partial tags.
import type { A11yFinding, EnrichRequest } from "../types.js";
import { isTestPath } from "./test-ratio.js";

const MAX_FINDINGS = 25;
const MAX_LINE_CHARS = 2000;

const MARKUP_PATH_RE = /\.(?:jsx|tsx|html?|vue)$/i;

// Elements with built-in interactive semantics — an onClick here needs no extra keyboard wiring.
const INTERACTIVE_TAGS = new Set([
"a",
"button",
"input",
"select",
"option",
"textarea",
"summary",
"label",
"audio",
"video",
"details",
"dialog",
"menuitem",
]);

// <input> types that are never associated with a visible label (hidden fields, buttons that carry their own text).
const LABELLESS_INPUT_TYPES = new Set(["hidden", "submit", "button", "reset", "image"]);

const TAG_RE = /<([a-zA-Z][\w-]*)\b([^>]*)>/g;
const ALT_RE = /\balt\s*=/i;
const ONCLICK_RE = /\bonClick\s*=/;
const KEY_HANDLER_OR_ROLE_RE = /\bonKeyDown\s*=|\bonKeyUp\s*=|\bonKeyPress\s*=|\brole\s*=/;
const TYPE_ATTR_RE = /\btype\s*=\s*["']?(\w+)/i;
const LABEL_ASSOC_RE = /\bid\s*=|\baria-label\s*=|\baria-labelledby\s*=/i;
const TABINDEX_RE = /\btabindex\s*=\s*["'{]?\s*(-?\d+)/i;

function isCommentLine(line: string): boolean {
const trimmed = line.trimStart();
return /^(?:\/\/|\/\*|\*|<!--|\{\/\*)/.test(trimmed);
}

function isMarkupPath(path: string): boolean {
return MARKUP_PATH_RE.test(path) && !isTestPath(path);
}

/** Classify one self-contained `<tag ...>` opening tag for a11y regressions. Pure. Returns every rule the tag
* trips (a single tag can trip more than one, e.g. a clickable div with a positive tabindex). */
export function detectA11yIssues(
tagName: string,
attrs: string,
): Array<A11yFinding["rule"]> {
const tag = tagName.toLowerCase();
const rules: Array<A11yFinding["rule"]> = [];

if (tag === "img" && !ALT_RE.test(attrs)) {
rules.push("img-alt");
}

if (
ONCLICK_RE.test(attrs) &&
!INTERACTIVE_TAGS.has(tag) &&
!KEY_HANDLER_OR_ROLE_RE.test(attrs)
) {
rules.push("click-events-have-key-events");
}

if (tag === "input" || tag === "textarea" || tag === "select") {
const typeMatch = TYPE_ATTR_RE.exec(attrs);
const type = typeMatch?.[1]?.toLowerCase();
const skippable = tag === "input" && type !== undefined && LABELLESS_INPUT_TYPES.has(type);
if (!skippable && !LABEL_ASSOC_RE.test(attrs)) {
rules.push("label-control");
}
}

const tabindexMatch = TABINDEX_RE.exec(attrs);
if (tabindexMatch && Number(tabindexMatch[1]) > 0) {
rules.push("positive-tabindex");
}

return rules;
}

type ScanLimits = {
maxFindings?: number;
signal?: AbortSignal;
};

/** Scan one file patch's added lines for accessibility regressions, line-cited via hunk headers. Pure. */
export function scanPatchForA11y(
path: string,
patch: string,
limits: ScanLimits = {},
): A11yFinding[] {
const maxFindings = limits.maxFindings ?? MAX_FINDINGS;
if (maxFindings <= 0 || !isMarkupPath(path)) return [];

const findings: A11yFinding[] = [];
let newLine = 0;
let inHunk = false;

for (const line of patch.split("\n")) {
if (limits.signal?.aborted) throw new Error("analyzer_aborted");
const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line);
if (hunk) {
newLine = Number(hunk[1]);
inHunk = true;
continue;
}
if (!inHunk) continue;
if (!line.startsWith("+")) {
if (!line.startsWith("-") && !line.startsWith("\\")) newLine++;
continue;
}

const body = line.slice(1);
if (body.length <= MAX_LINE_CHARS && !isCommentLine(body)) {
TAG_RE.lastIndex = 0;
let match: RegExpExecArray | null;
while ((match = TAG_RE.exec(body)) !== null) {
const [, tagName, attrs] = match;
if (tagName === undefined || attrs === undefined) continue;
for (const rule of detectA11yIssues(tagName, attrs)) {
findings.push({ file: path, line: newLine, rule });
if (findings.length >= maxFindings) return findings;
}
}
}
newLine++;
}

return findings;
}

/** Analyzer entrypoint: scan every changed markup file's added lines for accessibility regressions. */
export async function scanA11y(
req: EnrichRequest,
signal?: AbortSignal,
): Promise<A11yFinding[]> {
const findings: A11yFinding[] = [];
for (const file of req.files ?? []) {
if (signal?.aborted) throw new Error("analyzer_aborted");
if (!file.patch) continue;
for (const finding of scanPatchForA11y(file.path, file.patch, {
maxFindings: MAX_FINDINGS - findings.length,
signal,
})) {
findings.push(finding);
if (findings.length >= MAX_FINDINGS) return findings;
}
}
return findings;
}
42 changes: 42 additions & 0 deletions review-enrichment/src/analyzers/registry.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { scanA11y } from "./a11y-regression.js";
import { scanActionPins } from "./actions-pin.js";
import { scanApprovalIntegrity } from "./approval-integrity.js";
import { scanAssetWeight } from "./asset-weight.js";
Expand Down Expand Up @@ -1088,6 +1089,47 @@ export const ANALYZER_DESCRIPTORS = [
run: (req, { signal, analysis, diagnostics }) =>
scanCommitLint(req, fetch, { signal, analysis, diagnostics }),
}),
descriptor({
name: "a11y",
title: "Accessibility regressions",
category: "quality",
cost: "local",
defaultEnabled: true,
requires: ["files"],
limits: { maxFindings: 25, maxLineChars: 2000 },
docs: {
summary:
"Flags common accessibility regressions in added JSX/HTML markup: an <img> without alt, a clickable non-interactive element with no keyboard handler or role, a form control with no label association, and a positive tabindex.",
looksAt: "Self-contained added tags (open through close on one line) in .jsx, .tsx, .html, and .vue files.",
reports: "File, line, and public-safe rule kind — never markup content.",
network: "Pure local analyzer. No external network call.",
notes:
"Only matches tags whose full opening tag appears on a single added line; multi-line attribute lists are not scanned.",
},
render: (findings, helpers) => {
if (!findings.length) return [];
const explain = (rule: (typeof findings)[number]["rule"]): string => {
switch (rule) {
case "img-alt":
return "<img> missing alt text";
case "click-events-have-key-events":
return "onClick on a non-interactive element with no keyboard handler or role";
case "label-control":
return "form control with no way to associate a label";
case "positive-tabindex":
return "positive tabindex breaks natural tab order";
}
};
const lines = ["### Accessibility regressions (added markup)"];
for (const item of findings) {
lines.push(
`- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)} — ${explain(item.rule)}`,
);
}
return lines;
},
run: (req, { signal }) => scanA11y(req, signal),
}),
] as const satisfies readonly AnyAnalyzerDescriptor[];

export const ANALYZER_NAMES = ANALYZER_DESCRIPTORS.map(
Expand Down
1 change: 1 addition & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ export function renderBrief(
lines.push(...renderDescriptorSection("sizeSmell", findings.sizeSmell));
lines.push(...renderDescriptorSection("hardcodedUrl", findings.hardcodedUrl));
lines.push(...renderDescriptorSection("commitLint", findings.commitLint));
lines.push(...renderDescriptorSection("a11y", findings.a11y));

if (!lines.length) return { promptSection: "", systemSuffix: "" };

Expand Down
13 changes: 13 additions & 0 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,18 @@ export interface CommitLintFinding {
reason: "bad-type" | "missing-colon" | "too-long" | "empty";
}

/** An accessibility regression in added JSX/HTML markup — the a11y half of the #1499 epic 'accessibility and i18n
* regression' idea (#2026, part of #1499). Reports location + rule only, never markup content. */
export interface A11yFinding {
file: string;
line: number;
rule:
| "img-alt"
| "click-events-have-key-events"
| "label-control"
| "positive-tabindex";
}

/** Structured analyzer output. Each analyzer fills its own key; more land as analyzers ship (#1477/#1478). */
export interface BriefFindings {
dependency?: DependencyFinding[];
Expand Down Expand Up @@ -553,6 +565,7 @@ export interface BriefFindings {
sizeSmell?: SizeSmellFinding[];
hardcodedUrl?: HardcodedUrlFinding[];
commitLint?: CommitLintFinding[];
a11y?: A11yFinding[];
}

/** A JSDoc/TSDoc block whose `@param` tags name parameters the adjacent function no longer declares — a
Expand Down
Loading