Add a skill for benchmarking (and other improvements) by mfairchild365 · Pull Request #18 · microsoft/a11y-llm-eval

mfairchild365 · 2026-04-23T17:41:34Z

This PR:

Adds a skill for benchmarking (a11y-wizard)
Adjusts agentic benchmarking to better handle skills (e.g., limit adjustments, add support for skill benchmarking configs, etc.)
Fixes several bugs found in test cases / assertions while reviewing results

…target

…er content

…AA guide

…ose warnings

… mount

Sets cache_prompt=true on the Inspect GenerateConfig for Anthropic / Claude models across both the plain chat-completion path and the ReAct agent path, so the provider applies ephemeral cache_control to system / tools / last user segments and repeated prefixes are billed at the cached rate. The new flag is plumbed through GenerationRequest into the runtime, with the per-request value set by _build_generation_request for direct calls and forwarded through generate_html_batch_with_meta for batched calls.

…ault output/token caps Adds a limit-aware break inside the agent truncation retry loop in generate_html_with_agent_meta: when result.limit_error is set (working/time/token/cost/message), a retry would hit the same wall and double the spend with no improvement, so the loop exits instead. Also raises default_agent_limits() defaults to max_output_tokens=65536 (64*1024) and token_limit=1048576 (1024*1024) so Claude Opus and similar long-output models do not hit per-turn max_tokens on large single-file HTML outputs.

…umn header - Wrap per-sample tokens display in the Jinja TEMPLATE with 'is defined and ... is not none' checks for total_tokens, tokens_in, and tokens_out. Samples missing token usage keys previously raised TypeError: unsupported format string passed to Undefined.__format__ when '{:,}'.format() received a Jinja Undefined. - Update tests/test_report_not_applicable.py assertions from '<th>WCAG Pass Rate*</th>' to '<th>Pass rate*</th>' to match the column rename in 756de74 (chore: annotate report pass-rate columns with automated-check caveat).

Copilot

Pull request overview

Adds first-class skills benchmarking (multi-turn, sandbox-mounted packages) to the A11y LLM Evaluation Harness, including schema/report updates, new default skill content (a11y-wizard), and several related test-case/assertion fixes.

Changes:

Introduces --skills-file support with per-turn generation, caching, artifacts, and results.json schema fields for turn metadata.
Extends HTML report rendering with a Skills section, plus updated “pass rate” labeling/notes and richer sample cards.
Adds the a11y-wizard skill package and updates/expands test cases and unit tests around skills + visual label extraction.

Reviewed changes

Copilot reviewed 38 out of 39 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/test_skills_loader.py	New unit tests for skills YAML loading, hashing, prompt token substitution, and per-turn cache identity.
tests/test_report_not_applicable.py	Updates expected report header text for renamed “Pass rate*”.
tests/test_get_visual_label.py	Adds regression coverage for block-displayed inline elements in visual-label extraction.
test_cases/shopping-home-page/test.js	Improves assertion diagnostics; adds NA handling for footer; refines skip-link validation.
test_cases/shopping-home-page/examples/na-no-footer-content.html	Adds new NA fixture for “no footer-like content” scenario.
test_cases/shopping-home-page/examples/fail-skip-target-not-focusable.html	Removes an obsolete fixture aligned with updated skip-link logic.
test_cases/modal-dialog/test.js	Accepts `alertdialog` in addition to `dialog` for modal detection.
node_runner/helpers/get-visual-label.js	Inserts whitespace around block-displayed descendants to avoid concatenated labels.
docs/features-and-acceptance.md	Documents Anthropic caching, agent limits updates, and new Skills benchmark feature/criteria.
config/skills/a11y-wizard/reference/testing.md	Adds detailed testing guidance (Playwright/axe strategies, runtime probing, token discipline).
config/skills/a11y-wizard/reference/tables-grids.md	New reference guidance for table/grid semantics.
config/skills/a11y-wizard/reference/structure.md	New reference guidance for landmarks/headings/title.
config/skills/a11y-wizard/reference/status-messages.md	New reference guidance for live regions/status messages.
config/skills/a11y-wizard/reference/reflow.md	New reference guidance for reflow/320px behavior.
config/skills/a11y-wizard/reference/navigation.md	New reference guidance for nav/menus patterns.
config/skills/a11y-wizard/reference/keyboard-focus.md	New reference guidance for keyboard/focus, bypass blocks, overlays.
config/skills/a11y-wizard/reference/images-graphics.md	New reference guidance for images/icons text alternatives.
config/skills/a11y-wizard/reference/contrast-forced-colors.md	New reference guidance for contrast + forced-colors adaptation.
config/skills/a11y-wizard/components/single-checkbox.md	New component pattern for standalone checkbox.
config/skills/a11y-wizard/components/radio-button-group.md	New component pattern for radio groups.
config/skills/a11y-wizard/components/page-layout.md	New component pattern for full-page layout/accessibility structure.
config/skills/a11y-wizard/components/modal-dialog.md	New component pattern for dialogs (native `<dialog>` + ARIA fallback).
config/skills/a11y-wizard/components/forms.md	New component pattern for forms (labels, errors, required, focus-on-submit).
config/skills/a11y-wizard/components/disclosure-widget.md	New component pattern for disclosures.
config/skills/a11y-wizard/components/checkbox-group.md	New component pattern for checkbox groups.
config/skills/a11y-wizard/components/README.md	Index of component pattern files for the skill.
config/skills/a11y-wizard/SKILL.md	Adds the `a11y-wizard` skill constitution + checklist used in sandbox runs.
config/skills/a11y-wizard/AUTHORING.md	Authoring guidance for skill structure and token minimization.
config/instructions/accessible-basic.md	Expands basic accessibility instruction set into a structured, detailed markdown doc.
config/inspect_agent_sandbox/compose.yaml	Improves sandbox startup (install pinned Playwright py pkg, env, healthcheck) and reduces noisy output.
config/inspect_agent_sandbox/Dockerfile	Adds trivial Dockerfile to avoid compose “No services to build” warnings.
config/default_skills.yaml	Adds default skills config that benchmarks `a11y-wizard` over multiple turns.
a11y_llm_tests/schema.py	Extends schema with prompt variant kind + skill/turn metadata fields.
a11y_llm_tests/report.py	Adds Skills section, pass-rate note, per-turn transcript handling, and updated labeling.
a11y_llm_tests/inspect_runtime.py	Adds `cache_prompt` plumbing; improves agent limits, tool timeout control, skill mounts, and seeded conversations.
a11y_llm_tests/generator.py	Adds multi-turn skill generation + per-turn caching, prompt rendering, and cache safety for limit-hit transcripts.
a11y_llm_tests/cli.py	Adds `--skills-file` CLI path; task planning/artifacts for per-turn outputs; updated progress + summaries.
README.md	Documents skills benchmarking usage and artifacts; adds combined instruction-set + skills guidance.

Conditionally render the 'Instruction sets' and 'Skills' report-nav links and their <section> wrappers so they only appear when instruction_benchmark_rows/instruction_set_analysis or skill_benchmark_tables are present. Remove the empty-state fallback paragraphs previously shown for non-skill/non-instruction runs. Update docs/features-and-acceptance.md acceptance criteria to note that the corresponding report-nav link is also omitted when the feature is not enabled.

The YAML key for a skill's directory is skill_dir (per a11y_llm_tests/cli.py::_load_skills and README.md), not skill_path. skill_path is reserved for the resolved absolute path stored in meta.prompt_variants and for the {{skill_path}} prompt template token that resolves to the sandbox mount path.

The previous description claimed failing/subsequent turns produced ERROR records with an aborted_reason field. Neither result = "ERROR" nor aborted_reason exist on ResultRecord or anywhere in the schema/CLI path. Actual behavior: the failing turn's record is written with empty HTML and generation.agent_limit_error set to the reason; subsequent turns for the same sample are short-circuited with empty HTML and generation.agent_limit_error populated, and the stitched agent conversation sidecar marks those turn entries skipped: true with a skip_reason. Empty HTML fails node-runner checks, so those turns evaluate to result = "FAIL". Earlier turns remain evaluated normally.

The Skills benchmarking section referenced a non-existent a11y-reviewer skill at config/skills/a11y-reviewer. The actual default skill (per config/default_skills.yaml and config/skills/) is a11y-wizard at skills/a11y-wizard (resolved relative to the skills YAML). Updated README text and example YAML to use the correct id and skill_dir.

…nt->relevant)

scottaohara · 2026-04-29T14:15:56Z

+- Do not mark the whole group required on the `<fieldset>` itself. If the form requires "at least one" selection, enforce it with script-level validation and expose the requirement in the `<legend>` and (on submit failure) via an associated error message.
+- If a single option is required (e.g., "I agree"), mark that one checkbox `required` and visibly indicate it next to that option's label.
+
+## Web implementation


this generally looks good, but why would this limit to only using native HTML elements? The skill won't be useful or could lead to unnecessary changes if a team was using a custom ARIA implementation - which is unfortunately rather common, but fortunately custom ARIA checkboxes / custom ARIA groups are fairly straight forward to get "right"

Yeah, this is tricky. The native elements are just referenced for a few reasons:

To keep token count minimal (reduce costs and impact on context window)

The SKILL.md file actually outlines implementation priority. e.g., (exisiting local component > component from a library > native > native + aria > aria only.

So the LLM should read these sub files with that context in mind.

Does that address your concern?

scottaohara · 2026-04-29T14:17:16Z

+
+### General defaults
+
+- Trigger: `<button type="button" aria-expanded="false" aria-controls="panelId">`.


do we really care about aria-controls in this context? it does little for a pattern like this, and would introduce the need for an associated ID ref, and we all know how much people hate having to use IDs....

scottaohara · 2026-04-29T14:19:18Z

+### General defaults
+
+- Trigger: `<button type="button" aria-expanded="false" aria-controls="panelId">`.
+- Panel: a sibling element (`<div>`, `<section>`, etc.) with `id="panelId"` and `hidden` when collapsed.


using hidden is fine... so long as it is dynamically added and not part of the initial DOM construction of the disclosure. we've talked about this before, and how if the hidden attribute is used by default, and on pages where the content should be available in a browser's reader mode, that content will become hidden for everyone since the CSS/JS will be dropped from the page.

scottaohara · 2026-04-29T14:19:54Z

+    Why do cats purr?
+  </button>
+</h3>
+<div id="cat-panel" hidden>


per my prior comment on hidden - why not a "hidden" / display-none css class?

scottaohara · 2026-04-29T14:21:26Z

+      const panel = document.getElementById(btn.getAttribute('aria-controls'));
+      const open = btn.getAttribute('aria-expanded') === 'true';
+      btn.setAttribute('aria-expanded', String(!open));
+      panel.hidden = open;


open is not a valid value for this attribute. this should be true

open is a variable, not a string. It evals to true or false.

scottaohara · 2026-04-29T14:55:06Z

+</div>
+```
+
+### Review checklist


not going to re-comment on checklist items, but stuff here would be changed based on prior comments.

scottaohara · 2026-04-29T14:55:59Z

+### Pitfalls
+
+- Opening a dialog without moving focus — keyboard users are stranded on the trigger.
+- Not restoring focus on close — keyboard users lose their place.


Suggested change

- Not restoring focus on close — keyboard users lose their place.

- Not restoring focus on close — keyboard users may lose their place, screen reader users will commonly be returned to the top of the web page.

scottaohara · 2026-04-29T14:57:22Z

+
+### Pitfalls
+
+- Opening a dialog without moving focus — keyboard users are stranded on the trigger.


Suggested change

- Opening a dialog without moving focus — keyboard users are stranded on the trigger.

- Opening a dialog without moving focus — keyboard users are stranded on the trigger. Screen reader users will receive no confirmation of action for the changed state of the page or dialog.

scottaohara · 2026-04-29T14:59:07Z

+
+- Opening a dialog without moving focus — keyboard users are stranded on the trigger.
+- Not restoring focus on close — keyboard users lose their place.
+- Forgetting `inert` on background content in the ARIA fallback — Tab escapes the dialog.


Suggested change

- Forgetting `inert` on background content in the ARIA fallback — Tab escapes the dialog.

- Forgetting `inert` on background content in the ARIA fallback — Keyboard tabbing can reach focusable elements outside of the modal dialog.

scottaohara · 2026-04-29T15:00:20Z

+- Opening a dialog without moving focus — keyboard users are stranded on the trigger.
+- Not restoring focus on close — keyboard users lose their place.
+- Forgetting `inert` on background content in the ARIA fallback — Tab escapes the dialog.
+- Using `role="dialog"` without `aria-modal="true"` and then trapping focus anyway — mismatched semantics.


mmm.... this is maybe going a bit far. non-modal dialogs can also trap keyboard focus and expect someone to navigate around using ctrl + f6. i'd take this out.

mfairchild365 · 2026-04-29T21:38:16Z

Closing this in favor of #19

@scottaohara I've address all of your feedback in that PR too.

mfairchild365 and others added 15 commits April 20, 2026 14:57

Merge remote-tracking branch 'origin/main' into skills-v2

db1ca3e

Merge remote-tracking branch 'origin/main' into skills-v2

ea1462d

fix: visual label computation missing space between words

bc8940c

fix(shopping-home-page): drop focusability requirement for skip link …

0f15452

…target

fix: mark shopping-home-page single-footer assertion N/A when no foot…

6b5dbcf

…er content

fix(modal-dialog): accept role="alertdialog" as an open dialog

640390d

feat(instructions): expand accessible-basic into structured WCAG 2.2 …

8e3a2e0

…AA guide

chore(sandbox): pin playwright install, add healthcheck, silence comp…

73ff6f3

…ose warnings

feat(skills): add multi-turn skills benchmarking with sandboxed agent…

527ca7f

… mount

feat: split a11y-wizard skill into generate + review turns

af13c9d

fix: tighten agent tool timeout and cap agent parallelism at 4

020fd6d

chore: annotate report pass-rate columns with automated-check caveat

756de74

mfairchild365 requested a review from Copilot April 23, 2026 17:41

Copilot started reviewing on behalf of mfairchild365 April 23, 2026 17:42 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Michael Fairchild (MS Sec A11y) added 5 commits April 23, 2026 14:31

chore(docs): fix typos in a11y-wizard skill (astrisk->asterisk, relva…

f9f46b4

…nt->relevant)

mfairchild365 requested review from aarongustafson, scottaohara and slauriat April 23, 2026 19:48

mfairchild365 assigned aarongustafson, scottaohara and slauriat Apr 23, 2026

feat: add specs-documentation reference to a11y-wizard skill

d34bad4

slauriat approved these changes Apr 28, 2026

View reviewed changes

scottaohara reviewed Apr 29, 2026

View reviewed changes

mfairchild365 closed this Apr 29, 2026


		### General defaults

		- Trigger: `<button type="button" aria-expanded="false" aria-controls="panelId">`.

	- Not restoring focus on close — keyboard users lose their place.
	- Not restoring focus on close — keyboard users may lose their place, screen reader users will commonly be returned to the top of the web page.


		### Pitfalls

		- Opening a dialog without moving focus — keyboard users are stranded on the trigger.

	- Forgetting `inert` on background content in the ARIA fallback — Tab escapes the dialog.
	- Forgetting `inert` on background content in the ARIA fallback — Keyboard tabbing can reach focusable elements outside of the modal dialog.

Uh oh!

Conversation

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

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mfairchild365 commented Apr 23, 2026 •

edited

Loading

mfairchild365 commented Apr 29, 2026 •

edited

Loading