Skip to content

feat(config): [color], [radius], [alignment], [a11y] sections + schema - #109

Merged
aram-devdocs merged 1 commit into
mainfrom
codex/21-feat-config-color-radius-alignment-a11y-sections
Apr 25, 2026
Merged

feat(config): [color], [radius], [alignment], [a11y] sections + schema#109
aram-devdocs merged 1 commit into
mainfrom
codex/21-feat-config-color-radius-alignment-a11y-sections

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Target branch

All PRs target main. Plumb has no dev branch.

  • This PR targets main

Spec

Fixes #21

PRD §12.2. Phase 2 batch 2A. Sets the config shape that the phase-2 rules (color/palette-conformance #23, radius/scale-conformance #24, edge/near-alignment #27, a11y/touch-target #26) will read from.

Summary

  • Flesh out the four PRD §12.2 sections: [color], [radius], [alignment], [a11y] — every nested struct keeps deny_unknown_fields.
  • Add alignment.tolerance_px (default 3) and a11y.touch_target.{min_width_px, min_height_px} (default 24×24 per WCAG 2.5.8). color.delta_e_tolerance default settles at 2.0 (the rule itself can override at consume time).
  • Rename radius.allowed_pxradius.scale to match spacing.scale and type.scale. Schema, example, and tests updated; the old name is rejected.

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)

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 (if rule or schema changed)
  • just determinism-check passes (3× byte-diff clean)
  • cargo deny check passes
  • New/changed behavior has a test (unit, golden snapshot, or integration)

Round-trip tests (crates/plumb-config/tests/load_example.rs):

  • loads_prd_color_radius_alignment_a11y_sections populates every new field and asserts the parsed values.
  • defaults_for_color_radius_alignment_a11y_sections writes an empty config and asserts every default (including delta_e_tolerance = 2.0, tolerance_px = 3, touch-target 24×24).
  • rejects_old_config_aliases_and_unknown_fields extended with [radius] allowed_px, plus unknown fields under alignment, a11y, a11y.touch_target, and color.
  • schema_uses_prd_names_and_drops_old_aliases extended to assert RadiusSpec.scale, AlignmentSpec.tolerance_px, A11ySpec.touch_target, and TouchTargetSpec.{min_width_px, min_height_px}.

Documentation

  • Rustdoc added for every new public item
  • # Errors section on every new public fallible fn (no new fallible APIs)
  • docs/src/ updated when user-visible behavior changed
  • docs/src/rules/<category>-<id>.md written for new rules (n/a)
  • CHANGELOG updated if user-visible (otherwise release-please handles it)
  • Humanizer skill run on docs changes

docs/src/configuration.md adds a per-section reference covering defaults and the slash-namespacing convention. Prose kept short; no AI-tell vocabulary.

Breaking change?

  • No
  • Yes — describe migration path

The pre-release radius.allowed_px field is renamed to radius.scale. Anyone on a plumb.toml from before this PR replaces:

[radius]
allowed_px = [0, 4, 8]

with:

[radius]
scale = [0, 4, 8]

The old name is rejected (deny_unknown_fields), so the failure surface is loud. No shipped rule consumed radius.* yet, so no rule-side compat shim is needed.

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

Reviewer notes

Decisions worth flagging:

  1. Flat palette only. The ticket left it open whether to add a nested palette form alongside the flat one. I kept the flat slash-namespaced shape ("bg/canvas" = "#...") and documented it as canonical — clarity > scope creep. If a nested form turns out to be ergonomic in dogfooding, it's a clean follow-up.
  2. radius.allowed_pxradius.scale (no alias). The pre-release schema is unstable, no shipped rule reads radius.* yet, so a hard rename is cheaper than a long-lived #[serde(alias = "allowed_px")]. The CHANGELOG flags it under Changed.
  3. delta_e_tolerance default stays 2.0. The phase-2 runbook for feat(core): rule color/palette-conformance (Oklab ΔE00) #23 mentions 1.5; the pre-existing default is 2.0 and feat(core): rule color/palette-conformance (Oklab ΔE00) #23 can override at the rule-default level if the spec confirms 1.5. Leaving it as-is keeps this PR scoped to schema shape, not threshold tuning.

Flesh out the PRD §12.2 config shape so phase-2 rules (color/palette,
radius/scale, edge/near-alignment, a11y/touch-target) can pull their
thresholds from typed config rather than hard-coded constants.

Changes:
- ColorSpec: keep flat slash-namespaced tokens (e.g. "bg/canvas") with
  delta_e_tolerance default 2.0. The flat shape is documented as the
  canonical form; nested-table support deferred per ticket guidance.
- RadiusSpec: rename allowed_px → scale for consistency with
  spacing.scale and type.scale. The old name is rejected (no alias)
  since no shipped rule consumes it yet.
- AlignmentSpec: add tolerance_px (default 3) for edge-cluster window.
- A11ySpec: add nested touch_target (TouchTargetSpec) defaulting to
  24×24 CSS px per WCAG 2.5.8.

deny_unknown_fields stays on every nested struct. Schema regenerated
via `cargo xtask schema`. Round-trip tests cover both populated and
default forms; old aliases continue to be rejected.

Fixes #21
@github-actions

Copy link
Copy Markdown
Contributor

Review: feat(config): [color], [radius], [alignment], [a11y] sections

Files changed: crates/plumb-core/src/config.rs, crates/plumb-config/tests/load_example.rs, schemas/plumb.toml.json, docs/src/configuration.md, examples/plumb.toml, CHANGELOG.md


1. Determinism

All config fields use IndexMap<String, String> for the token map — correct, insertion-order-stable, not hash-based. No HashMap/HashSet anywhere in the diff. No wall-clock, no RNG. Pass.

2. Workspace layering

All struct definitions land in plumb-core/src/config.rs (types layer); test changes land in plumb-config/tests/. No cross-layer violations, no new unsafe, no println! in library code. Pass.

3. Error handling

No unwrap/expect/panic! in production code. Test code uses .expect("create tempdir") which is fine under the testing rules. #[serde(deny_unknown_fields)] is present on every new struct. Pass.

4. The Default consistency fix

The PR correctly removes #[derive(Default)] from ColorSpec and AlignmentSpec and replaces it with manual impl Default that calls the named serde-default functions (default_delta_e(), default_alignment_tolerance_px()). This fixes a pre-existing schema bug where schemas/plumb.toml.json emitted "delta_e_tolerance": 0.0 instead of 2.0.

A11ySpec retains #[derive(Default)] — this is correct because its only non-Option sub-field is touch_target: TouchTargetSpec, which now has a manual impl Default. The derived derive will call that impl and produce { min_width_px: 24, min_height_px: 24 }. Pass.

5. Test coverage

Four tests cover the new surface:

  • loads_prd_color_radius_alignment_a11y_sections — happy path for all new fields.
  • defaults_for_color_radius_alignment_a11y_sections — default values verified.
  • rejects_old_config_aliases_and_unknown_fields — confirms [radius] allowed_px is rejected.
  • schema_uses_prd_names_and_drops_old_aliases — validates generated schema shape.

Pass.

6. Documentation

docs/src/configuration.md prose is clean. No AI-tell phrases found. Field descriptions in rustdoc are specific and precise. Pass.


Punch list

Warnings only — no blockers:

  • crates/plumb-config/tests/load_example.rs:103(cfg.color.delta_e_tolerance - 1.5).abs() < f32::EPSILON uses the wrong epsilon idiom: f32::EPSILON is the unit roundoff for values near 1.0; for a value of 1.5 the correct bound is 1.5 * f32::EPSILON. Works here only because 1.5 is exactly representable in IEEE 754. Consider < 1e-5 or < 1.5 * f32::EPSILON to make the intent obvious.

  • schemas/plumb.toml.json — The diff reads as a hand-edit rather than a cargo xtask schema regeneration (the CLAUDE.md for plumb-config mandates the latter). The committed file is structurally correct, but if CI's cargo xtask pre-release schema-drift check isn't run before merge, a divergence could slip through. Confirm the xtask was run.

  • crates/plumb-core/src/config.rs:192A11ySpec retains #[derive(Default)] while its peers (ColorSpec, AlignmentSpec) needed manual impls in this same PR. The derive is correct here because TouchTargetSpec::default() is explicitly implemented, but the inconsistency will confuse the next person who adds a non-zero-default field to A11ySpec. A one-line comment // TouchTargetSpec::default() gives 24×24; safe to derive would prevent the same mistake that required the ColorSpec/AlignmentSpec fix.


Verdict: APPROVE

@aram-devdocs
aram-devdocs merged commit f5b0047 into main Apr 25, 2026
14 checks passed
@aram-devdocs
aram-devdocs deleted the codex/21-feat-config-color-radius-alignment-a11y-sections branch April 25, 2026 12:28
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(config): [color], [radius], [alignment], [a11y] sections + schema

1 participant