Repository navigation
test(themes): hold every theme to WCAG AA contrast floors - #2676
Conversation
Add src/lib/themes.contrast.test.ts: 4.5:1 for interface text on each surface, primary-button labels and danger text, the notice, syntax and light ANSI floors DESIGN.md already promises, and 3:1 for the status indicator colours. The notice yellow broke DESIGN.md's 4.5:1 promise in seven theme entries; each keeps its hue and is darkened (lightened on Solarized dark) until it passes.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 42 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
|
Reviews at 550bca5:
The |
… too Codex review: --syntax-function and the --bg canvas the source viewer draws code on were not checked. Every theme already passes both.
|
Pushed 4dc4deb (test only): the contrast test now checks every syntax colour, |
|
CodeRabbit hit its review limit on this PR ("Review limit reached"), so no CodeRabbit review ran. Per the lane brief I'm not waiting for it. Standing reviews at 4dc4deb: omo (dori review) CLEAR, plus the Codex adversarial review summarised above. |
|
Merged main (#2693, the typecheck fix) into this branch; no other change. |
|
Merge note: owner approved the visible --notice colour change from before/after screenshots on 2026-11-04. CodeRabbit was rate-limited for this head; merged on the omo review (CLEAR at the head), Codex Terra (no HIGH; coverage MEDIUMs noted above) and green required checks. The one earlier Integration failure was an unrelated OpenCode contract test (fix lane running). |
…d ctrl+c (#2715) ## Summary `OpenCode v2 native service > closes the shared stream when its last verified pane ends or stops being OpenCode` failed once on #2676's CI (run 41746205793, job 126299818453): "the stream closing after the last verified pane stopped being OpenCode did not happen (native {"stream":true,"panes":["w8:p1"]})". The bridge was not at fault. The run's herdr log (artifact `integration-logs-41746205793-1`, session `check-e45a77-4`) shows the step's pane turning OpenCode and never leaving: ``` 00:44:57.995 agent changed pane=8 previous_agent=None agent=Some(OpenCode) process=opencode pgid=Some(17887) ... (no agent change for pane 8) ... 00:45:13.063 workspace closed workspace_id="w8" <- the test's cleanup, after the 15 s wait ``` A passing run logs `agent changed pane=N previous_agent=Some(OpenCode) agent=None` about 600 ms after the ctrl+c. Here herdr kept reporting OpenCode, so keeping the pane verified and the stream open was correct. The log does not say which link broke: the ctrl+c not reaching the stand-in's process group, the stand-in surviving it, or herdr missing its exit. This change removes the first two from the step; herdr still has to see the stand-in leave. ## Change Test only. The stand-in for the agent-change step blocks reading a FIFO (`read _ < leave.fifo`) and exits when the test writes a line to it. OpenCode still leaves the pane by itself and herdr still has to report the change, but the step no longer depends on key input and terminal signal delivery. The other two steps (pane end, handover) are unchanged. Every workspace the test creates is now closed at the end, also one whose start failed, since a stand-in waiting on the FIFO never ends by itself. ## Validation - Mutation checks on this branch: making `releaseOpenCodePane` a no-op fails the test at the pane-end step; removing only the agent-change releases in `server/core/status.ts` (live event and snapshot) fails it at the agent-change step, with the same error CI reported. So the step still catches a bridge that keeps a pane whose agent changed. - Loaded repeats (16 parallel workers pinned to 4 cores, each its own isolated herdr session): - main, before the fix: this test passed in every run I made (80 filtered runs, 64 whole-file runs, 24 runs with a forced subscription gap, 128 with random workspace churn around the ctrl+c, 80 whole-file runs with CI's herdr 0.9.3). The CI failure did not reproduce locally, so the fix rests on the CI herdr log above. - this branch, whole file, herdr 0.9.3 (CI's pinned version): this test passed 80/80. - Both main and this branch saw two other tests in the file fail under that contention at the same rate (main 9/80 runs, branch 10/80): "settles a send whose own message arrives before anything after its Enter can run" and "answers the catalog and a cold @ send within budget while opencode.json is a FIFO". This PR does not touch them. - `bun run check fast`: pass (after rebasing onto b6551ca). - `bun run check run bun test --timeout 15000 ./server/opencode.contract.test.ts`: 26 pass, 0 fail. - Skipped: `check full`, the browser lanes and the native Windows install (test-only change to one Linux/macOS integration file).
Summary
Theme contrast was kept by hand, and the only automated check was the sidebar selection demo regression. This adds a unit test that holds every theme entry in
src/styles.css(all 25 dark and light rows) to WCAG 2.2 AA:--text,--text-strongand--text-dimon--bg,--bg-panel,--bg-elevated,--bg-inputand--bg-hover; for--primary-texton--primary; and for--danger-texton--bgand--bg-panel.--bg-panel.--noticeat 4.5:1 on--bg,--bg-paneland--bg-elevated; the six literal--syntax-*colours on--bg-paneland--bg-elevated; and a light row's ANSI colours on its--term-bg.The terminal's default foreground is left out, because xterm draws every cell with
minimumContrastRatio: 4.5(useTerminalMount.ts). A colour that is not plain#rrggbbfails instead of passing unmeasured.The test found one real break.
--notice, the yellow of the file viewer's "256 KB of 1.3 MB" partial-file note, was below the 4.5:1 DESIGN.md promises in seven entries, at 1.87:1 to 4.15:1: Tokyo Night Day, Gruvbox Light, One Light, Solarized (dark and light), Kanagawa Lotus and Rosé Pine Dawn. Each one keeps its palette yellow's hue and moves only its lightness to reach 4.6:1, the same way earlier AA repairs were made. All seven are recorded inscripts/herdr-theme-overrides.tswith their dE, so the herdr provenance test accepts them, and DESIGN.md's notice paragraph names them.Not covered, for a follow-up decision (measured, not changed here):
--accenton surfaces (min 3.11:1, 6 entries below 4.5),--primary-texton--primary-hover(min 3.30:1),--status-idle(min 2.44:1), and--border/--border-strongas control edges (well below 3:1 in most themes). Whether each is used as text or as an essential control boundary needs a design call.Design idea credit: the idea of a test, rather than a person, keeping the contrast came from a study of another project's design system. No code was taken from it.
Validation
bun run check fast: all green (unit 10909 pass, 0 fail).bun test src/lib/themes.contrast.test.ts: 26 pass. With thesrc/styles.csschange reverted, 7 entries fail (one per broken notice). With a degraded--text-dimin one theme, that entry fails and names each pair and ratio.bun test scripts/herdr-themes.test.ts: 13 pass with the 7 new overrides; without them, the provenance test lists the 7 notice values.