Skip to content

feat(core): rule color/palette-conformance - #112

Merged
aram-devdocs merged 2 commits into
mainfrom
codex/23-feat-core-color-palette-conformance
Apr 25, 2026
Merged

feat(core): rule color/palette-conformance#112
aram-devdocs merged 2 commits into
mainfrom
codex/23-feat-core-color-palette-conformance

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Target branch

  • This PR targets main

Spec

Fixes #23

Summary

  • Adds the color/palette-conformance built-in rule. Flags computed colors whose CIEDE2000 distance to every entry in color.tokens exceeds color.delta_e_tolerance (default 2.0).
  • Translucent colors are composited over the closest opaque ancestor background-color (default #ffffff) in linear sRGB before the ΔE measurement, so overlays are judged where they actually render.
  • Palette is converted to CIE Lab once per check call (never per node) and cached as Vec<PaletteEntry>.

Crates touched

  • plumb-core
  • plumb-format
  • plumb-cdp
  • plumb-config
  • plumb-mcp
  • plumb-cli
  • xtask
  • docs/
  • .agents/ or .claude/
  • .github/

System impact

  • New public API item (needs doc + # Errors section if fallible)
  • New MCP tool (needs tools/list entry + protocol test)
  • New rule (needs docs page + golden test + register_builtin entry)
  • CDP / browser surface change (needs security-auditor review)
  • Config schema change (run cargo xtask schema + commit result)
  • Dependency added / bumped (cargo-deny must still pass)
  • Determinism invariant touched (see .agents/rules/determinism.md)

The rule reuses Config::color (tokens + delta_e_tolerance), which already shipped with PR #109; no schema change required.

Architectural compliance

  • Layer discipline: plumb-core has no internal deps; unsafe only in plumb-cdp; println!/eprintln! only in plumb-cli.
  • Error shape: thiserror-derived in libs; anyhow only in plumb-cli::main.
  • No new unwrap/expect/panic! in library crates.
  • No new SystemTime::now / Instant::now in plumb-core.
  • No new HashMap in observable output paths (use IndexMap).
  • Every #[allow(...)] is local and has a one-line rationale.

Test plan

  • just validate passes locally
  • cargo xtask pre-release passes (4 built-in rules; all have docs pages)
  • just determinism-check passes (3× byte-diff clean)
  • cargo deny check passes
  • New/changed behavior has a test — unit tests in palette_conformance.rs::tests, golden snapshot in tests/golden_color_palette_conformance.rs

Documentation

  • Rustdoc added for every new public item (PaletteConformance)
  • # Errors section on every new public fallible fn (no new fallible fn)
  • docs/src/ updated when user-visible behavior changed
  • docs/src/rules/<category>-<id>.md written for new rules
  • CHANGELOG updated if user-visible (otherwise release-please handles it)
  • Humanizer skill run on docs changes (avoided AI-tell phrasing in color-palette-conformance.md)

Breaking change?

  • No
  • Yes — describe migration path

Checklist

  • Conventional Commits title
  • Branch name: codex/<primary>-<type>-<slug>
  • All review gates passed: spec → quality → architecture → test (+ security if triggered)
  • /gh-review --local-diff main...HEAD run locally

Implementation notes

ΔE00 implementation choice. palette 0.7 only implements Ciede2000 for Lab<D65, _> and Lch<D65, _>; the Oklab type carries EuclideanDistance only. Per the spec's "use what palette exposes" hint, I converted candidates and tokens through LinSrgb → Xyz → Lab(D65) and used Ciede2000::difference for ΔE00. Math runs on f32 to keep the conversion path light.

Backdrop fallback. resolve_backdrop walks parent_dom_order until it finds an ancestor with background-color and alpha == 1.0. If the walk never finds one (or hits the root), the rule defaults to CssColor { r: 1.0, g: 1.0, b: 1.0, a: 1.0 } — the User Agent default. The walk starts at start.parent, never the start node, since the start node's own color is what's being judged.

Compositing. Standard "source over" Porter–Duff in linear-light sRGB. The ancestor walk guarantees the destination has alpha == 1.0, so the simplified out = src.a * src + (1 - src.a) * dst form is correct.

🤖 Generated with Claude Code

Flags computed colors whose CIEDE2000 distance to every entry in
`color.tokens` exceeds `color.delta_e_tolerance` (default 2.0). The
palette is converted to CIE Lab once per check call. Translucent
foregrounds are composited over the closest opaque ancestor
`background-color` (defaulting to `#ffffff`) before the ΔE measurement
so overlays are judged where they actually render.

Properties iterated: `color`, `background-color`, the four
`border-*-color` longhands, and `outline-color`. One violation per
`(node, property)` pair. Adds a `parse_css_color` helper that handles
the `rgb(...) / rgba(...) / #hex` shapes Chromium normalizes to.

Closes #23.

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

github-actions Bot commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

I have a complete picture. Here is the full review.


Review: feat(core): rule color/palette-conformance + fix(core): downgrade color/palette violation confidence on translucent sources

Files reviewed: crates/plumb-core/src/rules/color/{mod,palette_conformance}.rs, crates/plumb-core/src/rules/{mod,util}.rs, crates/plumb-core/tests/golden_color_palette_conformance.rs, crates/plumb-core/tests/snapshots/…snap, docs/src/rules/color-palette-conformance.md


1. Determinism — ✅ Clean

  • No SystemTime, Instant, or rand anywhere in the production path.
  • metadata fields are inserted into IndexMap in the same hard-coded order every call; serialization is deterministic.
  • COLOR_PROPERTIES (color/mod.rs:20–27) is alphabetically sorted — within-node emission order is deterministic and that order is preserved by the engine's stable sort on (rule_id, viewport, selector, dom_order).
  • nearest_palette_entry uses strict < and comment-documents the first-seen tie-winner policy (palette_conformance.rs:194), which is deterministic given IndexMap's insertion-order guarantee on config.color.tokens.
  • delta_e_metadata rounds to 3 d.p. before serialization (palette_conformance.rs:225), eliminating float-printing churn.
  • No HashMap/HashSet in any observable output path.

2. Workspace layering — ✅ Clean

  • All new code lives in plumb-core. No internal cross-crate imports.
  • No unsafe, no println!/eprintln!, no tracing inside check.
  • palette, indexmap, serde_json were already present in plumb-core's dependency set; no new crate boundaries crossed.

3. Error handling — ✅ Clean

  • All expect() hits are inside #[cfg(test)] blocks (palette_conformance.rs:339,341,347,357; util.rs:384–441). The production path has none.
  • unwrap_or(input) at util.rs:111 is the non-panicking combinator — safe; strip_prefix('#') is called only after starts_with('#') is confirmed.
  • delta_e_metadata(...).unwrap_or(serde_json::Value::Null) at palette_conformance.rs:121,125 is defensive and correct; the ΔE for valid in-gamut colors is always finite, making the fallback dead in practice but harmless.
  • The early-return guard on tolerance.is_finite() || tolerance < 0.0 (palette_conformance.rs:63) is correct defensive design.

4. Test coverage — ✅ Solid

  • Golden snapshot test covering: exact match (no violation), within-tolerance (no violation), off-palette opaque (violation), off-palette translucent-over-ancestor (violation, Confidence::Low).
  • Determinism test: three consecutive runs diffed at the String level.
  • Empty-palette edge case: zero violations asserted.
  • util.rs unit tests cover all six hex lengths, rgb()/rgba() with commas, space-separated CSS Color 4 form, percentage channels, transparent, and malformed inputs.
  • palette_conformance.rs unit tests cover build_palette skip-unparseable, nearest_palette_entry minimum-delta, and three composite_over invariants.

5. Documentation — ✅ Clean

  • Full docs/src/rules/color-palette-conformance.md with status, severity, behavior description, example violation JSON, configuration TOML, suppression guidance, and "See also" cross-links.
  • RFC 2119 MUST/SHOULD used correctly for normative statements.
  • No AI-tell phrasing ("dive in", "seamless", "leverage", etc.) detected.
  • doc_url wired correctly at palette_conformance.rs:141.
  • All pub(crate) items in util.rs have field-level doc comments.

Punch list (non-blocking warnings)

palette_conformance.rs:238–243node_by_dom_order is O(n) per call

fn node_by_dom_order(snapshot: &PlumbSnapshot, dom_order: u64) -> Option<&SnapshotNode> {
    snapshot.nodes.iter().find(|n| n.dom_order == dom_order)
}

resolve_backdrop calls this inside a while loop that climbs the ancestor chain, once per translucent node. Worst-case complexity is O(nodes × ancestor_depth) per rule invocation. A pre-built IndexMap<u64, usize> (dom_order → vec index), built alongside parent_index on line 77, would drop this to O(1) per hop. Not a blocker at current snapshot sizes, but a real cliff for large pages with many translucent layers.

util.rs:156Vec allocation in parse_rgb_functional hot path

let parts: Vec<&str> = if inner.contains(',') { ... } else { ... };

Called for every color property of every node. A fixed [&str; 4] with a manual split loop avoids the heap allocation. Minor at current call volumes; note for future optimization.


Verdict: APPROVE

…t sources

When the source color carries `alpha < 1.0`, the rule's nearest-palette
match runs against the COMPOSITED appearance (over the resolved opaque
background), not the literal CSS value. Replacing the literal with the
opaque token's hex therefore changes more than the rendered color.
Branch the fix's `confidence` to `Low` in that case; opaque sources
keep `Medium`.

Also add unit tests for `parse_css_color` covering hex-3, hex-4, hex-8,
percentage rgb, whitespace-separated rgb, rgba, the `transparent`
keyword, and malformed input.

Regenerate the golden snapshot — the translucent fixture now reports
`"confidence": "low"`.
@aram-devdocs
aram-devdocs merged commit 0220e55 into main Apr 25, 2026
14 checks passed
@aram-devdocs
aram-devdocs deleted the codex/23-feat-core-color-palette-conformance branch April 25, 2026 13:42
aram-devdocs added a commit that referenced this pull request Apr 25, 2026
Brings in #112 (color/palette-conformance rule) and #114 (CSS
custom-properties scraper). Resolves a trivial conflict in
plumb-config/src/lib.rs (both branches added a new mod + pub use;
combine them) and corrects the CHANGELOG entry to reflect the
lowered MAX_NESTING (256 -> 64) from the prior review-feedback
commit.
aram-devdocs added a commit that referenced this pull request Apr 25, 2026
Brings the color/palette-conformance rule (#112) onto the MVP-bundle
branch and corrects a stale 'rounded mean' inline comment in
edge/near_alignment.rs that the prior review pass left untouched.
aram-devdocs added a commit that referenced this pull request Apr 25, 2026
- docs/src/rules/sibling-height-consistency.md: align example fix
  text with the committed snapshot ('Drift: 30px.').
- docs/src/rules/overview.md + docs/src/SUMMARY.md: list
  color/palette-conformance now that PR #112 has merged, and trim the
  stale 'Coming soon' paragraph that still listed all three landed
  categories.
- crates/plumb-core/src/rules/edge/near_alignment.rs: drop the dead
  '#[allow(clippy::too_many_arguments)]' (clippy.toml threshold is
  12; the helper has 11 args), keep the rationale comment, and add a
  one-line invariant note above the unreachable '> tolerance' guard.
aram-devdocs added a commit that referenced this pull request Apr 25, 2026
* feat(core): rule radius/scale-conformance

Add the radius/scale-conformance rule, mirroring spacing/scale-conformance
but operating over the four physical-longhand corner-radius properties.
Border-radius values not within 0.5px of a member of `radius.scale` fire
a violation with a nearest-in-scale fix at confidence: medium.

Fixes #24

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

* feat(core): rule a11y/touch-target

Add the a11y/touch-target rule, implementing WCAG 2.5.8 (Target Size,
Minimum). Interactive nodes (button/select/textarea, anchor with href,
button-shaped input, role="button") whose rendered Rect is below
`a11y.touch_target.min_width_px` × `min_height_px` (default 24×24)
fire a violation, with a description-shaped fix at confidence: low.

Fixes #26

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

* feat(core): rule sibling/height-consistency

Add the sibling/height-consistency rule: groups DOM siblings by parent,
clusters them into visual rows by top-edge proximity (±2 CSS px) and
horizontal overlap (≥50% of the smaller width), then flags any element
whose height drifts from its row's median by more than 4 CSS px.
Falls back to a single DOM-sibling group when row clustering yields
only singletons (e.g. absolute positioning, transforms). Default
severity Info.

Fixes #25

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

* feat(core): rule edge/near-alignment

Add the edge/near-alignment rule. For each parent group of siblings,
the rule clusters left/right/top/bottom edges within
`alignment.tolerance_px` (default 3 CSS px) and flags any element
whose edge sits within `0 < delta <= tolerance_px` of its cluster's
centroid. Pixel-perfect alignments (delta == 0) and clusters of size
< 2 are skipped. Default severity Info.

Fixes #27

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

* fix(core): address review feedback on MVP rule bundle

- Switch private intra-doc links in `sibling/height_consistency.rs`
  module-level docs to inline code spans (fixes
  `cargo doc -Dwarnings` failure on `private_intra_doc_links`).
- Replace "drift is small but visible" wording in violation fix text;
  the deviation can be 30+px which is not small. Use neutral
  "Drift: {dev}px." Also regenerate the golden snapshot.
- Correct misleading test comment on the three-cards fixture in
  `golden_sibling_height_consistency.rs`; that fixture exercises the
  DOM-sibling fallback, not the primary clustering path.
- Fix terminology in `edge/near_alignment.rs` and
  `docs/src/rules/edge-near-alignment.md`: the centroid is an
  integer truncated mean (`sum / len`), not a rounded mean.

* docs(core): correct terminology and fixture comment for MVP rules

- `edge/near_alignment` module doc + docs page: the cluster centroid
  is an integer mean (`sum / len`, truncating), not a rounded mean.
- `golden_sibling_height_consistency` fixture comment: clarify that
  the three-cards row exercises the DOM-sibling fallback (the cards
  are spaced 220px apart with no rect overlap), and point readers
  to the dedicated unit test for the primary clustering path.

* fix(core): document clippy::too_many_arguments rationale on emit_violation

Per CLAUDE.md, every `#[allow(...)]` carries a one-line rationale.
The `emit_violation` helper takes the loop's locals; grouping into
a struct duplicates them without hiding complexity.

* fix: address review nits on PR #113

- docs/src/rules/sibling-height-consistency.md: align example fix
  text with the committed snapshot ('Drift: 30px.').
- docs/src/rules/overview.md + docs/src/SUMMARY.md: list
  color/palette-conformance now that PR #112 has merged, and trim the
  stale 'Coming soon' paragraph that still listed all three landed
  categories.
- crates/plumb-core/src/rules/edge/near_alignment.rs: drop the dead
  '#[allow(clippy::too_many_arguments)]' (clippy.toml threshold is
  12; the helper has 11 args), keep the rationale comment, and add a
  one-line invariant note above the unreachable '> tolerance' guard.

---------

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/palette-conformance (Oklab ΔE00)

1 participant