Skip to content

feat(core): add color/contrast-aa rule - #202

Merged
aram-devdocs merged 3 commits into
mainfrom
issue-68-color-contrast-aa
May 2, 2026
Merged

feat(core): add color/contrast-aa rule#202
aram-devdocs merged 3 commits into
mainfrom
issue-68-color-contrast-aa

Conversation

@aram-devdocs

@aram-devdocs aram-devdocs commented May 2, 2026

Copy link
Copy Markdown
Owner

Spec

Closes #68

Summary

  • add color/contrast-aa in plumb-core with WCAG 2.1 AA thresholds, large-text classification, relative-luminance contrast math, and composited background resolution from existing computed styles only
  • add a golden snapshot fixture plus rule-local unit tests that cover the 24px regular / 18.5px bold boundary and ancestor background resolution
  • register the rule and wire its docs through the book index, summary, configuration reference, and MCP explain_rule table

Validation

Confirmed in this environment:

  • cargo test -p plumb-core --test golden_color_contrast_aa --features test-fake -- --nocapture
  • cargo test -p plumb-core --all-features
  • cargo test -p plumb-mcp explain -- --nocapture
  • cargo clippy -p plumb-core --all-targets --all-features -- -D warnings
  • cargo fmt --all -- --check
  • cargo check --workspace --all-features
  • git diff --check

Environment blockers:

  • cargo test -p plumb-core without --all-features fails in this repo because integration tests call PlumbSnapshot::canned(), which is gated behind plumb-core/test-fake
  • cargo xtask pre-release fails here because Python lacks the yaml module required by the runbook validator
  • just validate cannot run here because just is not installed

Breaking change?

  • No
  • Yes — describe the migration path

Anything reviewers should double-check

  • a11y.min_contrast_ratio now acts as a stricter global floor on top of WCAG AA defaults; the default behavior remains fixed at 4.5:1 for normal text and 3.0:1 for large text.

@github-actions

github-actions Bot commented May 2, 2026

Copy link
Copy Markdown
Contributor

Here is the review from the general-purpose reviewer for the local diff (main...HEAD), which covers three commits adding the color/contrast-aa rule:


Plumb Code Review — color/contrast-aa rule

Commits reviewed: 4872ced, 8cc1cde, 0650777
Buckets touched: plumb-core, plumb-format (SARIF snapshot), plumb-mcp (explain table), docs


Blockers

None.


Warnings

W1 — crates/plumb-core/src/rules/color/contrast_aa.rs ~L102/217
The f32::EPSILON alpha-opaque check is correct in practice (since parse_css_color only returns exactly 1.0_f32 for no-alpha-channel paths), but this is a silent invariant. A one-line comment confirming it would prevent a future reader from widening the tolerance incorrectly.

W2 — crates/plumb-core/src/rules/color/contrast_aa.rs (check fn)
parent_index and nodes_by_dom_order are rebuilt from scratch on every Rule::check() invocation — O(n) per rule per viewport, repeated for every registered rule. This is an architectural limitation of the current Rule trait (no shared-state slot), not a defect in this PR. Acceptable today; worth a tracking issue if profiling shows it.

W3 — crates/plumb-core/tests/golden_color_contrast_aa.rs fixture comments
The comments mapping nth-child(N) → px size are accurate but the dom_order mapping is implicit. On a complex fixture this creates selector-confusion risk for future authors. A short table in the fixture doc comment would help. No functional impact.


Architecture compliance

Check Status
No new unsafe outside plumb-cdp PASS
No unwrap/expect/panic! in library crates PASS
No println!/eprintln! outside plumb-cli PASS
No SystemTime::now/Instant::now in plumb-core PASS
No HashMap/HashSet in observable output PASS — IndexMap used throughout
No todo!/unimplemented!/dbg! PASS
Rule has golden test PASS — golden_color_contrast_aa.rs + committed snapshot
Rule registered in register_builtin PASS
docs/src/rules/color-contrast-aa.md present PASS
MCP RULE_DOCS table updated PASS
SARIF snapshot updated PASS — ruleIndex incremented correctly
Layer discipline (plumb-core imports only) PASS
doc_url slug matches explain.rs PASS
No AI-vocabulary in docs PASS
No commented-out code / open-ended TODOs PASS

Correctness

  • WCAG 2.1 luminance formula is correct (palette sRGB linearisation → 0.2126R + 0.7152G + 0.0722B).
  • Large-text cutoffs are correct: 24.0 px (18 pt) and ≈18.667 px (14 pt bold, using 14 × 96/72).
  • Boundary fixture exhaustively covers 18 px / 18.6 px / 19 px at font-weight: 700 — all three snapshot entries are consistent with computed ratios.
  • Porter-Duff compositing is correct; background walk stops at the first opaque ancestor.
  • Determinism: IndexMap throughout, three-run determinism test present, output sorted by (rule_id, viewport, selector, dom_order).
  • At most one violation per node per viewport — satisfied.
  • min_contrast_ratio config knob is a floor (ratio.max(...)), not a replacement — confirmed by the config.a11y.min_contrast_ratio = Some(7.0) test case.

Verdict: APPROVE

@aram-devdocs
aram-devdocs merged commit 1794cc2 into main May 2, 2026
16 checks passed
aram-devdocs added a commit that referenced this pull request May 12, 2026
…292)

* fix(ci): unstick Preflight + tolerate Dogfood warnings/nextjs flake

PR #287 deleted docs/src/ci/release-prep.md and removed the matching
guard in tests/install-smoke-validate.sh, but missed the parallel guard
in tests/release-security-validate.sh. The validator has been failing
on every CI run since 2026-05-07, skipping every downstream job (MSRV,
deny, Test, Size, Coverage, Determinism, Docs).

Drop the stale RELEASE_PREP_DOC declaration and the corresponding
sentinel-grep block. The Homebrew + npm contracts are still validated
authoritatively against dist-workspace.toml and install-smoke.yml in
sections 5 and 6 of the same script.

Two follow-on dogfood fixes since the daily run has been red since
2026-05-02 (after the Phase 7 rule additions in #202/#204/#207):

- lint-canonical-docs now tolerates exit 3 (warnings/info only)
  alongside exit 0, mirroring the test-sites leg in the same workflow.
  Errors still fail. The 318-violation steady state on the live docs
  site is a separate content/theme cleanup, not a CI bug.
- nextjs leg gets continue-on-error on both plumb invocations to mirror
  the e2e-sites stance on the upstream chromiumoxide WebSocket flake
  tracked in #233.

Also drops the dead docs/src/ci/release-prep.md reference from the
install-smoke.yml header comment.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(cdp): gate macos_candidate ordering test to unix

The `macos_candidate_order_lists_system_apps_then_user_apps` test in
crates/plumb-cdp/src/chrome_path.rs asserts that paths returned by
`macos_candidates` start with `/Users/example/Applications/...` —
forward slashes against `Path::display()`. On Windows, `Path::join`
uses `\`, so display[3] becomes
`/Users/example\Applications\Google Chrome.app\...` and the
starts_with check fails.

`detect_with` short-circuits to `None` on non-macOS targets, so the
function is never reached in production on Windows. Gate the test to
`#[cfg(unix)]` to keep the existing Linux coverage and drop the
unreachable Windows leg.

This test was previously masked because Preflight has been failing on
every CI run since 2026-05-07, which skipped Test (windows-latest)
entirely. Surfacing now as a side effect of the validator fix in this
same PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(cdp): gate macos_candidates import to unix in tests

Follow-up to 890b14c. Gating the test to `#[cfg(unix)]` left
`macos_candidates` as an unused import on Windows, breaking the
`Test (windows-latest)` build with `error: unused import:
\`macos_candidates\``. Mirror the gate on the import.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(core): rule color/contrast-aa (WCAG 2.1 AA)

1 participant