Skip to content

feat(config): add DTCG 2025.10 token adapter - #111

Merged
aram-devdocs merged 4 commits into
mainfrom
codex/28-feat-config-dtcg-adapter
Apr 25, 2026
Merged

feat(config): add DTCG 2025.10 token adapter#111
aram-devdocs merged 4 commits into
mainfrom
codex/28-feat-config-dtcg-adapter

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Spec

Fixes #28

Summary

  • Adds plumb_config::merge_dtcg(&mut Config, &DtcgSource) — a Design Tokens Community Group (DTCG) JSON importer that maps tokens onto ColorSpec, SpacingSpec, TypeScaleSpec, and RadiusSpec, resolves {path.to.token} brace aliases and { "$ref": "#/..." } JSON-Pointer references with cycle detection, caps tree depth at 256 levels for untrusted input, and reuses the existing is_valid_hex_color helper from PR feat(config): span-annotated validation errors via miette #110.
  • Surfaces non-fatal issues (unsupported $type values, multi-mode siblings, duplicate token names) as a typed DtcgWarning list rather than failing the import.
  • Three round-trip fixtures (flat-palette.json, nested-aliases.json, multi-mode.json) plus 12 integration tests and 8 unit tests cover the spec examples, the alias forms, and the security-sensitive depth/type/hex guards.

Test plan

  • just check passes (fmt + clippy, no warnings)
  • just test passes on my machine
  • just determinism-check passes
  • cargo deny check passes
  • New tests added (or reason-not given)
  • Docs updated (docs/src/**, rustdoc, CHANGELOG if user-visible)
  • Humanizer skill run on any docs/src/** prose (no docs/src/** changes in this PR)

Screenshots / terminal output

n/a — merge_dtcg is a library API, not a user-visible CLI surface.

Breaking change?

  • No
  • Yes — describe the migration path

The new symbols are additive (DtcgSource, DtcgImport, DtcgWarning, DtcgWarningKind, MAX_NESTING, merge_dtcg) and two new ConfigError variants (DtcgParse, DtcgAlias). The existing ConfigError enum was already #[non_exhaustive], so adding variants is non-breaking.

Anything reviewers should double-check

  • Dimension namespace heuristic. Bare $type: "dimension" tokens go to SpacingSpec.tokens unless the path contains one of typography / type / font-size / text / font / size, in which case they go to TypeScaleSpec.tokens. The list is documented in the rustdoc on dtcg.rs. Open to expanding the typography keys if Tokens Studio exports name them differently.
  • Dimension units. Only unitless and px dimensions are accepted. rem / em raise Unconvertible warnings — there's no DPI/root-size context in Config, so converting silently would be misleading.
  • Multi-mode handling. The canonical $value always wins; mode payloads (DTCG $extensions.modes) raise MultiMode warnings rather than overwriting. Plumb does not yet model design-token modes; this keeps imports loss-tolerant rather than ambiguous.
  • Cycle reporting. ConfigError::DtcgAlias carries the visit order. Single-element cycles encode dangling references — the missing path is the only entry. Two-or-more entries are real cycles.
  • Untrusted input. MAX_NESTING = 256 is enforced in a separate pass after serde_json has already accepted the document; this is belt-and-braces because serde_json's default recursion limit is 128 and not configurable per-call. Hex colors go through the existing is_valid_hex_color so a DTCG file cannot smuggle in a non-hex string.
  • Schema impact. merge_dtcg is internal to plumb-config; nothing flows through Config serialization. cargo xtask schema produces byte-identical output, verified locally.

CI checkboxes

  • Dependency added / bumped — added indexmap to plumb-config/Cargo.toml (already a workspace dep used by plumb-core); no new external deps.
  • CDP / browser surface change — no.

Imports a Design Tokens Community Group JSON document into a `Config`
through a new public `plumb_config::merge_dtcg(&mut Config, &DtcgSource)`
entry point. Callers own filesystem reads; the adapter is a pure
function of `(input bytes, into.snapshot)`.

Mapped `$type` values:

* `color`             → `ColorSpec.tokens` (hex `#rgb` / `#rrggbbaa`)
* `dimension`         → `SpacingSpec.tokens` or `TypeScaleSpec.tokens`
                        via a parent-namespace heuristic
                        (`typography` / `type` / `font-size` / `text` /
                        `font` / `size` route to typography; everything
                        else routes to spacing)
* `fontFamily`        → `TypeScaleSpec.families` (deduped, insertion-order)
* `fontWeight`        → `TypeScaleSpec.weights`
* `radius` / `borderRadius` → `RadiusSpec.scale`
* `shadow`, `duration`, `cubicBezier`, etc. → `DtcgWarning::UnsupportedType`
                        (no hard error — the tokens are dropped and reported)

Alias resolution is a single forward pass with cycle detection. Both
DTCG forms are accepted: the brace shorthand `"{path.to.token}"` and
the JSON-Pointer object `{ "$ref": "#/path/to/token" }`. Cycles raise
`ConfigError::DtcgAlias` with the visit order so the failing edge is
human-readable; dangling references raise the same variant with a
single-element cycle naming the missing path.

Defensive parsing for untrusted input: tree depth is capped at 256
levels before walking, malformed JSON returns `ConfigError::DtcgParse`
with a miette `NamedSource`, hex colors reuse the existing
`is_valid_hex_color` helper.

Three fixtures under `tests/fixtures/dtcg/`:

* `flat-palette.json`   — bare color palette.
* `nested-aliases.json` — multi-section file (color, spacing,
                          typography, radius, shadow) with brace
                          aliases across groups.
* `multi-mode.json`     — `$extensions.modes` payload; the canonical
                          `$value` imports, alternate modes surface as
                          `MultiMode` warnings.

Twelve integration tests in `tests/dtcg_adapter.rs` plus eight unit
tests cover the happy paths, alias cycles, dangling refs, malformed
JSON, depth-limit enforcement, invalid hex colors, multi-mode handling,
duplicate names, and the `{path}` ↔ `$ref` parity.

The `merge_dtcg` API is internal to `plumb-config`; it does not flow
through `Config` serialization, so the JSON Schema emitted by
`cargo xtask schema` is byte-identical.

Fixes #28
@github-actions

github-actions Bot commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

PR #111feat(config): add DTCG 2025.10 token adapter

Scope: crates/plumb-config/ only. New module dtcg.rs (878 lines) + integration tests (454 lines) + 3 fixtures. One new workspace dep (indexmap — already in the workspace graph).


1. Determinism

dtcg.rs:64,209,343,400std::collections::HashSet instead of indexmap::IndexSet

The determinism rule says: "Use indexmap::IndexMap / IndexSet. The ahash-backed variants … are fine as internal accelerators if their iteration order never leaks out."

seen: HashSet<String> is used exclusively with .contains() / .insert() / .remove() — it is never iterated, so its order does not leak into resolved (IndexMap) or visiting (Vec). The DtcgAlias.cycle field is built from visiting, not from seen. The HashSet is functionally correct and does not break output determinism.

However, the workspace convention is IndexSet for all set-like structures, and indexmap is already a declared dependency after this PR. This should be changed to IndexSet<String> for style consistency, but it is not a correctness blocker.

No HashMap in observable output. resolved and tokens are both IndexMap. schemars/preserve_order is enabled workspace-wide, which transitively enables serde_json/preserve_order; serde_json::Map iteration is therefore insertion-order and deterministic. ✓

No wall-clock sources, no SystemTime::now, no Instant::now. ✓


2. Workspace layering

plumb-configplumb-core
New dep: indexmap (already in workspace) ✓
No println! / eprintln! in library code ✓
No unsafe
#![forbid(unsafe_code)] assumed present from workspace lints ✓


3. Error handling

merge_dtcg returns Result<DtcgImport, ConfigError> with thiserror-derived variants. All error paths use ? or explicit Err(...). No bare unwrap or expect in library code. #![allow(clippy::expect_used)] is scoped to the integration test file, where clippy.toml's allow-expect-in-tests = true already covers it (redundant, harmless). ✓

dtcg.rs:345resolve_alias is recursively unbounded on alias chain depth

The JSON nesting cap (MAX_NESTING = 64) catches a deeply nested JSON tree, but a flat JSON with a long linear alias chain (A→B→C→…→N) bypasses it entirely. A crafted file with ~900 non-cyclic chained aliases causes unbounded recursion → stack overflow → process panic from library code. Given that the author explicitly added the nesting guard for DoS protection, this looks like an oversight. It does not violate a clippy rule but it violates the spirit of the no-panic-in-library policy. Recommend adding a visiting.len() > MAX_ALIAS_CHAIN guard at the top of resolve_alias.


4. Test coverage

Unit tests (8) in dtcg.rs cover: brace alias parsing, $ref alias parsing, garbage rejection, object/string dimension forms, non-px unit rejection, typography heuristic, depth overflow + pass. ✓

Integration tests (12) cover: flat round-trip, insertion-order assertion, nested aliases with all mapped types, alias cycles, dangling references, multi-mode, unsupported type, malformed JSON, invalid hex, deep JSON nesting, $ref-style alias, duplicate name, nesting_above_plumb_cap, radius unconvertible type field. ✓

dtcg_adapter.rsflat_palette_round_trip_preserves_insertion_order is a weak test

The fixture keys (brand-primary, brand-secondary, neutral-bg, neutral-fg) happen to be in alphabetical order already. The assertion passes whether or not serde_json/preserve_order is active, so it does not actually prove the insertion-order contract advertised in the module doc. The test name is misleading. Not a blocker, but the fixture should include at least one key that precedes a prior key alphabetically (e.g. accent/blue) to actually exercise the ordering claim.


5. Documentation

Module-level doc with a $type mapping table, alias resolution prose, untrusted-input section, and intra-doc links: excellent. ✓
merge_dtcg has an # Errors section listing both error variants. ✓
All public items (DtcgSource, DtcgImport, DtcgWarning, DtcgWarningKind, MAX_NESTING, merge_dtcg) are documented. ✓
CHANGELOG.md updated under ## [Unreleased] with an accurate summary (CHANGELOG correctly says "64 levels"; PR description body mistakenly says "256 levels" — no code impact). ✓

Minor: ConfigError::DtcgAlias reuses one variant for both alias cycles and dangling references. The error format string includes (cycle: {cycle:?}) even for dangling refs, which is semantically odd. The reason field disambiguates, but consider DtcgAlias { dangling: bool, … } or a separate DtcgDangling variant to make the API self-describing.


Punch list

File Line Class Finding
dtcg.rs 64, 209, 343, 400 WARN HashSet<String>IndexSet<String> per workspace convention; indexmap dep already present
dtcg.rs 351 (top of resolve_alias) WARN No alias-chain depth cap; a flat file with ~900 chained aliases can stack-overflow from library code
dtcg_adapter.rs ~54 (flat_palette_round_trip_preserves_insertion_order) WARN Fixture is already alphabetically ordered; test does not prove the insertion-order contract
lib.rs 138 (DtcgAlias variant) MINOR Variant used for both cycles and dangling refs; (cycle: …) in format string is misleading for dangling case
PR body INFO Body says "256 levels"; code and tests say 64; no code impact

Verdict: APPROVE

@aram-devdocs aram-devdocs changed the title feat(config): DTCG 2025.10 token adapter feat(config): add DTCG 2025.10 token adapter Apr 25, 2026
aram-devdocs and others added 3 commits April 25, 2026 07:23
ColorSpec / SpacingSpec / TypeScaleSpec / RadiusSpec live under
`plumb_core::config::`, not at the crate root. Update the rustdoc
references so `cargo doc -Dwarnings` resolves them.
- Lower MAX_NESTING from 256 to 64 so exceeds_depth covers the
  range below serde_json's 128 recursion limit; add a regression
  test that exercises the check directly.
- Plumb the actual $type into apply_radius so Unconvertible
  warnings report "radius" vs "borderRadius" accurately; add
  regression test.
- Remove redundant DuplicateName::name field; the path is already
  on DtcgWarning.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
aram-devdocs merged commit 778e212 into main Apr 25, 2026
14 checks passed
@aram-devdocs
aram-devdocs deleted the codex/28-feat-config-dtcg-adapter branch April 25, 2026 14:32
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): DTCG 2025.10 token adapter

1 participant