ReviewOS

also looking at this

stacks/lastsoulultra

LSU's own coaching surface, and what auditing the site turned up

#12
Merged glennmichael123 wants to merge claude/last-soul-ultra-web-6e0718 into main
86 files +3943 -1190

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

package.jsonmodified+2-1
Changes to package.json
@@ -72,7 +72,8 @@
7272 "deps:lockfile:check": "bun .github/scripts/check-lockfile-version.ts",
7373 "bud": "./buddy",
7474 "stx": "./buddy",
75 "build:assets": "bun scripts/build-site-assets.ts"
75 "build:assets": "bun scripts/build-site-assets.ts",
76 "audit:html": "bun scripts/audit-html.ts"
7677 },
7778 "dependencies": {
7879 "@stacksjs/components": "0.2.687",
scripts/audit-html.tsadded+128-0
Changes to scripts/audit-html.ts
@@ -0,0 +1,128 @@
1/* eslint-disable no-console */
2/**
3 * Audit the built site for the mistakes a template cannot catch itself.
4 *
5 * A page renders perfectly well with a link to a page that was renamed, two
6 * elements sharing an id, an h3 under an h1, or an image whose alt was never
7 * written. Nothing errors, nothing fails to build, and nobody notices until
8 * somebody using a screen reader or a keyboard does.
9 *
10 * bun run build && bun scripts/audit-html.ts
11 *
12 * Reads dist/, so it checks what is actually deployed rather than what the
13 * source says should be. Exits non-zero on any finding.
14 */
15
16import { readdirSync } from 'node:fs'
17import { join, relative } from 'node:path'
18import process from 'node:process'
19import server from '../config/server'
20
21const ROOT = 'dist'
22
23interface Finding {
24 page: string
25 rule: string
26 detail: string
27}
28
29function pages(dir = ROOT): string[] {
30 return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
31 const path = join(dir, entry.name)
32 if (entry.isDirectory()) return entry.name.startsWith('_') ? [] : pages(path)
33 return entry.name.endsWith('.html') ? [path] : []
34 })
35}
36
37/** The URL a built file is served at. */
38function urlOf(file: string): string {
39 const path = `/${relative(ROOT, file).replace(/\.html$/, '')}`
40 return path === '/index' ? '/' : path
41}
42
43/** Strip comments, script and style so their contents are not read as markup. */
44function visible(html: string): string {
45 return html
46 .replace(/<!--[\s\S]*?-->/g, '')
47 .replace(/<script\b[^>]*>[\s\S]*?<\/script>/gi, '')
48 .replace(/<style\b[^>]*>[\s\S]*?<\/style>/gi, '')
49}
50
51function attr(tag: string, name: string): string | null {
52 const match = new RegExp(`\\s${name}\\s*=\\s*"([^"]*)"`, 'i').exec(tag)
53 ?? new RegExp(`\\s${name}\\s*=\\s*'([^']*)'`, 'i').exec(tag)
54 if (match) return match[1]!
55 return new RegExp(`\\s${name}(?=[\\s/>])`, 'i').test(tag) ? '' : null
56}
57
58const built = new Set(pages().map(urlOf))
59const redirected = new Set(Object.keys(server.redirects ?? {}))
60const findings: Finding[] = []
61
62for (const file of pages()) {
63 const page = urlOf(file)
64 const raw = await Bun.file(file).text()
65 const body = visible(raw)
66
67 // 1. Internal links point at something this build serves.
68 for (const [, href] of raw.matchAll(/<a\b[^>]*\shref="(\/[^"#?]*)[^"]*"/gi)) {
69 const target = href!.length > 1 ? href!.replace(/\/$/, '') : href!
70 if (built.has(target) || redirected.has(target)) continue
71 // Assets are files on disk rather than pages.
72 if (/\.[a-z0-9]{2,5}$/i.test(target) || target.startsWith('/assets/')) continue
73 findings.push({ page, rule: 'dead-link', detail: target })
74 }
75
76 // 2. Every image says what it shows, or declares itself decorative.
77 for (const [tag] of body.matchAll(/<img\b[^>]*>/gi)) {
78 if (attr(tag, 'alt') === null)
79 findings.push({ page, rule: 'img-no-alt', detail: attr(tag, 'src') ?? tag.slice(0, 60) })
80 }
81
82 // 3. One id, one element.
83 const ids = [...body.matchAll(/\sid="([^"]+)"/g)].map(match => match[1]!)
84 for (const id of new Set(ids)) {
85 if (ids.filter(value => value === id).length > 1)
86 findings.push({ page, rule: 'duplicate-id', detail: id })
87 }
88
89 // 4. One h1, and no level skipped on the way down.
90 const levels = [...body.matchAll(/<h([1-6])\b/gi)].map(match => Number(match[1]))
91 const h1s = levels.filter(level => level === 1).length
92 if (h1s !== 1)
93 findings.push({ page, rule: 'h1-count', detail: String(h1s) })
94 for (let index = 1; index < levels.length; index++) {
95 if (levels[index]! - levels[index - 1]! > 1)
96 findings.push({ page, rule: 'heading-skip', detail: `h${levels[index - 1]} to h${levels[index]}` })
97 }
98
99 // 5. A link a screen reader can announce.
100 for (const [match, tag, inner] of body.matchAll(/(<a\b[^>]*>)([\s\S]*?)<\/a>/gi)) {
101 void match
102 const text = inner!.replace(/<[^>]+>/g, '').replace(/&[a-z]+;/gi, ' ').trim()
103 if (text) continue
104 if (attr(tag!, 'aria-label') || attr(tag!, 'title')) continue
105 if (/<img\b[^>]*\salt="[^"]+"/i.test(inner!)) continue
106 if (attr(tag!, 'aria-hidden') === 'true') continue
107 findings.push({ page, rule: 'link-no-name', detail: attr(tag!, 'href') ?? '' })
108 }
109
110 // 6. A document that states its language.
111 if (!/<html\b[^>]*\slang="[^"]+"/i.test(raw))
112 findings.push({ page, rule: 'no-lang', detail: '' })
113}
114
115const byRule = new Map<string, Finding[]>()
116for (const finding of findings) {
117 if (!byRule.has(finding.rule)) byRule.set(finding.rule, [])
118 byRule.get(finding.rule)!.push(finding)
119}
120
121console.log(`audited ${built.size} pages`)
122for (const [rule, rows] of [...byRule].sort((a, b) => b[1].length - a[1].length)) {
123 console.log(`\n${rule} (${rows.length})`)
124 for (const row of rows.slice(0, 12)) console.log(` ${row.page} ${row.detail}`)
125 if (rows.length > 12) console.log(` ... ${rows.length - 12} more`)
126}
127if (findings.length === 0) console.log('\nno findings')
128process.exit(findings.length === 0 ? 0 : 1)