From 2ceb467fc1ed1c03f07478242473b3b36e13779d Mon Sep 17 00:00:00 2001 From: Aaryan Choudhary Date: Fri, 6 Nov 2026 10:52:11 +0530 Subject: [PATCH] fix(accessibility): leave saved false positives out of issue answers fetchAccessibilityIssues only handed the agent the account's false-positive memory and left it to apply the entries; startAccessibilityScan did not read the memory at all, so a scan's first answer listed and counted issues the account had already marked as not real. Both now leave out, server side, the issues an entry names by its pattern's tag, tags, tagPrefix or classes (within its site and scope), and say how many each entry left out. The rest of the memory still comes with the issues for the agent to apply with judgment. - report-parser: a leaveOut hook drops covered issues from every page and count (summary, duplicates, per-page counts) while keeping the order, rule texts and cursor offsets of the full report, so held cursors stay valid. - paginate: pageOf takes a skip predicate that still advances the cursor. - fp-memory: applyMemory matches entries by pattern fields, never prose; counter-examples, excludeTags and tags a predicate excludes are kept. - accessibility: shared memoryLeaveOut helper used by both fetchAccessibilityIssues and startAccessibilityScan. - web_accessibility capability index: pattern.tags and pattern.excludeTags. Co-Authored-By: Claude Opus 5.5 --- .../web_accessibility.capability-index.json | 14 +- src/tools/accessibility.ts | 50 +++- src/tools/accessiblity-utils/fp-memory.ts | 218 ++++++++++++++++- src/tools/accessiblity-utils/paginate.ts | 23 +- src/tools/accessiblity-utils/report-parser.ts | 88 ++++++- .../tools/accessibilityFpMemoryFilter.test.ts | 178 ++++++++++++++ tests/tools/accessibilityReport.test.ts | 141 +++++++++++ tests/tools/fpMemory.test.ts | 230 ++++++++++++++++++ 8 files changed, 910 insertions(+), 32 deletions(-) create mode 100644 tests/tools/accessibilityFpMemoryFilter.test.ts diff --git a/capability/web_accessibility.capability-index.json b/capability/web_accessibility.capability-index.json index 78f15c92..609f24e0 100644 --- a/capability/web_accessibility.capability-index.json +++ b/capability/web_accessibility.capability-index.json @@ -479,7 +479,7 @@ "name": "pattern", "type": "object", "required": true, - "description": "What identifies the element and its kind. predicate is required; add what you observed.", + "description": "What identifies the element and its kind. predicate is required; add what you observed. Later fetchAccessibilityIssues answers leave out issues by tag, tags, tagPrefix and classes, never by predicate: set the ones that identify the element, and list in excludeTags any tag tagPrefix takes in that the root cause does not hold for.", "fields": [ { "name": "predicate", @@ -491,10 +491,20 @@ "name": "tag", "type": "string" }, + { + "name": "tags", + "type": "array", + "description": "The exact tags the root cause holds for, when it is a set rather than a prefix." + }, { "name": "tagPrefix", "type": "string" }, + { + "name": "excludeTags", + "type": "array", + "description": "Tags tagPrefix takes in that this entry does not cover; their issues are never left out." + }, { "name": "isCustomElement", "type": "boolean" @@ -979,7 +989,7 @@ ], "key_facts": [ "The memory belongs to the account: every user's agent reads it. An entry applies to the site it was saved on, or to all sites when global.", - "fetchAccessibilityIssues already brings the memory for the rules of the issues it answers with." + "fetchAccessibilityIssues leaves out the issues an entry names by its pattern's tag, tags, tagPrefix or classes, and brings the memory for the rules of the issues it answers with." ] } }, diff --git a/src/tools/accessibility.ts b/src/tools/accessibility.ts index a50c277c..59ee39c0 100644 --- a/src/tools/accessibility.ts +++ b/src/tools/accessibility.ts @@ -37,8 +37,13 @@ import { elicitCredentialsIfSupported } from "../lib/elicit-credentials.js"; import logger from "../logger.js"; import { NEEDS_A11Y_CONFIG_ID, NEEDS_A11Y_SCAN_ID } from "./tool-handoff.js"; import { + applyMemory, + leftOutMessages, memoryForReport, memoryMessages, + type FpMemory, + type LeftOut, + type MemoryIssue, } from "./accessiblity-utils/fp-memory.js"; interface AuthCredentials { @@ -946,6 +951,28 @@ function exportPendingMessages( ]; } +/** The memory read for a report, and how many issues each entry left out of it. */ +interface FpApplied { + memory?: FpMemory; + leftOut: LeftOut[]; +} + +/** + * The account's saved false positives for the report's rules: the issues they certainly + * cover are left out of every page and count, whether or not the agent thinks to ask for + * the memory. Records what it read and left out in `fp`, for the answer to say. + */ +function memoryLeaveOut(config: BrowserStackConfig, fp: FpApplied) { + return async (issues: T[]): Promise> => { + fp.memory = await memoryForReport(setupAuth(config), [ + ...new Set(issues.map((issue) => issue.issue_type)), + ]); + const { covered, leftOut } = applyMemory(fp.memory, issues); + fp.leftOut = leftOut; + return covered; + }; +} + async function fetchAccessibilityIssues( requested: RequestedSource, config: BrowserStackConfig, @@ -1020,6 +1047,7 @@ async function fetchAccessibilityIssues( } const pageArg = pageUrl ? `, pageUrl: "${pageUrl}"` : ""; + const fp: FpApplied = { leftOut: [] }; let reportLink: string; try { reportLink = await reportFetcher.getReportLink(source, { @@ -1051,13 +1079,8 @@ async function fetchAccessibilityIssues( } = await parseAccessibilityReportFromCSV(reportLink, { nextPage: cursor, pageUrl, + leaveOut: memoryLeaveOut(config, fp), }); - // The memory for this page's rules comes with the issues, so it is applied whether or - // not the agent thinks to ask for it. - const memory = await memoryForReport( - setupAuth(config), - Object.keys(rules ?? {}), - ); return createSuccessResponse([ ...preface, @@ -1067,9 +1090,11 @@ async function fetchAccessibilityIssues( // The whole report's counts, said on its first page. ...(cursor ? [] : webSummaryNote(summary, total_issues)), ...excludedIssuesNote(duplicate_issues, unlinked_duplicates, hidden_issues), + ...leftOutMessages(fp.leftOut), fullReportLine(reportLink), WEB_ISSUES_NOTE, - ...memoryMessages(memory), + // The rest of the memory for this page's rules, for the agent to apply with judgment. + ...memoryMessages(fp.memory, Object.keys(rules ?? {})), `Issues: ${wrapUntrusted("accessibility scan results", JSON.stringify({ issues: records, rules }))}`, closingMessage(next_page, remaining, source, stillProcessing, pageArg), ]); @@ -1485,6 +1510,7 @@ function createScanSuccessResponse( dashboardLink: string | undefined, settings: ScanSettings = {}, applied?: ScanSettings, + fp: FpApplied = { leftOut: [] }, ): CallToolResult { const cursor = report.next_page; const messages = [ @@ -1499,7 +1525,10 @@ function createScanSuccessResponse( report.unlinked_duplicates, report.hidden_issues, ), + ...leftOutMessages(fp.leftOut), WEB_ISSUES_NOTE, + // The rest of the memory for this page's rules, for the agent to apply with judgment. + ...memoryMessages(fp.memory, Object.keys(report.rules ?? {})), `Scan results: ${wrapUntrusted("accessibility scan results", JSON.stringify({ issues: report.records, rules: report.rules }))}`, ]; @@ -1575,15 +1604,20 @@ async function runAccessibilityScan( throw error; } + const fp: FpApplied = { leftOut: [] }; + const report = await parseAccessibilityReportFromCSV(reportLink, { + leaveOut: memoryLeaveOut(config, fp), + }); return createScanSuccessResponse( name, - await parseAccessibilityReportFromCSV(reportLink), + report, scanId, scanRunId, reportLink, dashboardLink, settings, applied, + fp, ); } diff --git a/src/tools/accessiblity-utils/fp-memory.ts b/src/tools/accessiblity-utils/fp-memory.ts index a7c8f1cf..053253a2 100644 --- a/src/tools/accessiblity-utils/fp-memory.ts +++ b/src/tools/accessiblity-utils/fp-memory.ts @@ -2,6 +2,7 @@ import { apiClient } from "../../lib/apiClient.js"; import { getRegionalHost } from "../../lib/regional-host.js"; import { wrapUntrusted } from "../../lib/untrusted-content.js"; import logger from "../../logger.js"; +import { samePage } from "./report-parser.js"; /** * The account's false-positive (FP) memory: what users and their agents said is not a real @@ -85,6 +86,203 @@ export async function memoryForReport( } } +/** What an issue is matched on. */ +export interface MemoryIssue { + issue_type: string; + affected_page: string; + HTML_snippet: string; +} + +/** How many issues one entry left out of an answer. */ +export interface LeftOut { + rule: string; + entry: number; + count: number; +} + +interface Element { + tag: string; + id: string; + classes: string[]; + /** The whole opening tag, whitespace collapsed; unset when the snippet cuts it short. */ + opening?: string; +} + +/** The element an issue's HTML snippet opens with. */ +function elementOf(snippet: string | undefined): Element | undefined { + const match = /^\s*<([a-z][a-z0-9-]*)([^>]*)(>?)/i.exec(snippet ?? ""); + if (!match) return undefined; + const [, tag, attributes, closed] = match; + const attribute = (name: string) => { + const found = new RegExp( + `(?:^|\\s)${name}\\s*=\\s*(?:"([^"]*)"|'([^']*)'|([^\\s"'>]+))`, + "i", + ).exec(attributes); + return found ? (found[1] ?? found[2] ?? found[3] ?? "") : ""; + }; + return { + tag: tag.toLowerCase(), + id: attribute("id").toLowerCase(), + classes: attribute("class").toLowerCase().split(/\s+/).filter(Boolean), + opening: closed + ? `<${tag}${attributes}>`.replace(/\s+/g, " ").toLowerCase() + : undefined, + }; +} + +const lowered = (value: unknown): string => + typeof value === "string" ? value.trim().toLowerCase() : ""; + +const loweredList = (value: unknown): string[] => + Array.isArray(value) ? value.map(lowered).filter(Boolean) : []; + +/** The words of a text, keeping the hyphens and underscores of tag and class names. */ +const wordsOf = (text: string): string[] => + text + .toLowerCase() + .split(/[^a-z0-9_-]+/) + .filter(Boolean); + +/** A sentence that says what an entry does NOT cover. */ +const EXCLUDING = + /\b(?:not|never|except|excluding|exclude[sd]?|unlike)\b|n't\b|other than/i; + +/** + * What an entry is matched on, from its pattern's fields and never from its prose. The one + * use of prose is to keep an issue: a tag or class named in a sentence of the predicate that + * excludes it ("Does not cover sbb-breadcrumb"), in an entry saved before excludeTags. + */ +function matcherOf(entry: FpEntry) { + const pattern = (entry.pattern ?? {}) as Record; + const tagName = (value: unknown) => + /^[a-z][a-z0-9-]*/.exec(lowered(value))?.[0] ?? ""; + const seed = elementOf(entry.seedIssue?.htmlSnippet); + // A new sentence starts with a capital: "e.g. button" does not split one. + const sentences = + typeof pattern.predicate === "string" + ? pattern.predicate.split(/(?<=[.!?])\s+(?=[A-Z(])|\n+/) + : []; + const kept = new Set([ + ...loweredList(pattern.excludeTags), + ...(entry.counterExamples ?? []).flatMap((example) => + wordsOf(typeof example === "string" ? example : JSON.stringify(example)), + ), + ...sentences + .filter((sentence) => EXCLUDING.test(sentence)) + .flatMap(wordsOf), + ]); + return { + entry, + site: lowered(entry.site), + scope: entry.matchScope, + seed, + seedPage: entry.seedIssue?.pageUrl, + tag: tagName(pattern.tag), + tags: loweredList(pattern.tags).map(tagName).filter(Boolean), + tagPrefix: lowered(pattern.tagPrefix).replace(/\*+$/, ""), + classes: loweredList(pattern.classes).map((c) => c.replace(/^\./, "")), + customOnly: pattern.isCustomElement === true, + kept, + }; +} + +type Matcher = ReturnType; + +function hostOf(url: string): string { + try { + return new URL(url).hostname.toLowerCase(); + } catch { + return ""; + } +} + +function covers(m: Matcher, issue: MemoryIssue, element: Element): boolean { + if (!m.entry.global && hostOf(issue.affected_page) !== m.site) return false; + // A counter-example or an excluded element is never left out. A plain tag (div, a) names + // too much to exclude by; a custom tag, a class or an id does not. + const names = [ + ...(element.tag.includes("-") ? [element.tag] : []), + ...element.classes, + ...(element.id ? [element.id] : []), + ]; + if (names.some((name) => m.kept.has(name))) return false; + if (m.customOnly && !element.tag.includes("-")) return false; + const hasClasses = m.classes.every((c) => element.classes.includes(c)); + + switch (m.scope) { + case "element": + return ( + element.opening !== undefined && + element.opening === m.seed?.opening && + samePage(issue.affected_page) === samePage(m.seedPage ?? "") + ); + case "component": { + const tag = m.tag || m.seed?.tag; + return Boolean(tag) && element.tag === tag && hasClasses; + } + case "component_family": + case "site_predicate": { + const byTag = Boolean(m.tag || m.tags.length || m.tagPrefix); + if (!byTag && m.classes.length === 0) return false; + const tagMatches = + !byTag || + element.tag === m.tag || + m.tags.includes(element.tag) || + (m.tagPrefix !== "" && element.tag.startsWith(m.tagPrefix)); + return tagMatches && hasClasses; + } + default: + return false; + } +} + +/** + * The issues the memory certainly covers, and how many each entry covers. An issue is left + * out only when an entry of its rule, for its site, names its element by the pattern's tag, + * tags, tagPrefix or classes, within the entry's scope. Everything else stays in the answer, + * where the agent applies the memory with judgment and asks the user when unsure. + */ +export function applyMemory( + memory: FpMemory | undefined, + issues: T[], +): { covered: Set; leftOut: LeftOut[] } { + const covered = new Set(); + const counts = new Map(); + const byRule = new Map( + (memory?.rules ?? []).map((rule) => [ + rule.ruleCode, + rule.entries.map(matcherOf), + ]), + ); + for (const issue of issues) { + const matchers = byRule.get(issue.issue_type); + const element = matchers?.length + ? elementOf(issue.HTML_snippet) + : undefined; + if (!matchers || !element) continue; + const match = matchers.find((m) => covers(m, issue, element)); + if (!match) continue; + covered.add(issue); + const tally = counts.get(match.entry.id) ?? { + rule: issue.issue_type, + entry: match.entry.id, + count: 0, + }; + tally.count++; + counts.set(match.entry.id, tally); + } + return { covered, leftOut: [...counts.values()] }; +} + +/** What an answer says about the issues the memory left out of it. */ +export function leftOutMessages(leftOut: LeftOut[]): string[] { + const total = leftOut.reduce((sum, l) => sum + l.count, 0); + if (total === 0) return []; + return [ + `False-positive memory: left out ${total} issue${total === 1 ? "" : "s"} the account's saved false positives cover. They are in none of the issues or counts here; the dashboard and the CSV export still list them. Tell the user how many were left out and by which entries: ${wrapUntrusted("false positives left out (rule, entry id, issues)", JSON.stringify(leftOut))}`, + ]; +} + function compactEntry(entry: FpEntry) { const snippet = entry.seedIssue?.htmlSnippet ?? ""; return { @@ -146,13 +344,25 @@ export function memoryPayload( } export const FP_APPLY_NOTE = - "False-positive memory: below are issues this account's users already confirmed are not real, saved per rule. Before showing the issues, remove the ones the memory covers, and do not ask the user about them again. Apply an entry only to issues of the same rule (issue_type), on its site (the host of an issue's affected_page) unless it says all sites. Match on its pattern and root_cause, never on selector or exact HTML: ids, selectors and labels differ per page and language. Remove automatically only up to the entry's scope (element: that element; component: the same component; component_family: any component its pattern.predicate describes; site_predicate: anything on the site the predicate describes). Never remove a counter_example. List issues that look like the same pattern but fall outside the scope as likely false positives and ask the user once per group; if they agree, save that at the wider scope. Say how many issues you removed per rule, with the entry ids. Each rule's docs link explains what the rule checks."; + "False-positive memory: below are issues this account's users already confirmed are not real, saved per rule. Issues an entry names by its pattern's tag, tags, tagPrefix or classes are already left out of this answer and its counts. Before showing the issues, remove any other one the memory covers, and do not ask the user about them again. Apply an entry only to issues of the same rule (issue_type), on its site (the host of an issue's affected_page) unless it says all sites. Match on its pattern and root_cause, never on selector or exact HTML: ids, selectors and labels differ per page and language. Remove automatically only up to the entry's scope (element: that element; component: the same component; component_family: any component its pattern.predicate describes; site_predicate: anything on the site the predicate describes). Never remove a counter_example, or an element the predicate excludes. List issues that look like the same pattern but fall outside the scope as likely false positives and ask the user once per group; if they agree, save that at the wider scope. Say how many issues were left out or you removed per rule, with the entry ids. Each rule's docs link explains what the rule checks."; -export const FP_FILING_GUIDE = `When the user says an issue is not a real problem, save it with the web_accessibility capability ${SUBMIT_CAPABILITY} (invokeCapability), so later reports are filtered without asking again. First inspect the element yourself (open the page if you have a browser, else read the issue's HTML): what it is built from (tag, custom element, shadow root, classes, design system) and why the rule is wrong about it. Write rootCause about how the element is built, not only that it works for the user, and pattern.predicate as the widest set of elements that root cause covers (e.g. every custom element of the same design system). Propose that scope and ask the user to confirm it, and to name any similar element that is a real issue (counterExamples). Quote the user as they said it in userStatement. Mark global only when the user says it holds on all their sites.`; +export const FP_FILING_GUIDE = `When the user says an issue is not a real problem, save it with the web_accessibility capability ${SUBMIT_CAPABILITY} (invokeCapability), so later reports are filtered without asking again. First inspect the element yourself (open the page if you have a browser, else read the issue's HTML): what it is built from (tag, custom element, shadow root, classes, design system) and why the rule is wrong about it. Write rootCause about how the element is built, not only that it works for the user, and pattern.predicate as the widest set of elements that root cause covers (e.g. every custom element of the same design system). Later answers leave issues out by the pattern's fields, never by its prose: set tag, tags (the exact tags it holds for) or tagPrefix, and classes where they identify the element, and list in excludeTags any tag the prefix takes in that the root cause does not hold for. Propose that scope and ask the user to confirm it, and to name any similar element that is a real issue (counterExamples). Quote the user as they said it in userStatement. Mark global only when the user says it holds on all their sites.`; -/** What an issues answer adds for the memory: how to apply it, the memory, how to add to it. */ -export function memoryMessages(memory: FpMemory | undefined): string[] { +/** + * What an issues answer adds for the memory: how to apply it, the memory, how to add to it. + * With `ruleCodes`, only those rules' entries: the ones the answer's issues name. + */ +export function memoryMessages( + memory: FpMemory | undefined, + ruleCodes?: string[], +): string[] { if (!memory) return []; + if (ruleCodes) { + memory = { + ...memory, + rules: memory.rules.filter((rule) => ruleCodes.includes(rule.ruleCode)), + }; + } if (memory.rules.length === 0) return [FP_FILING_GUIDE]; const { json, omitted } = memoryPayload(memory); return [ diff --git a/src/tools/accessiblity-utils/paginate.ts b/src/tools/accessiblity-utils/paginate.ts index d0b66e1f..5323ba08 100644 --- a/src/tools/accessiblity-utils/paginate.ts +++ b/src/tools/accessiblity-utils/paginate.ts @@ -58,6 +58,7 @@ export function pageOf( maxCharacterLength = PAGE_CHARACTERS, nextPage = 0, extraLength, + skip, }: { maxCharacterLength?: number; nextPage?: number; @@ -66,6 +67,11 @@ export function pageOf( * the cursor: the text of a rule the page has not named yet. */ extraLength?: (record: T) => number; + /** + * Rows left out of every page: each still moves the cursor by its length, so the + * offsets an agent holds stay valid when what is left out changes between calls. + */ + skip?: (record: T) => boolean; } = {}, ): Page { // Walk to the starting offset @@ -80,20 +86,23 @@ export function pageOf( // Collect up to maxCharacterLength; a row larger than that still makes a page of its // own, or the cursor would never move past it const page: T[] = []; - let extra = 0; - for (let i = idx; i < all.length; i++) { + let used = charCursor - nextPage; + let i = idx; + for (; i < all.length; i++) { const recStr = JSON.stringify(all[i]); + if (skip?.(all[i])) { + charCursor += recStr.length; + continue; + } const more = extraLength?.(all[i]) ?? 0; - const fits = - charCursor - nextPage + extra + more + recStr.length <= - maxCharacterLength; + const fits = used + more + recStr.length <= maxCharacterLength; if (!fits && page.length > 0) break; page.push(all[i]); charCursor += recStr.length; - extra += more; + used += more + recStr.length; } - const remaining = all.length - idx - page.length; + const remaining = all.slice(i).filter((record) => !skip?.(record)).length; return { records: page, page_length: page.length, diff --git a/src/tools/accessiblity-utils/report-parser.ts b/src/tools/accessiblity-utils/report-parser.ts index 25db2258..4fca04a3 100644 --- a/src/tools/accessiblity-utils/report-parser.ts +++ b/src/tools/accessiblity-utils/report-parser.ts @@ -9,7 +9,7 @@ import { sortBySeverity, } from "./paginate.js"; -type SimplifiedAccessibilityIssue = { +export type SimplifiedAccessibilityIssue = { issue_type: string; component: string; issue_description: string; @@ -90,6 +90,13 @@ type PaginationOptions = { nextPage?: number; /** Only the issues on this page. */ pageUrl?: string; + /** + * The issues to leave out of the pages and counts (those the account's saved false + * positives cover), given every issue read. + */ + leaveOut?: ( + issues: SimplifiedAccessibilityIssue[], + ) => Promise>; }; type PaginatedResult = { @@ -134,7 +141,7 @@ function isMainIssue(row: Record): boolean { } /** A page address as the report writes it, give or take host case, trailing slash and hash. */ -function samePage(url: string): string { +export function samePage(url: string): string { try { const u = new URL(url.trim()); return `${u.protocol}//${u.host.toLowerCase()}${u.pathname.replace(/\/+$/, "")}${u.search}`; @@ -273,17 +280,28 @@ function summaryOf( * counts of all of them. Most severe first, and within a severity rule by rule, the most * common rule first: a page then names fewer rules, and a rule's text, counted against the * page's length the first time the page names it, is said once for all its issues there. + * + * Issues in `leftOut` are on no page and in no count, but the rule texts, the order and the + * cursor are those of every issue: a page's offsets then hold whatever is left out. */ function pageWithRules( all: SimplifiedAccessibilityIssue[], rows: Record[], options: { maxCharacterLength: number; nextPage: number }, + leftOut: ReadonlySet = new Set(), ) { const rules = rulesOf(rows, all); - const summary = summaryOf(all, rules); + const everyIssue = summaryOf(all, rules); + const summary = leftOut.size + ? summaryOf( + all.filter((issue) => !leftOut.has(issue)), + rules, + ) + : everyIssue; const ruleRank = new Map( - summary.by_rule.map(({ issue_type }, i) => [issue_type, i]), + everyIssue.by_rule.map(({ issue_type }, i) => [issue_type, i]), ); + const skipped = new Set(); const ordered = all .map((issue, i) => ({ issue, i })) .sort( @@ -293,10 +311,15 @@ function pageWithRules( (ruleRank.get(b.issue.issue_type) ?? 0) || a.i - b.i, ) - .map(({ issue }) => withoutRuleText(issue, rules[issue.issue_type])); + .map(({ issue }) => { + const row = withoutRuleText(issue, rules[issue.issue_type]); + if (leftOut.has(issue)) skipped.add(row); + return row; + }); const named = new Set(); const page = pageOf(ordered, { ...options, + skip: (row) => skipped.has(row), extraLength: (row) => { if (named.has(row.issue_type)) return 0; named.add(row.issue_type); @@ -421,6 +444,31 @@ function pageIssues( return { all, page }; } +/** A page's counts without the issues left out, and every occurrence of them on it. */ +function pageWithout( + page: PageSummary, + leftOut: ReadonlySet, +): PageSummary { + if (leftOut.size === 0) return page; + const bySeverity = { ...page.by_severity }; + let occurrences = page.occurrences; + let firstFoundHere = page.first_found_here; + for (const issue of leftOut) { + const count = issue.occurrences_on_page ?? 0; + occurrences -= count; + bySeverity[issue.severity] = (bySeverity[issue.severity] ?? 0) - count; + if (bySeverity[issue.severity] <= 0) delete bySeverity[issue.severity]; + if (!issue.first_found_on) firstFoundHere--; + } + return { + ...page, + occurrences, + distinct_issues: page.distinct_issues - leftOut.size, + first_found_here: firstFoundHere, + by_severity: bySeverity, + }; +} + /** A main issue as two runs are matched on: the rule, the element and the page. */ export type RunIssue = { key: string; @@ -572,6 +620,7 @@ export async function parseAccessibilityReportFromCSV( maxCharacterLength = PAGE_CHARACTERS, nextPage = 0, pageUrl, + leaveOut, }: PaginationOptions = {}, ): Promise { // 1) Download & parse @@ -579,14 +628,20 @@ export async function parseAccessibilityReportFromCSV( if (pageUrl) { const { all, page } = pageIssues(parsed, columns, pageUrl); - const slice = pageWithRules(all, parsed, { maxCharacterLength, nextPage }); + const leftOut = (await leaveOut?.(all)) ?? new Set(); + const slice = pageWithRules( + all, + parsed, + { maxCharacterLength, nextPage }, + leftOut, + ); return { ...slice, - total_issues: all.length, + total_issues: all.filter((issue) => !leftOut.has(issue)).length, duplicate_issues: null, unlinked_duplicates: 0, hidden_issues: null, - page, + page: pageWithout(page, leftOut), }; } @@ -645,12 +700,23 @@ export async function parseAccessibilityReportFromCSV( else unlinkedDuplicates++; } - // 3) Most severe first, in pages by JSON length, each rule's text once per page - const page = pageWithRules(all, parsed, { maxCharacterLength, nextPage }); + // 3) Leave out what the caller says to, with its repeats. + const leftOut = (await leaveOut?.(all)) ?? new Set(); + for (const issue of all) { + if (leftOut.has(issue)) duplicates -= issue.duplicate_count ?? 0; + } + + // 4) Most severe first, in pages by JSON length, each rule's text once per page + const page = pageWithRules( + all, + parsed, + { maxCharacterLength, nextPage }, + leftOut, + ); return { ...page, - total_issues: all.length, + total_issues: all.filter((issue) => !leftOut.has(issue)).length, duplicate_issues: grouped ? duplicates : null, unlinked_duplicates: unlinkedDuplicates, hidden_issues: columns.includes(STATUS_COLUMN) diff --git a/tests/tools/accessibilityFpMemoryFilter.test.ts b/tests/tools/accessibilityFpMemoryFilter.test.ts new file mode 100644 index 00000000..0f29e71e --- /dev/null +++ b/tests/tools/accessibilityFpMemoryFilter.test.ts @@ -0,0 +1,178 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// fetchAccessibilityIssues and the account's false-positive memory: the memory is read for +// the rules of every issue in the report, the issues it certainly covers are left out of +// the answer and said to be, and the rest of the memory comes with the issues that remain. + +const m = vi.hoisted(() => ({ + getReportLink: vi.fn(), + parse: vi.fn(), + memoryForReport: vi.fn(), +})); + +vi.mock("../../src/tools/accessiblity-utils/report-fetcher.js", () => ({ + AccessibilityReportFetcher: class { + setAuth = vi.fn(); + getReportLink = m.getReportLink; + reportStatus = vi.fn().mockResolvedValue("completed"); + resolveBuild = vi.fn(async (source: unknown) => source); + }, + EXPORT_WAIT_MS: 45_000, + ExportPendingError: class extends Error {}, +})); +vi.mock("../../src/tools/accessiblity-utils/report-parser.js", () => ({ + parseAccessibilityReportFromCSV: m.parse, + loadMainIssues: vi.fn(), + compareIssues: vi.fn(), +})); +vi.mock( + "../../src/tools/accessiblity-utils/fp-memory.js", + async (importOriginal) => ({ + ...(await importOriginal()), + memoryForReport: m.memoryForReport, + }), +); +vi.mock("../../src/lib/get-auth.js", () => ({ + getBrowserStackAuth: vi.fn().mockReturnValue("fake-user:fake-key"), +})); +vi.mock("../../src/logger.js", () => ({ + default: { error: vi.fn(), info: vi.fn(), debug: vi.fn(), warn: vi.fn() }, +})); +vi.mock("../../src/lib/instrumentation.js", () => ({ trackMCP: vi.fn() })); +vi.mock("../../src/index.js", () => ({ + trackMCP: vi.fn(), + PackageJsonVersion: "0.0.0", +})); + +import addAccessibilityTools from "../../src/tools/accessibility.js"; +import { FP_APPLY_NOTE } from "../../src/tools/accessiblity-utils/fp-memory.js"; + +function fetchIssues() { + let handler: (...args: any[]) => any = () => undefined; + const server: any = { + tool: vi.fn((...args: any[]) => { + if (args[0] === "fetchAccessibilityIssues") + handler = args[args.length - 1]; + }), + server: { getClientVersion: vi.fn().mockReturnValue({ version: "1.0" }) }, + }; + addAccessibilityTools(server, { + "browserstack-username": "fake-user", + "browserstack-access-key": "fake-key", + } as any); + return (args: Record) => + handler(args, { sendNotification: vi.fn(), _meta: {} }); +} + +const text = (result: any) => + result.content.map((c: { text: string }) => c.text).join("\n"); + +const PAGE = "https://www.sbb.ch/de/angebote/halbtax"; +const issue = (issue_type: string, HTML_snippet: string) => ({ + issue_type, + HTML_snippet, + affected_page: PAGE, + component: "sbb#", + severity: "serious", +}); +const ISSUES = [ + issue("role-required", ''), + issue("role-required", ''), + issue("image-alt", ' '), +]; + +const SBB_MEMORY = { + truncated: false, + rules: [ + { + ruleCode: "role-required", + docsUrl: "https://www.browserstack.com/docs/role-required/", + entryCount: 1, + entries: [ + { + id: 3, + site: "www.sbb.ch", + global: false, + verdict: "false_positive", + matchScope: "component_family", + rootCause: + "SBB Lyne components set their role through ElementInternals, not a role attribute.", + pattern: { + predicate: + "Custom elements with tag prefix 'sbb-'. Does not cover sbb-breadcrumb.", + tagPrefix: "sbb-", + }, + userStatement: "this sbb component is a false positive", + }, + ], + }, + ], +}; + +/** The parser as the handler drives it: asks what to leave out of every issue read. */ +function parseLeavingOut() { + m.parse.mockImplementation( + async ( + _link: string, + { leaveOut }: { leaveOut: (i: unknown[]) => Promise> }, + ) => { + const leftOut = await leaveOut(ISSUES); + const records = ISSUES.filter((i) => !leftOut.has(i)); + return { + records, + rules: Object.fromEntries( + records.map((r) => [ + r.issue_type, + { description: r.issue_type, how_to_fix: "fix" }, + ]), + ), + page_length: records.length, + total_issues: records.length, + next_page: null, + remaining: 0, + duplicate_issues: null, + unlinked_duplicates: 0, + hidden_issues: null, + }; + }, + ); +} + +beforeEach(() => { + vi.clearAllMocks(); + m.getReportLink.mockResolvedValue("https://csv"); + parseLeavingOut(); +}); + +describe("fetchAccessibilityIssues with a false-positive memory", () => { + it("reads the memory for every rule in the report and leaves out what it certainly covers", async () => { + m.memoryForReport.mockResolvedValue(SBB_MEMORY); + + const answer = text( + await fetchIssues()({ scanId: "4304785", scanRunId: "2425482" }), + ); + + expect(m.memoryForReport).toHaveBeenCalledWith( + { username: "fake-user", password: "fake-key" }, + ["role-required", "image-alt"], + ); + expect(answer).toContain("left out 1 issue the account's saved"); + expect(answer).toContain('"entry":3'); + const issues = answer.slice(answer.indexOf("Issues:")); + expect(issues).not.toContain(" { + m.memoryForReport.mockResolvedValue(undefined); + + const answer = text( + await fetchIssues()({ scanId: "4304785", scanRunId: "2425482" }), + ); + + expect(answer).not.toMatch(/False-positive memory/); + expect(answer).toContain(" { expect(records.every((r) => !("source_test" in r))).toBe(true); }); }); + +describe("parseAccessibilityReportFromCSV leaving issues out", () => { + beforeEach(() => vi.clearAllMocks()); + + const leaveOutRule = + (rule: string) => async (issues: { issue_type: string }[]) => + new Set(issues.filter((issue) => issue.issue_type === rule)); + + it("leaves them and their repeats out of the issues and every count", async () => { + get.mockResolvedValue(csvResponse(toCsv(COLUMNS, GROUPED_ROWS))); + const leaveOut = vi.fn(leaveOutRule("color-contrast")); + + const result = await parseAccessibilityReportFromCSV("https://csv", { + leaveOut: leaveOut as any, + }); + + // Asked about every issue read, not one page's. + expect(leaveOut.mock.calls[0][0].map((i: any) => i.issue_type)).toEqual([ + "color-contrast", + "image-alt", + ]); + expect(result.records.map((r) => r.issue_type)).toEqual(["image-alt"]); + expect(result.total_issues).toBe(1); + expect(result.summary.by_rule).toEqual([ + { issue_type: "image-alt", count: 1 }, + ]); + expect(result.summary.by_severity).toEqual({ critical: 1 }); + // The two repeats of the issue left out go with it; the unlinked one stays counted. + expect(result.duplicate_issues).toBe(1); + expect(result.unlinked_duplicates).toBe(1); + }); + + it("keeps the cursor an agent holds when an issue it already read is left out next time", async () => { + // Six issues of one length, one to a page, so a cursor that moved with the issues left + // out would land a whole issue late and skip one. + const csv = [ + "Issue type,Severity,Component,Issue description,HTML snippet,How to fix this issue", + ...[1, 2, 3, 4, 5, 6].map( + (n) => + `region,moderate,div,Not in a landmark,
,Add a landmark`, + ), + ].join("\n"); + get.mockResolvedValue(csvResponse(csv)); + const ids = (records: { HTML_snippet: string }[]) => + records.map((r) => r.HTML_snippet); + + const first = await parseAccessibilityReportFromCSV("https://csv", { + maxCharacterLength: 1, + }); + expect(ids(first.records)).toEqual(["
"]); + + // The user says the first one is a false positive; the agent reads on. + const leaveOut = async (issues: { HTML_snippet: string }[]) => + new Set(issues.filter((i) => i.HTML_snippet === "
")); + const seen: string[] = []; + let cursor = first.next_page; + while (cursor !== null) { + const page = await parseAccessibilityReportFromCSV("https://csv", { + maxCharacterLength: 1, + nextPage: cursor, + leaveOut: leaveOut as any, + }); + seen.push(...ids(page.records)); + cursor = page.next_page; + } + + expect(seen).toEqual([2, 3, 4, 5, 6].map((n) => `
`)); + }); + + it("hands out every other issue once when issues between pages are left out", async () => { + const csv = [ + "Issue type,Severity,Component,Issue description,HTML snippet,How to fix this issue", + ...[1, 2, 3, 4, 5].map( + (n) => + `region,moderate,div,Not in a landmark,
,Add a landmark`, + ), + ].join("\n"); + get.mockResolvedValue(csvResponse(csv)); + const leaveOut = async (issues: { HTML_snippet: string }[]) => + new Set( + issues.filter((i) => + ["
", "
"].includes(i.HTML_snippet), + ), + ); + + const seen: string[] = []; + let cursor: number | null = 0; + // Bounded, so a cursor that stops moving fails the test instead of hanging it. + for (let calls = 0; cursor !== null && calls < 10; calls++) { + const page = await parseAccessibilityReportFromCSV("https://csv", { + maxCharacterLength: 1, + nextPage: cursor, + leaveOut: leaveOut as any, + }); + seen.push(...page.records.map((r) => r.HTML_snippet)); + cursor = page.next_page; + } + + expect(seen).toEqual(["
", "
", "
"]); + expect(cursor).toBeNull(); + }); + + it("does not let an issue left out take a place on its page", async () => { + const csv = [ + "Issue type,Severity,Component,Issue description,HTML snippet,How to fix this issue", + ...[1, 2, 3].map( + (n) => + `region,moderate,div,Not in a landmark,
,Add a landmark`, + ), + ].join("\n"); + get.mockResolvedValue(csvResponse(csv)); + + const page = await parseAccessibilityReportFromCSV("https://csv", { + maxCharacterLength: 1, + leaveOut: (async (issues: { HTML_snippet: string }[]) => + new Set(issues.filter((i) => i.HTML_snippet !== "
"))) as any, + }); + + expect(page.records.map((r) => r.HTML_snippet)).toEqual(["
"]); + expect(page.next_page).toBeNull(); + expect(page.remaining).toBe(0); + }); + + it("counts one page's occurrences without the issues left out", async () => { + get.mockResolvedValue(csvResponse(toCsv(COLUMNS, GROUPED_ROWS))); + + const result = await parseAccessibilityReportFromCSV("https://csv", { + pageUrl: "https://site.example/", + leaveOut: leaveOutRule("color-contrast") as any, + }); + + expect(result.records.map((r) => r.issue_type)).toEqual(["image-alt"]); + expect(result.total_issues).toBe(1); + expect(result.page).toMatchObject({ + occurrences: 1, + distinct_issues: 1, + first_found_here: 1, + by_severity: { critical: 1 }, + }); + }); +}); diff --git a/tests/tools/fpMemory.test.ts b/tests/tools/fpMemory.test.ts index a4b9e157..583bf3bd 100644 --- a/tests/tools/fpMemory.test.ts +++ b/tests/tools/fpMemory.test.ts @@ -21,6 +21,8 @@ import { FP_FILING_GUIDE, READ_CAPABILITY, SUBMIT_CAPABILITY, + applyMemory, + leftOutMessages, memoryForReport, memoryMessages, memoryPayload, @@ -138,6 +140,234 @@ describe("memoryMessages", () => { "all sites", ); }); + + it("brings only the rules the answer's issues name, when told which", () => { + expect(memoryMessages(memory([entry(1)]), ["image-alt"])).toEqual([ + FP_FILING_GUIDE, + ]); + expect(memoryMessages(memory([entry(1)]), ["role-required"])[0]).toBe( + FP_APPLY_NOTE, + ); + }); +}); + +describe("applyMemory", () => { + const SBB = "https://www.sbb.ch/de/angebote/halbtax"; + + /** Entry 3 as the Accessibility API stores it for www.sbb.ch. */ + const sbbEntry = entry(3, { + site: "www.sbb.ch", + counterExamples: null, + pattern: { + predicate: + "Custom elements with tag prefix 'sbb-' (SBB Lyne design system) that have no role attribute on the host but set a role through ElementInternals (internals.role, e.g. button or radio): sbb-expansion-panel-header, sbb-radio-button-panel, sbb-header-button, sbb-button, sbb-mini-button, sbb-secondary-button, sbb-block-link-button, sbb-card-button, sbb-tag, sbb-datepicker-toggle, sbb-datepicker-next-day, sbb-datepicker-previous-day. Does not cover sbb-breadcrumb (no internals role; it renders a native in its shadow root) or non-sbb elements.", + tagPrefix: "sbb-", + designSystem: "SBB Lyne", + hasShadowRoot: true, + isCustomElement: true, + }, + seedIssue: { + pageUrl: SBB, + component: "sbb#", + htmlSnippet: + '', + }, + }); + + const issue = ( + HTML_snippet: string, + extra: { issue_type?: string; affected_page?: string } = {}, + ) => ({ + issue_type: "role-required", + affected_page: SBB, + HTML_snippet, + ...extra, + }); + + it("leaves out the family a tag prefix names, on its site and rule only", () => { + const issues = { + panelHeader: issue( + '