feat: say what a run did not measure - #19
Merged
Merged
Conversation
A crawl that fetched twelve pages of a two-hundred-page site produced a report byte-identical to a complete one. The crawler had already counted the failures, the truncation and how the pages were found; every one of those numbers went to stderr and was then dropped on the floor. That is the same false assurance this codebase refuses everywhere else. `passes` is kept apart from `inapplicable` so an empty page cannot look like a compliant one, and a rule the engine is blind to is reported as unevaluated rather than as a pass. A clean verdict over a fraction of a site is the same mistake, and it was the one still being made — to the reader least able to catch it, since whoever opens the report is usually not whoever watched it run. So the facts reach the report now: a `completeness` object in JSON, a section in the HTML report naming each page it could not reach and why, lines in the console summary, and `run.properties` in SARIF. Pages that were never fetched are counted apart from pages that were fetched and then failed to audit, because they are different problems with different fixes and summing them would name neither. The HTML report grows a fourth verdict. A clean result over part of a site no longer gets the plain pass banner: it says that no violations were found and that the whole site was not measured, because the banner is the part that gets quoted. Exit codes are deliberately unchanged. An incomplete run is not a violation, and failing builds on it would break every pipeline already running this tool. Also: one unreadable file no longer rejects the whole collection. A single bad permission bit reported as a crash rather than as the one page nobody could look at. The failure is only swallowed where a caller has undertaken to report it, so nothing goes missing silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht
The registry recognised fourteen builders; the route mapper knew five. Everyone else — Remix, Gatsby, Docusaurus, VitePress, Starlight, Hugo — got told their finding was in `leistungen.html`, which this module's own doc comment calls accurate and unhelpful: nobody wrote that file. Two changes made room for them. Conventions each carry their own transform now, because the routers agree on almost nothing: Remix delimits segments with dots and marks a directory's own page `_index`, Hugo names a section index `_index.md`, SvelteKit puts the route in the directory and the kind in the filename. One function branching on the framework would have become a chain of special cases nobody could read. And the convention is chosen by what the project is rather than by which directory exists. `src/pages` belongs to Next.js, Astro and Gatsby alike, so probing in declaration order labelled every one of them `next-pages`: the mapping was right and the name beside it was wrong, which is the kind of detail a reader notices and stops trusting the rest of the report over. Where the registry recognises nothing, the directories are probed as before — most builds are not framework projects, and a map is better than no map. Angular, Eleventy and Jekyll stay unmapped on purpose, and the docs now say why rather than leaving it looking like an oversight. Angular keeps its routes in a TypeScript object, and reading it means parsing or running project code — which `outputFromConfig` already refuses to do. Eleventy and Jekyll let a page set its own URL in front matter, so their file layout is not the route: mapping the default convention alone would be right until somebody used the feature, and silently wrong after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht
WordPress, TYPO3, Craft, Laravel, Symfony, Rails and Django render every page per request and write no browsable HTML anywhere. Pointed at one, the tool reported that ./dist held no HTML files — describing a directory that was never going to hold any, and offering to fix a path that was not wrong. They are identified by a file rather than a dependency. A package.json in one of these belongs to a theme's asset build and says nothing about the CMS around it, which is also why they are ordered ahead of Vite: a WordPress theme bundled with Vite is a WordPress site, and auditing the folder Vite filled would audit its stylesheets. Detected, and then left alone. `eaa-kit audit` with no arguments will run a static builder's own build and even start its preview server; for these it does neither. Starting one means spawning a stateful, usually container-backed stack that may touch a database, which is a great deal more than an accessibility audit was asked to do and not something to do to somebody's machine uninvited — and `npm run build` here would produce stylesheets and no pages. The guard runs before the package.json check, because most Django and Rails projects have no package.json and the ordinary path gives up there. The registry invariant moves with it: an entry used to need an output directory, and now needs somewhere to audit — an output directory, or the ability to serve and a command that says how. An entry with neither can be detected and never acted on, which is how the advice would come to name a directory that cannot exist. Also adds --sitemap. A CMS rarely puts its page list at /sitemap.xml: WordPress with Yoast serves /sitemap_index.xml, TYPO3 and Craft put it behind a route of their own, and link following alone then finds only what the navigation happens to link to — on a site with a thousand articles, a menu. A named sitemap replaces the default rather than supplementing it, so a wrong path is a visible mistake rather than a silent fall back to a crawl that quietly covers less. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht
Neither report answers the question a review actually asks. Running the auditor on a branch prints everything wrong with the site; almost all of it was already there, and the two or three findings somebody introduced are somewhere in the middle of it. `eaa-kit diff before.json after.json` sorts every violating element into new, fixed, unchanged — and not compared. That fourth bucket is the reason to trust the other three. A violation missing from the second run has two possible explanations: somebody fixed it, or nothing looked at that page. They are not interchangeable, and reporting the second as fixed would turn a crawl that stopped early into a changelog of work nobody did — the same false assurance the completeness block was just added to prevent. So a violation is called fixed only where the later run actually reached a verdict on its page, and the rest are listed apart with their pages named. A page that errored counts as unmeasured too: it was reached and then not audited, so it testifies to nothing. Identity comes from elementFingerprint, the function the baseline and SARIF already share, so all three agree on what one defect is. Markup that changed reads as a different violation rather than the same one, which is the conservative direction: it avoids calling something fixed because its surroundings moved. --fail-on judges new violations only. Failing on what was already there would be an audit with extra steps, and would go red on every branch of a site carrying any debt. This is not a baseline and does not replace one. A baseline is a file somebody commits and maintains, answering what we have agreed to live with. A diff is stateless, needs no decision from anybody, and answers what this change did. Every node in the JSON report now carries its fingerprint, so a consumer comparing two reports need not reimplement the hash and risk disagreeing with the two tools that already depend on it agreeing. schema.ts gains boolean(), which its own note invites: it does what these schemas need and no more. Also fixes the two stale pins the roadmap turned up: the Action example in the docs named a release three versions old, and the runnable workflow told readers to use a moving @v0 tag that the same docs page says does not and will not exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht
The tool refuses to divide passes by failures, and it is right to: only `passes` is evidence, `inapplicable` is not, and the two summed would make an empty page look compliant. But refusing that number left nothing in its place, so a reader got a page of honest disclaimers and no measurement at all, and "automated testing finds a minority of barriers" stayed a sentence in the footer rather than a figure. There is an honest denominator and it is the standard, not the DOM. WCAG 2.2 has 55 success criteria at Levels A and AA. axe-core has rules touching 23 of them. Every run says so in a line, --coverage lists all 55 and what the run reached on each, and the HTML report always carries the table. Four outcomes, and every criterion lands in exactly one — the same discipline shapeResults enforces for rules, for the same reason. `nothing-to-check` is kept apart from `evaluated` because a rule that ran and matched nothing has not shown the criterion is met: a page with no images proves nothing about image alternatives, which is precisely why `inapplicable` is already kept out of `passes`. They are never summed or divided. `no-automated-rule` is the majority of WCAG, so a percentage would present a limit of automated testing as though it were a measurement of somebody's site — a nicer-looking number than the truth and a worse one. Tests assert the partition holds, that the majority stays unautomatable, and that no output contains a percent sign. The engine gap falls out of the same table. MANUAL_CHECKS already recorded, per rule, whether a real browser resolves it; nothing had ever read that field. The report now says how many more criteria --browser would answer, which replaces general advice with a count. ENGINE_BLIND_RULES and blindRulesInScope move to result.ts, beside the BlindRule type they instantiate. They describe what an engine can see rather than the jsdom library, and coverage.ts is reachable from the CLI's startup path — reading them where they were would have pulled 630 ms of jsdom into `eaa-kit --help`. Both are still exported from the runner, so nothing that imported them had to change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht
axe-core says "images must have alternative text" and links to a page about the rule. Neither is the fix. The first tells somebody who already knows nothing they did not know, and the gap between a report and a corrected line of code is where an accessibility tool either earns its place in a build or becomes something people mute. Each finding now carries who the barrier stops, what to change, and the corrected form of the markup that actually failed — the user's own element, because a textbook snippet is a second thing to translate before anybody can use it. Where the fix genuinely differs by framework it is given in that idiom: where `lang` lives in a Next.js, Nuxt, Astro, SvelteKit or Remix project is a different question from which attribute is missing. Where it does not differ, which is most rules, the same advice is given whatever built the site — a missing alt is a missing alt in all fourteen builders the registry knows, and emitting fourteen near-identical snippets would be churn pretending to be coverage. An overlay states only what changes and inherits the rest, so a framework-specific fix cannot quietly drop the corrected snippet and the sentence about who is shut out along with it. Deterministic and offline: no model, no API key, no network. This sits beside a document with legal weight, and a plausible fix that is wrong is a worse failure here than no fix at all — somebody would paste it, the report would go green, and the barrier would still be there. A fixed table is reviewed once and then trusted; a generated one has to be reviewed every time, by the person least equipped to. componentFor now returns a location rather than a path. It already had to find the literal in the source to know which file contained it, so recording where it found it costs one more scan of one string — and `src/components/Header.astro:12` opens an editor where `src/components/Header.astro` starts a search. This is also what the PR annotations deferred to 0.5.0 will need. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht
Four more ways to run the audit as part of a build. Each is a thin adapter over the decision function the Astro integration and the Vite plugin already share, and each describes its host structurally, so none of them adds a dependency: installing eaa-kit in a project with no webpack still typechecks. Two of them earn their place by knowing something the Vite plugin cannot. The Nuxt module audits in `close`, not when Vite finishes. Nuxt builds on Vite, so `eaa-kit/vite` already runs there — and it runs too early: Nitro prerenders after Vite is done, so the hook fires before the pages the audit exists to read have been written. It also refuses to pass a `nuxt build`, which produces a server and no browsable HTML: auditing nothing and reporting success is the worst outcome available here, so a run that prerendered no pages says so instead. The webpack plugin skips watch rebuilds — auditing a whole site on every keystroke would make a dev server unusable, and a failing audit there cannot stop anything being shipped — and skips a build that already failed, because it has no output worth judging and reporting its missing pages as accessibility findings would send somebody after defects nothing ever measured. Eleventy and Docusaurus are the straightforward two. Both hand over the directory they wrote, which matters for Eleventy in particular: its output directory is configurable and the audit would otherwise be guessing at `_site`. There is deliberately no Next.js plugin, and the docs say why rather than leaving it looking like an oversight. Next has no hook that fires after `output: 'export'` writes `out/` — webpack's compiler hooks all run before it — so a `withEaaKit()` wrapper would audit stale output or none at all, and a plugin that silently audits the previous build is worse than having none. The four copies of the stderr logger these needed became one in integration/run.ts, beside the interface it satisfies. The packaged harness now imports every subpath export from a real install rather than checking the file exists. A file that ships and does not load fails inside somebody's build rather than here, which is how the browser-mode bugs of 0.2.x reached users in the first place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
They failed on windows-latest. The guard asked whether the tests were running as root, and root is only one of the two environments where a file chmodded to 000 stays readable — Windows is the other, because its chmod toggles a read-only flag that does not stop a read. Worse, the guard could not even see it: `process.getuid` does not exist on Windows, so `process.getuid?.() === 0` was `undefined === 0`, the cases ran, and they asserted against a file the process opened without complaint. Naming environments was the mistake, so the precondition is measured instead: write a file, deny it, try to read it. That is exactly the thing the cases need to be true, it covers root and Windows without mentioning either, and it stays right on a platform nobody thought about. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht
It did neither. The line chained `&&` onto an audit of tests/fixtures/impacts, which exits 1 by design because those fixtures carry serious violations — so the chain stopped there and `diff` was never invoked. The trailing `; test $? -le 1` then turned that into a pass. The check reported green on every platform while exercising nothing, which is worse than not having it. Windows only made it visible: `test` is not a command under cmd.exe, and `/tmp` is not a path there either. Both reports now come from `--include about/**`, the clean subset the rest of this script already uses, so each audit exits 0 and the chain reaches the end. Comparing them exercises the whole path — read two reports, compare, serialise — and exits 0 because nothing changed between them. What diff decides is asserted in its own suite; what this has to prove is that the command runs at all in a built package. The scratch reports go to .eaa-kit/, which is already gitignored and is the tool's own namespace, rather than to a hardcoded /tmp. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht
WCAG 2.1.1 Keyboard is the only criterion whose rules are mixed: scrollable-region-focusable needs computed overflow, while frame-focusable-content and server-side-image-map are DOM-determinable. Requiring every rule to be blind before saying "not evaluated" meant a page with no frames and no image maps fell through to "rules ran and found nothing on this site to check" — while the same report's rule listing said, four sections earlier, that scrollable-region-focusable had reached no verdict on all five pages. One document contradicting itself about a Level A criterion, and in the direction that flatters the tool: the browserless engine's cost was reported as one criterion lower than it is. On the fixture site `--browser would answer` was 4 and is 5. The rule is now what it should always have been. `nothing-to-check` is reserved for a criterion where every rule ran and matched nothing; anything the engine could not see puts it in `not-evaluated`, where the rule listing already had it. A test asserts the two views cannot drift apart again: every criterion carrying a rule reported as an engine limitation must read as unevaluated in the coverage table too. Also adds the drift test the third copy of the report schema version never had. json.ts writes it, the statement reader asserts it matches, and diff.ts added a third that nothing checked — so a bump would have left `diff` quietly refusing every report the tool writes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht
It took the output directory from `nuxt.options.nitro.output.publicDir`. Nuxt never populates that — Nitro resolves the output onto its own instance — so the module would have refused every build it was wired into, `nuxt generate` included, with "there was nothing to audit". Its unit test agreed with the bug. A hand-written stand-in confirms whatever its author assumed, and I had assumed wrong in both places at once, so the two matched and the suite went green over a module that could never work. It is now driven by a real `nuxt generate` and a real `nuxt build`, the way the Astro integration already was, and nuxt is a devDependency for it. The directory comes from `nitro:init`, where Nitro actually puts it, and a server build is told apart from a static one by `nitro.options.static` rather than by guesswork. The timing claim the module exists for survived the check: at `build:done` the public directory does not exist at all, and by `close` it holds every prerendered page. Auditing in `close` is right, and the Vite plugin genuinely would have run too early. Two things the fixture taught, which are worth knowing: Nuxt emits neither a lang attribute nor a title by default, and both are serious violations on every page it renders — the "clean" fixture needed both before these cases stopped passing for the wrong reason. Also in this commit: - 0.4.0, and the changelog dated. - The Action gains a `sitemap` input. CMS support is a headline of this release and the Action is how the audience it is aimed at would run it; without this the flag was unreachable from CI. - The README says what the release does. Coverage, remediation and four of the six integrations were invisible in it, and the line listing where it runs was stale. - A duplicate "Not Next.js" section removed from the integration docs. One was already there, with better reasoning than mine: Next 16 defaults to Turbopack, which never calls the webpack function a plugin would have to hook. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht
After the Nuxt module turned out to read a field that does not exist, leaving the others on trust was not defensible. Eleventy is cheap to install and builds in under a second, so there was no reason to. Its reading of the hook was right: `eleventy.after` does hand over the resolved output directory, and one of the cases points the project at a directory that is not `_site` to prove the plugin uses what it is given rather than a guess that happens to match the default. The build did surface something else. `eleventy.after` fires on every rebuild under --watch and --serve, so a dev server would have run a full site audit between keystrokes. The webpack plugin already refused that and Eleventy did not, for no reason other than that I wrote the guard in one file and not the other. It now audits one-shot builds only, and skips the output modes that write nothing to disk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht
The generated eleventy.config.mjs did `import eaaKit from "D:\a\..."`. Eleventy loads that config with import(), and a bare Windows absolute path is not a valid ESM specifier — Node rejects it as an unsupported 'd:' scheme, so the config failed to load and every case in the file went red on windows-latest. The Astro fixture has resolved this the right way since it was written, with pathToFileURL, and says why in a comment two lines long. The Nuxt fixture is unaffected: Nuxt resolves module paths itself rather than importing them as specifiers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht
…once `nuxt generate` failed in CI with "the module dist/nuxt/index.js could not be loaded". The module was there when the fixture was written and gone by the time Nuxt resolved it. Three suites drive a real build of somebody else's tool — Astro, Eleventy, Nuxt — and each has to load this package's built entry point, because that is what a consumer loads. CI runs `pnpm test` before `pnpm smoke`, so dist/ does not exist yet and something has to make it. Each of them made it, with the same beforeAll copied between them. That was fine while Astro was the only one. With three, vitest runs the files in parallel workers, all three find dist/ missing at the same moment, and all three spawn tsdown — which is configured `clean: true` and so deletes the directory the other two are reading. It never failed on this machine because dist/ was already built here, so the `access` check short-circuited and nothing ever entered the race. That is also why removing dist/ locally does not reproduce it reliably: the window is real but narrow, and a shared CI runner is slower and busier than a laptop. The mechanism is not in doubt — the CI log shows the file resolving and then not existing — so this is fixed by removing the hazard rather than by trying to lose the race again. The build moves to a vitest globalSetup, which runs once in the main process before any worker starts. Three copies of the beforeAll go away with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A crawl that fetched twelve pages of a two-hundred-page site produced a
report byte-identical to a complete one. The crawler had already counted
the failures, the truncation and how the pages were found; every one of
those numbers went to stderr and was then dropped on the floor.
That is the same false assurance this codebase refuses everywhere else.
passesis kept apart frominapplicableso an empty page cannot looklike a compliant one, and a rule the engine is blind to is reported as
unevaluated rather than as a pass. A clean verdict over a fraction of a
site is the same mistake, and it was the one still being made — to the
reader least able to catch it, since whoever opens the report is usually
not whoever watched it run.
So the facts reach the report now: a
completenessobject in JSON, asection in the HTML report naming each page it could not reach and why,
lines in the console summary, and
run.propertiesin SARIF. Pages thatwere never fetched are counted apart from pages that were fetched and
then failed to audit, because they are different problems with different
fixes and summing them would name neither.
The HTML report grows a fourth verdict. A clean result over part of a
site no longer gets the plain pass banner: it says that no violations
were found and that the whole site was not measured, because the banner
is the part that gets quoted.
Exit codes are deliberately unchanged. An incomplete run is not a
violation, and failing builds on it would break every pipeline already
running this tool.
Also: one unreadable file no longer rejects the whole collection. A
single bad permission bit reported as a crash rather than as the one
page nobody could look at. The failure is only swallowed where a caller
has undertaken to report it, so nothing goes missing silently.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01HKU933TcRaKinharQxrNht