Public: anyone can read this; the crew can change it.
The design check and docs/DESIGN.md stop parking pulls: the red says exactly what to change, and DESIGN.md is no longer one file every pull edits #4064
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/4063-the-design-check-and-docs-design-md-stop → main 2 commits fork 2 threads · 2 unresolved
4 files
changed +136
−15
docs/design/areas/README.md added +7
| @@ -0,0 +1,7 @@ | ||
| 1 | # Component areas | |
| 2 | ||
| 3 | A new component class is named here, in a file for its area (`docs/design/areas/<area>.md`), in a code span | |
| 4 | (`` `.my-class` ``) with a line on what it is for. The design check reads every file in this directory as §5 vocabulary, | |
| 5 | so two pulls documenting different components touch different files and never conflict on `docs/DESIGN.md`. | |
| 6 | ||
| 7 | Do not edit `docs/DESIGN.md` §5d: it only shrinks. Prefer a component §5 already has. |
scripts/design-lint.ts +83 −12
| @@ -30,6 +30,11 @@ | ||
| 30 | 30 | * target a button, link, input or select under 24px wide or high, WCAG 2.5.8 |
| 31 | 31 | * (specification.website, target size) |
| 32 | 32 | * |
| 33 | * A red names the file:line, the literal and what to use instead, so one pass | |
| 34 | * fixes it; run `bun scripts/design-lint.ts` before pushing. A new class is named | |
| 35 | * in its own `docs/design/areas/<area>.md` rather than in DESIGN.md, which | |
| 36 | * every pull would otherwise edit (#4063). | |
| 37 | * | |
| 33 | 38 | * Known gaps, so nobody mistakes the gate for more than it is: colours built |
| 34 | 39 | * in JS strings other than the forms above, `oklch()`, |
| 35 | 40 | * `lab()` and system colours are not seen; sizes in `rem`/`em` are not |
| @@ -67,7 +72,9 @@ const WEB_MESSAGE: Record<(typeof WEB_KEYS)[number], string> = { | ||
| 67 | 72 | target: "interactive target under 24x24 CSS px, WCAG 2.5.8 (specification.website, target size)", |
| 68 | 73 | }; |
| 69 | 74 | export type Baseline = { files: Record<string, Counts> }; |
| 70 | export type Scan = Baseline & { classes: string[] }; | |
| 75 | /** One offence, where it is and what the system offers instead. */ | |
| 76 | export type Finding = { line: number; kind: "colour" | "size"; text: string; use: string }; | |
| 77 | export type Scan = Baseline & { classes: string[]; found?: Record<string, Finding[]> }; | |
| 71 | 78 | |
| 72 | 79 | const SKIP = /(^|\/)(og\/vendor)\//; |
| 73 | 80 | |
| @@ -86,9 +93,14 @@ export function sourceFiles(root: string): string[] { | ||
| 86 | 93 | |
| 87 | 94 | /** The CSS inside `/* css *\/ \`…\`` templates — the only place a class is defined. */ |
| 88 | 95 | function cssBlocks(text: string): string[] { |
| 89 | const out: string[] = []; | |
| 96 | return cssBlocksAt(text).map((b) => b.css); | |
| 97 | } | |
| 98 | ||
| 99 | /** The same blocks with the offset in `text` where each one's css starts. */ | |
| 100 | function cssBlocksAt(text: string): { css: string; at: number }[] { | |
| 101 | const out: { css: string; at: number }[] = []; | |
| 90 | 102 | const re = /\/\* css \*\/\s*`((?:[^`\\]|\\.)*)`/g; |
| 91 | for (let m = re.exec(text); m; m = re.exec(text)) out.push(m[1]!); | |
| 103 | for (let m = re.exec(text); m; m = re.exec(text)) out.push({ css: m[1]!, at: m.index + m[0].length - 1 - m[1]!.length }); | |
| 92 | 104 | return out; |
| 93 | 105 | } |
| 94 | 106 | |
| @@ -116,6 +128,43 @@ export function countFile(text: string): Counts { | ||
| 116 | 128 | return { colour, size }; |
| 117 | 129 | } |
| 118 | 130 | |
| 131 | /** Comments blanked but newlines kept, so an offset still maps to its line. */ | |
| 132 | const blanked = (text: string): string => | |
| 133 | text.replace(/\/\*[\s\S]*?\*\//g, (m) => m.replace(/[^\n]/g, " ")).replace(/(^|\s)\/\/.*$/gm, (_m, a) => a); | |
| 134 | ||
| 135 | /** The token a colour literal on this line should be, from the property it sits in. */ | |
| 136 | function tokenFor(line: string): string { | |
| 137 | if (/\b(?:border|outline)[-\w]*\s*:/.test(line)) return "var(--border) (a hairline) or var(--border-strong)"; | |
| 138 | if (/\bbackground[-\w]*\s*:/.test(line)) return "var(--card) (raised), var(--muted) (recessed) or var(--bg)"; | |
| 139 | if (/\b(?:fill|stroke)\s*:/.test(line)) return "var(--fg), var(--muted-fg) or a status token"; | |
| 140 | if (/\bcolor\s*:/.test(line)) return "var(--fg), var(--muted-fg) or a status token (--success, --danger, --attn)"; | |
| 141 | return "a token from docs/DESIGN.md §2 (var(--…))"; | |
| 142 | } | |
| 143 | ||
| 144 | /** Where `countFile` counts colours and off-scale sizes: file line, the literal, the system's answer. */ | |
| 145 | export function locate(text: string): Finding[] { | |
| 146 | const code = blanked(text); | |
| 147 | const lines = code.split("\n"); | |
| 148 | const out: Finding[] = []; | |
| 149 | const colour = /#[0-9a-fA-F]{6}\b|#[0-9a-fA-F]{3}\b(?![0-9a-zA-Z_-])|\b(?:rgba?|hsla?)\([^)]*\)/g; | |
| 150 | lines.forEach((l, i) => { | |
| 151 | for (const m of l.matchAll(colour)) out.push({ line: i + 1, kind: "colour", text: m[0], use: tokenFor(l) }); | |
| 152 | for (const m of l.matchAll(/font-size:\s*([0-9.]+)px/g)) { | |
| 153 | const px = Number(m[1]); | |
| 154 | if (TYPE_SCALE.includes(px)) continue; | |
| 155 | const near = TYPE_SCALE.reduce((a, b) => (Math.abs(b - px) < Math.abs(a - px) ? b : a)); | |
| 156 | out.push({ line: i + 1, kind: "size", text: `font-size: ${m[1]}px`, use: `${near}px (the scale is ${TYPE_SCALE.join(", ")}px, §3)` }); | |
| 157 | } | |
outdated
Reads README.md too, so its |
||
| 158 | }); | |
| 159 | for (const { css, at: start } of cssBlocksAt(text)) { | |
| 160 | for (const m of css.matchAll(NAMED_RE)) { | |
| 161 | const line = text.slice(0, start + m.index!).split("\n").length; | |
| 162 | out.push({ line, kind: "colour", text: m[0].trim(), use: tokenFor(m[0]) }); | |
| 163 | } | |
| 164 | } | |
| 165 | return out.sort((a, b) => a.line - b.line); | |
| 166 | } | |
| 167 | ||
| 119 | 168 | const INTERACTIVE = /(^|[\s,>+~])(a|button|input|select|summary)\b|\.btn\b|\[role=["']?button/i; |
| 120 | 169 | const FIELD = /(^|[\s,>+~])(input|select|textarea)\b/i; |
| 121 | 170 | |
| @@ -177,8 +226,24 @@ function namedIn(text: string): string[] { | ||
| 177 | 226 | } |
| 178 | 227 | |
| 179 | 228 | /** Every class DESIGN.md §5 names, its subsections and the §5d register included. */ |
| 180 | export function documentedClasses(design: string): string[] { | |
| 181 | return namedIn(section(design, "5.")); | |
| 229 | export function documentedClasses(design: string, areas: readonly string[] = []): string[] { | |
| 230 | return [...new Set([...namedIn(section(design, "5.")), ...areas.flatMap(namedIn)])].sort(); | |
| 231 | } | |
| 232 | ||
| 233 | /** The area files under docs/design/areas/: each is wholly vocabulary, one file per component area (#4063). */ | |
| 234 | export function areaFiles(root: string): string[] { | |
| 235 | try { | |
| 236 | return readdirSync(join(root, AREAS)).filter((n) => n.endsWith(".md") && n !== "README.md").sort().map((n) => readFileSync(join(root, AREAS, n), "utf8")); | |
| 237 | } catch { | |
| 238 | return []; | |
| 239 | } | |
| 240 | } | |
| 241 | ||
| 242 | /** A documented class that shares a word with the new one, or none: the nearest thing to reuse. */ | |
| 243 | function similar(k: string, named: readonly string[]): string { | |
| 244 | const words = k.split("-").filter((w) => w.length > 2); | |
| 245 | const hit = named.filter((n) => n !== k && n.split("-").some((w) => words.includes(w))).slice(0, 3); | |
| 246 | return hit.length ? ` (already named and alike: ${hit.map((h) => "." + h).join(" ")})` : ""; | |
| 182 | 247 | } |
| 183 | 248 | |
| 184 | 249 | /** The §5d register alone: the page-local classes that predate the rule. */ |
| @@ -192,30 +257,35 @@ export function registeredClasses(design: string): string[] { | ||
| 192 | 257 | |
| 193 | 258 | export function scan(root: string): Scan { |
| 194 | 259 | const files: Record<string, Counts> = {}; |
| 260 | const found: Record<string, Finding[]> = {}; | |
| 195 | 261 | const classes = new Set<string>(); |
| 196 | 262 | for (const f of sourceFiles(root)) { |
| 197 | 263 | const text = readFileSync(f, "utf8"); |
| 198 | 264 | const c: Counts = { ...countFile(text), ...webCss(text) }; |
| 199 | 265 | if (c.colour || c.size || WEB_KEYS.some((k) => c[k])) files[relative(root, f)] = c; |
| 266 | if (c.colour || c.size) found[relative(root, f)] = locate(text); | |
| 200 | 267 | for (const k of classesIn(text)) classes.add(k); |
| 201 | 268 | } |
| 202 | return { files, classes: [...classes].sort() }; | |
| 269 | return { files, classes: [...classes].sort(), found }; | |
| 203 | 270 | } |
| 204 | 271 | |
| 205 | 272 | /** What a pull made worse, one line each. Empty means the check passes. */ |
| 206 | export function compare(base: Baseline, now: Scan, design: string): string[] { | |
| 273 | export function compare(base: Baseline, now: Scan, design: string, areas: readonly string[] = []): string[] { | |
| 207 | 274 | const out: string[] = []; |
| 208 | 275 | for (const [file, c] of Object.entries(now.files)) { |
| 209 | 276 | const b = base.files[file] ?? { colour: 0, size: 0 }; |
| 210 | 277 | for (const k of WEB_KEYS) { |
| 211 | 278 | if ((c[k] ?? 0) > (b[k] ?? 0)) out.push(`${file}: ${(c[k] ?? 0) - (b[k] ?? 0)} new ${WEB_MESSAGE[k]}`); |
| 212 | 279 | } |
| 213 | if (c.colour > b.colour) out.push(`${file}: ${c.colour - b.colour} new colour literal(s) — use a token from themes.ts (var(--…))`); | |
| 214 | if (c.size > b.size) out.push(`${file}: ${c.size - b.size} new off-scale font-size — the scale is ${TYPE_SCALE.join(", ")}px`); | |
| 280 | const where = (kind: Finding["kind"]) => | |
| 281 | (now.found?.[file] ?? []).filter((f) => f.kind === kind).map((f) => `\n ${file}:${f.line} ${f.text} → use ${f.use}`).join(""); | |
| 282 | if (c.colour > b.colour) out.push(`${file}: ${c.colour - b.colour} new colour literal(s) — a colour is a token, not a literal (${b.colour} already there are the baseline; the new one is among these):${where("colour") || " use var(--…) from docs/DESIGN.md §2"}`); | |
| 283 | if (c.size > b.size) out.push(`${file}: ${c.size - b.size} new off-scale font-size — the scale is ${TYPE_SCALE.join(", ")}px:${where("size") || " pick the nearest"}`); | |
| 215 | 284 | } |
| 216 | const named = new Set(documentedClasses(design)); | |
| 285 | const all = documentedClasses(design, areas); | |
| 286 | const named = new Set(all); | |
| 217 | 287 | for (const k of now.classes) { |
| 218 | if (!named.has(k)) out.push(`.${k}: a component class the design system does not have — use one docs/DESIGN.md §5 names, or name it there first`); | |
| 288 | if (!named.has(k)) out.push(`.${k}: a component class the design system does not have${similar(k, all)} — use one docs/DESIGN.md §5 names, or name it in a code span in your own file docs/design/areas/<area>.md (not DESIGN.md, so parallel pulls never conflict)`); | |
| 219 | 289 | } |
| 220 | 290 | const defined = new Set(now.classes); |
| 221 | 291 | for (const k of registeredClasses(design)) { |
| @@ -226,6 +296,7 @@ export function compare(base: Baseline, now: Scan, design: string): string[] { | ||
| 226 | 296 | |
| 227 | 297 | const BASELINE = "docs/design/baseline.json"; |
| 228 | 298 | const DESIGN = "docs/DESIGN.md"; |
| 299 | const AREAS = "docs/design/areas"; | |
| 229 | 300 | |
| 230 | 301 | if (import.meta.main) { |
| 231 | 302 | const root = process.cwd(); |
| @@ -236,7 +307,7 @@ if (import.meta.main) { | ||
| 236 | 307 | console.log(`wrote ${BASELINE}`); |
| 237 | 308 | } else { |
| 238 | 309 | const base: Baseline = JSON.parse(readFileSync(join(root, BASELINE), "utf8")); |
| 239 | const problems = compare(base, now, readFileSync(join(root, DESIGN), "utf8")); | |
| 310 | const problems = compare(base, now, readFileSync(join(root, DESIGN), "utf8"), areaFiles(root)); | |
| 240 | 311 | for (const p of problems) console.error(p); |
| 241 | 312 | if (problems.length) process.exit(1); |
| 242 | 313 | console.log("design: no colour, size or component added outside the system"); |
src/crew/brief.ts +1
| @@ -153,6 +153,7 @@ export function systemPrompt(input: BriefInput): string { | ||
| 153 | 153 | "- Keep state under `FORGE_APP_DATA`. Never write elsewhere at runtime.", |
| 154 | 154 | "- The app's terminal (`tty` in forge.json) is reached with the exact command in `FORGE_APP_SSH` (for example `ssh -p 2222 notes@localhost`: the app is the login name, the forge is the host). When a page or the terminal tells people how to connect, print that variable verbatim and never invent a form like `[email protected]`; when it is unset, say the terminal is off.", |
| 155 | 155 | "- Run the tests (`bun test`, or whatever `forge.json` names) before you push: the deploy runs them, and a failing test fails the preview. When a test fails, fix the code; change the test only when the test was wrong, and say which in the pull.", |
| 156 | "- Touching src/web/: run `bun scripts/design-lint.ts` before you push; it names the file:line and what to use instead. A new component class is named in your own docs/design/areas/<area>.md, never in docs/DESIGN.md, which every pull would otherwise edit.", | |
| 156 | 157 | "- The bar is the forge's own: small modules named for one concept, every input validated at the boundary and every output escaped, the security floor in `src/server.ts` kept on every response, no dependency without a reason in the pull, and a page that answers in milliseconds. `AGENTS.md` in the repository says how this app is shaped; keep its shape.", |
| 157 | 158 | ...(input.previewUrl === undefined |
| 158 | 159 | ? [] |
test/design-lint.test.ts +45 −3
| @@ -1,6 +1,6 @@ | ||
| 1 | 1 | import { describe, expect, test } from "bun:test"; |
| 2 | 2 | import { readFileSync } from "node:fs"; |
| 3 | import { classesIn, compare, countFile, documentedClasses, registeredClasses, scan, webCss } from "../scripts/design-lint.ts"; | |
| 3 | import { areaFiles, classesIn, compare, countFile, documentedClasses, locate, registeredClasses, scan, webCss } from "../scripts/design-lint.ts"; | |
| 4 | 4 | |
| 5 | 5 | /** A DESIGN.md in miniature: the §5 table, a §5d register, and a §6 that names a class too. */ |
| 6 | 6 | const DESIGN = [ |
| @@ -25,7 +25,7 @@ const SLOW = 60_000; | ||
| 25 | 25 | describe("design check: nothing added outside the system (#2678)", () => { |
| 26 | 26 | test("the tree is no worse than its baseline", () => { |
| 27 | 27 | const base = JSON.parse(readFileSync("docs/design/baseline.json", "utf8")); |
| 28 | expect(compare(base, treeNow(), readFileSync("docs/DESIGN.md", "utf8"))).toEqual([]); | |
| 28 | expect(compare(base, treeNow(), readFileSync("docs/DESIGN.md", "utf8"), areaFiles("."))).toEqual([]); | |
| 29 | 29 | }, SLOW); |
| 30 | 30 | |
| 31 | 31 | test("counts colour literals and off-scale sizes, not on-scale ones", () => { |
| @@ -91,7 +91,49 @@ describe("design check: nothing added outside the system (#2678)", () => { | ||
| 91 | 91 | }); |
| 92 | 92 | |
| 93 | 93 | test("every class the stylesheets define is one DESIGN.md §5 names (#2700)", () => { |
| 94 | const named = new Set(documentedClasses(readFileSync("docs/DESIGN.md", "utf8"))); | |
| 94 | const named = new Set(documentedClasses(readFileSync("docs/DESIGN.md", "utf8"), areaFiles("."))); | |
| 95 | 95 | expect(treeNow().classes.filter((k) => !named.has(k))).toEqual([]); |
| 96 | 96 | }, SLOW); |
| 97 | ||
| 98 | test("a red names the file:line, the literal and the token to use (#4063)", () => { | |
| 99 | const text = "const css = /* css */ `\n.card{\n color: #abc;\n font-size: 13.5px;\n}`;"; | |
| 100 | expect(locate(text)).toEqual([ | |
| 101 | { line: 3, kind: "colour", text: "#abc", use: expect.stringContaining("var(--fg)") }, | |
| 102 | { line: 4, kind: "size", text: "font-size: 13.5px", use: expect.stringContaining("13px") }, | |
| 103 | ]); | |
| 104 | const base = { files: {} }; | |
| 105 | const now = { classes: ["btn", "run-log", "x-card", "x-row", "x-wrapped"], files: { "src/web/x.ts": countFile(text) }, found: { "src/web/x.ts": locate(text) } }; | |
| 106 | const [colour, size] = compare(base, now, DESIGN); | |
| 107 | expect(colour).toContain("src/web/x.ts:3 #abc → use var(--fg)"); | |
| 108 | expect(size).toContain("src/web/x.ts:4 font-size: 13.5px → use 13px"); | |
| 109 | }); | |
| 110 | ||
| 111 | test("an unknown class says where to name it, and what is alike (#4063)", () => { | |
| 112 | const now = { files: {}, classes: ["btn", "run-log", "x-card", "x-row", "x-wrapped", "btn-big"] }; | |
| 113 | const [msg] = compare({ files: {} }, now, DESIGN); | |
| 114 | expect(msg).toContain("docs/design/areas/<area>.md"); | |
| 115 | expect(msg).toContain(".btn"); | |
| 116 | }); | |
| 117 | ||
| 118 | test("two pulls naming different components in their own area files do not touch DESIGN.md (#4063)", () => { | |
| 119 | const now = { files: {}, classes: ["btn", "run-log", "x-card", "x-row", "x-wrapped", "gizmo", "widget"] }; | |
| 120 | expect(compare({ files: {} }, now, DESIGN)).toHaveLength(2); | |
| 121 | expect(compare({ files: {} }, now, DESIGN, ["`.gizmo` a gizmo"])).toHaveLength(1); | |
| 122 | expect(compare({ files: {} }, now, DESIGN, ["`.gizmo` a gizmo", "`.widget` a widget"])).toEqual([]); | |
| 123 | }); | |
| 124 | ||
| 125 | test("a named colour is reported at its own line, repeats included (#4063)", () => { | |
| 126 | const text = "const css = /* css */ `\na{color: red}\nb{color: blue}\nc{color: red}`;"; | |
| 127 | expect(locate(text).map((f) => f.line)).toEqual([2, 3, 4]); | |
| 128 | }); | |
| 129 | ||
| 130 | test("the areas README is not vocabulary (#4063)", () => { | |
| 131 | const readme = readFileSync("docs/design/areas/README.md", "utf8"); | |
| 132 | const own = namedFromReadme(readme); | |
| 133 | expect(own.length).toBeGreaterThan(0); | |
| 134 | const read = areaFiles(".").join("\n"); | |
| 135 | for (const k of own) expect(read).not.toContain("`." + k + "`"); | |
| 136 | }); | |
| 97 | 137 | }); |
| 138 | ||
| 139 | const namedFromReadme = (text: string) => documentedClasses("## 5. x\n" + text); |
new above
offline
indexOf finds the first occurrence in the file, not the match: the line is wrong for repeats. Use the match offset.