Skip to content
Merged
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
12 changes: 6 additions & 6 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -68,28 +68,28 @@ 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,floatingPromise,deepNesting,errorSwallow,unsafeAny,i18n
# commitLint
# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,a11y
# i18n,commitLint
#
# 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,floatingPromise,deepNesting,errorSwallow,unsafeAny,i18n
# debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,a11y,i18n
# 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,floatingPromise,deepNesting
# errorSwallow,unsafeAny,i18n,commitLint
# errorSwallow,unsafeAny,a11y,i18n,commitLint
# 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,floatingPromise,deepNesting,errorSwallow,unsafeAny,i18n
# commitLint
# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,a11y
# i18n,commitLint
# 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 @@ -1071,6 +1071,29 @@ export const REES_ANALYZERS = [
"Structural regex only (no type-checker). String literals and comment lines are skipped; findings are capped.",
},
},
{
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 newly added JSX/HTML markup lines.",
looksAt:
"Added lines in changed non-test .jsx/.tsx/.html/.vue files for missing img alt text, click-only handlers, unlabeled controls, and positive tabindex.",
reports:
"File, line, and rule: img-alt, click-events-have-key-events, label-control, or positive-tabindex.",
network: "Pure local analyzer. No external network call.",
notes:
"Structural markup heuristics only — buttons/links and controls with aria-label/id associations are not flagged.",
},
},
{
name: "i18n",
title: "i18n regressions",
Expand Down
26 changes: 26 additions & 0 deletions review-enrichment/analyzer-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -1213,6 +1213,32 @@
"notes": "Structural regex only (no type-checker). String literals and comment lines are skipped; findings are capped."
}
},
{
"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 newly added JSX/HTML markup lines.",
"looksAt": "Added lines in changed non-test .jsx/.tsx/.html/.vue files for missing img alt text, click-only handlers, unlabeled controls, and positive tabindex.",
"reports": "File, line, and rule: img-alt, click-events-have-key-events, label-control, or positive-tabindex.",
"network": "Pure local analyzer. No external network call.",
"notes": "Structural markup heuristics only — buttons/links and controls with aria-label/id associations are not flagged."
}
},
{
"name": "i18n",
"title": "i18n regressions",
Expand Down
124 changes: 124 additions & 0 deletions review-enrichment/src/analyzers/a11y-regression.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
// Accessibility regression analyzer (#2026). Flags common a11y regressions in newly added JSX/HTML markup:
// images without alt text, click-only handlers on non-interactive elements, unlabeled form controls, and
// positive tabindex values. Pure compute over added lines in .jsx/.tsx/.html/.vue files, no network.
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 = /\.(?:tsx|jsx|html|vue)$/i;

const POSITIVE_TABINDEX_RE =
/\btabIndex\s*=\s*\{\s*([1-9]\d*)\s*\}|\btabindex\s*=\s*["']([1-9]\d*)["']/i;
const IMG_TAG_RE = /<img\b/i;
const ALT_ATTR_RE = /\balt\s*=/i;
const ON_CLICK_RE = /\bonClick\s*=/;
const KEYBOARD_HANDLER_RE = /\bonKey(?:Down|Up|Press)\s*=/;
const INTERACTIVE_TAG_RE = /<(?:button|a)\b/i;
const INTERACTIVE_ROLE_RE =
/\brole\s*=\s*["'](?:button|link|menuitem|tab|switch|checkbox|radio)["']/i;
const NON_INTERACTIVE_CLICK_TARGET_RE =
/<(?:div|span|p|li|td|tr|section|article|header|footer|main|nav)\b/i;
const FORM_CONTROL_RE = /<(?:input|select|textarea)\b/i;
const LABEL_ASSOC_RE = /\b(?:aria-label|aria-labelledby|id)\s*=|<label\b/i;

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

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

/** Classify one added markup line for an a11y regression, or null. Pure. */
export function detectA11yRegression(line: string): A11yFinding["rule"] | null {
if (isCommentLine(line) || line.length > MAX_LINE_CHARS) return null;

if (POSITIVE_TABINDEX_RE.test(line)) return "positive-tabindex";

if (IMG_TAG_RE.test(line) && !ALT_ATTR_RE.test(line)) return "img-alt";

if (ON_CLICK_RE.test(line)) {
const hasKeyboard = KEYBOARD_HANDLER_RE.test(line);
const isInteractive =
INTERACTIVE_TAG_RE.test(line) || INTERACTIVE_ROLE_RE.test(line);
if (
!hasKeyboard &&
!isInteractive &&
NON_INTERACTIVE_CLICK_TARGET_RE.test(line)
) {
return "click-events-have-key-events";
}
}

if (FORM_CONTROL_RE.test(line) && !LABEL_ASSOC_RE.test(line)) {
return "label-control";
}

return null;
}

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

/** Scan one file patch's added lines for a11y regressions, line-cited via hunk headers. Pure. */
export function scanPatchForA11yRegression(
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("+")) {
const body = line.slice(1);
const rule = detectA11yRegression(body);
if (rule) {
findings.push({ file: path, line: newLine, rule });
if (findings.length >= maxFindings) return findings;
}
newLine++;
} else if (!line.startsWith("-") && !line.startsWith("\\")) {
newLine++;
}
}

return findings;
}

/** Analyzer entrypoint: scan markup files for accessibility regressions. */
export async function scanA11yRegression(
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 scanPatchForA11yRegression(file.path, file.patch, {
maxFindings: MAX_FINDINGS - findings.length,
signal,
})) {
findings.push(finding);
if (findings.length >= MAX_FINDINGS) return findings;
}
}
return findings;
}
32 changes: 32 additions & 0 deletions review-enrichment/src/analyzers/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { scanI18nRegression } from "./i18n-regression.js";
import { scanErrorSwallow } from "./error-swallow.js";
import { scanFloatingPromise } from "./floating-promise.js";
import { scanSizeSmell } from "./size-smell.js";
import { scanA11yRegression } from "./a11y-regression.js";
import { scanCommitLint } from "./commit-lint.js";
import { scanUnsafeAny } from "./unsafe-any.js";
import { scanTerminology } from "./terminology.js";
Expand Down Expand Up @@ -1167,6 +1168,37 @@ export const ANALYZER_DESCRIPTORS = [
},
run: (req, { signal }) => scanUnsafeAny(req, signal),
}),
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 newly added JSX/HTML markup lines.",
looksAt:
"Added lines in changed non-test .jsx/.tsx/.html/.vue files for missing img alt text, click-only handlers, unlabeled controls, and positive tabindex.",
reports:
"File, line, and rule: img-alt, click-events-have-key-events, label-control, or positive-tabindex.",
network: "Pure local analyzer. No external network call.",
notes:
"Structural markup heuristics only — buttons/links and controls with aria-label/id associations are not flagged.",
},
render: (findings, helpers) => {
if (!findings.length) return [];
const lines = ["### Accessibility regressions (added markup)"];
for (const item of findings) {
lines.push(
`- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)} — ${helpers.safeCodeSpan(item.rule)}`,
);
}
return lines;
},
run: (req, { signal }) => scanA11yRegression(req, signal),
}),
descriptor({
name: "i18n",
title: "i18n regressions",
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 @@ -491,6 +491,7 @@ export function renderBrief(
lines.push(...renderDescriptorSection("deepNesting", findings.deepNesting));
lines.push(...renderDescriptorSection("errorSwallow", findings.errorSwallow));
lines.push(...renderDescriptorSection("unsafeAny", findings.unsafeAny));
lines.push(...renderDescriptorSection("a11y", findings.a11y));
lines.push(...renderDescriptorSection("i18n", findings.i18n));
lines.push(...renderDescriptorSection("hardcodedUrl", findings.hardcodedUrl));
lines.push(...renderDescriptorSection("commitLint", findings.commitLint));
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 @@ -536,6 +536,18 @@ export interface UnsafeAnyFinding {
kind: "annotation" | "cast" | "assertion";
}

/** A common accessibility regression in newly added markup (#2026, part of #1499).
* Reports file, line, and rule only — never string content. */
export interface A11yFinding {
file: string;
line: number;
rule:
| "img-alt"
| "click-events-have-key-events"
| "label-control"
| "positive-tabindex";
}

/** An absolute HTTP(S) URL or raw IP:port endpoint hardcoded in non-test, non-config source (#2027, part of #1499).
* Reports location, kind, and a redacted/truncated host — never full paths or query strings. */
export interface HardcodedUrlFinding {
Expand Down Expand Up @@ -597,6 +609,7 @@ export interface BriefFindings {
deepNesting?: DeepNestingFinding[];
errorSwallow?: ErrorSwallowFinding[];
unsafeAny?: UnsafeAnyFinding[];
a11y?: A11yFinding[];
i18n?: I18nFinding[];
hardcodedUrl?: HardcodedUrlFinding[];
commitLint?: CommitLintFinding[];
Expand Down
89 changes: 89 additions & 0 deletions review-enrichment/test/a11y-regression.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Units for the a11y regression analyzer (#2026). Own file so concurrent analyzer PRs don't collide.
import { test } from "node:test";
import assert from "node:assert/strict";
import {
detectA11yRegression,
scanA11yRegression,
scanPatchForA11yRegression,
} from "../dist/analyzers/a11y-regression.js";
import { renderBrief } from "../dist/render.js";

const patchOf = (lines: string[]) =>
`@@ -1,0 +1,${lines.length} @@\n${lines.map((l) => `+${l}`).join("\n")}`;

test("detectA11yRegression: flags each rule on representative markup", () => {
assert.equal(detectA11yRegression('<img src="/logo.png" />'), "img-alt");
assert.equal(
detectA11yRegression('<div onClick={() => open()}>Open</div>'),
"click-events-have-key-events",
);
assert.equal(detectA11yRegression('<input type="email" />'), "label-control");
assert.equal(detectA11yRegression('<div tabIndex={3}>Skip</div>'), "positive-tabindex");
});

test("detectA11yRegression: does not flag compliant markup", () => {
assert.equal(detectA11yRegression('<img alt="Logo" src="/logo.png" />'), null);
assert.equal(detectA11yRegression('<button onClick={() => open()}>Open</button>'), null);
assert.equal(
detectA11yRegression('<div role="button" onClick={() => open()}>Open</div>'),
null,
);
assert.equal(
detectA11yRegression('<div onClick={() => open()} onKeyDown={handleKey}>Open</div>'),
null,
);
assert.equal(detectA11yRegression('<input aria-label="Email" type="email" />'), null);
assert.equal(detectA11yRegression("<label><input type=\"email\" /></label>"), null);
assert.equal(detectA11yRegression('<div tabIndex={0}>Focusable</div>'), null);
});

test("scanPatchForA11yRegression: flags added lines with correct locations", () => {
const findings = scanPatchForA11yRegression(
"src/Widget.tsx",
patchOf([
"export function Widget() {",
' return <img src="/logo.png" />;',
"}",
]),
);
assert.deepEqual(findings, [
{ file: "src/Widget.tsx", line: 2, rule: "img-alt" },
]);
});

test("scanPatchForA11yRegression: skips test files and non-markup paths", () => {
assert.deepEqual(
scanPatchForA11yRegression("src/Widget.test.tsx", patchOf(['<img src="/x" />'])),
[],
);
assert.deepEqual(
scanPatchForA11yRegression("src/worker.ts", patchOf(['<img src="/x" />'])),
[],
);
});

test("scanPatchForA11yRegression: respects the findings cap", () => {
const lines = Array.from({ length: 10 }, () => '<img src="/x" />');
assert.equal(
scanPatchForA11yRegression("src/a.tsx", patchOf(lines), { maxFindings: 2 }).length,
2,
);
});

test("scanA11yRegression: aggregates across files and renders a public-safe brief", async () => {
const findings = await scanA11yRegression({
repoFullName: "owner/repo",
prNumber: 1,
files: [
{ path: "src/a.tsx", patch: patchOf(['<div tabIndex={5}>Skip</div>']) },
{ path: "src/b.html", patch: patchOf(['<input type="text" />']) },
],
});
assert.equal(findings.length, 2);
assert.equal(findings[0]?.rule, "positive-tabindex");
assert.equal(findings[1]?.rule, "label-control");
const { promptSection } = renderBrief({ a11y: findings });
assert.match(promptSection, /Accessibility regressions/);
assert.match(promptSection, /positive-tabindex/);
assert.doesNotMatch(promptSection, /Skip/);
});
Loading
Loading