Skip to content

feat(core): map design tokens to DESIGN.md fields - #490

Open
wudilyy999 wants to merge 10 commits into
OpenCoworkAI:mainfrom
wudilyy999:feat/design-system-token-mapping
Open

wudilyy999 wants to merge 10 commits into
OpenCoworkAI:mainfrom
wudilyy999:feat/design-system-token-mapping

Conversation

@wudilyy999

@wudilyy999 wudilyy999 commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Second step of #484 (design tokens from a folder into DESIGN.md), after #486. It turns the tokens the scan finds into DESIGN.md fields. Nothing calls it yet; the IPC, the preview with the per-field diff, and the write are step 3.

This is larger than the 400 lines I estimated in #484: about 510 lines outside tests and 630 in tests. The five commits stand alone, and I can open them as separate PRs if you prefer.

  1. --font-* fix (tailwindExtractor.ts, one rule). The v4 classifier matched only font-family, so --font-sans, --font-mono and the like were dropped. --font-weight-* and companions such as --font-sans--font-feature-settings stay out. None of the 45 existing extractor tests changes: the fixtures use --font-family-* and --font-size-*, which the old rule already classified.
  2. Theme block field (DesignToken.block, optional). The CSS variable extractor sets it to the selector of a [data-theme] block and leaves it unset for :root, so :root tokens, @theme tokens, and every existing caller and test are unchanged. A [data-theme="dark"] value no longer looks like the default.
  3. Color conversion (cssColorToHex, matched in linear time: CodeQL flagged the first version's function pattern, fixed in f269488, and its number pattern had the same ambiguity). Hex, rgb(), hsl() and oklch() become lowercase sRGB hex. oklch() goes OKLab → linear sRGB → gamma; a channel outside sRGB is clamped to [0, 1] and the result says so. An alpha below 1, or any other format (color-mix(), lab(), named colors, transparent), is reported, not approximated.
  4. Serializer (serializeDesignMd in packages/shared, next to the parser because yaml is a dependency there). It writes the fields in the canonical order, then the body sections in the order formatDesignMdForPrompt already uses; #fff stays a quoted string and long values are not folded.
  5. Mapping (mapTokensToDesignFields in core, no new dependency). Colors → colors, font families → typography (fontFamily), radii → rounded, spacing → spacing. Font sizes, line heights and shadows have no frontmatter field, so they come back as Typography and Elevation & Depth sections. A length is accepted in any CSS spelling and written in the form validateDesignMd takes (.5rem → 0.5rem, 1REM → 1rem, .5 → 0.5). Names lose the prefix each extractor adds (colors.brand.500, color-brand-500 and color.brand.500 all become brand-500); --spacing and --radius alone become base.

What the result reports

Nothing is logged (core and shared have no console.*, and the mapper has no logger). The result carries origins (the file and token behind every value, keyed like colors.brand-500) and a report:

  • clamped: path, the original value, and the hex it became;
  • unsupported: alpha, another color format, a unit DESIGN.md does not take (50%, calc(), clamp()), or an untyped token;
  • unresolved: a var() that names no scanned token, names itself, or is nested past the limits (32 levels, 4,096 characters), reason too-complex;
  • otherThemes: values in a [data-theme] block, left out; the default block wins;
  • shadowed: a name already taken by an earlier value; the first is kept. The same value again is ignored.

var()

A var(--name, fallback) is replaced with the scanned token it names, recursively, with a cycle guard. The fallback is used only when no scanned token has that name. It resolves only against tokens the extractors return, not every custom property in the folder, so hsl(var(--hue) 50% 40%) is unresolved when --hue is a bare number the extractor does not classify. The limits exist because the folder can be someone else's repository: a value that names the same token twice at every level takes 2^levels steps to resolve in full.

Known limits

  • :root inside @media (prefers-color-scheme: dark) is read as a default block (the extractor ignores at-rules). The mapper keeps the first value and lists the other under shadowed, so nothing changes silently, but the label is "name already taken", not "dark theme". .dark class blocks are not read at all. Both predate this PR; fixing them means the extractor tracking at-rules, which I would do separately.
  • Non-:root themes are listed, not imported, as agreed in [Feature]: Import a design system from a local codebase into DESIGN.md #484. There is no theme picker.

Type of change

  • Bug fix
  • New feature
  • Refactor (no behavior change)
  • Documentation
  • Build / CI / tooling
  • Breaking change

Linked issue

Refs #484

Checklist

  • I checked the linked issue / relevant context before starting
  • pnpm lint && pnpm typecheck && pnpm test passes locally
  • Added/updated tests for the change
  • Added a changeset (pnpm changeset) if user-visible
  • Updated docs if behavior changed

Principles

  • Compatibility: no existing test expectation changes; block is optional and absent on every token that has no theme variant; the only output that moves is that --font-* tokens now appear.
  • Upgradeability: no pi internals; the mapping and the conversion are pure functions over plain values.
  • No bloat: no new dependencies; the color math is one small module; the serializer reuses the parser's yaml and its section order.
  • Elegance: each commit is one concern; the mapper returns a report instead of logging, so the renderer decides what to show.

Checks

  • pnpm -r typecheck, pnpm lint, CI=1 pnpm test: all passed; core 709 tests, shared 334, desktop 181 files / 2,640 tests.
  • color.test.ts (90): fixed oklch() vectors for in-gamut, out-of-gamut and achromatic input. The expected values were computed on a separate route (the CSS Color 4 reference matrices through XYZ), not the matrices used in color.ts, and the Tailwind v4 palette steps match the hex values Tailwind documents (red-500 #fb2c36, gray-500 #6a7282, blue-50 #eff6ff). Also hue units, percent forms, none, alpha in each notation, values a fraction of a step outside sRGB (reported only when a channel changes by a full step), and 100,000-character inputs that must be rejected, or converted, within 250 ms.
  • designFields.test.ts (49): every type and name style, clamping and its report, each unsupported reason, var() chains, fallbacks, cycles and unresolved names, theme variants, repeated names, the nesting limits (a 40-level chain and a 20-level chain that doubles at each level), the CSS spellings of a length (each one also checked through validateDesignMd), and a Tailwind v4 stylesheet (@theme inline, :root, [data-theme="dark"]) run through the real extractors, mapped, serialized and checked with validateDesignMd.
  • design-md.test.ts (+7): the serialized document validates and parses back to the same fields, a document with every field set, canonical field and section order, hex stays a string, long values are not folded.
  • Each rule fails a test when it is removed or loosened: 24 changes to the mapper, 13 to the color module, 6 to the serializer, 3 to the font classifier and 2 to the block field. One change is deliberately not caught: altering the tenth decimal of an OKLab matrix coefficient, which does not change an 8-bit hex.

The v4 classifier matched only `font-family`, so `--font-sans`,
`--font-mono` and the other `--font-*` theme variables were dropped.
`--font-weight-*` and companions such as `--font-sans--font-feature-settings`
are not families and stay out.

Refs OpenCoworkAI#484
A `:root` value and a `[data-theme="dark"]` value with the same name looked
identical. DesignToken gets an optional `block`, set by the CSS variable
extractor to the selector of a `[data-theme]` block and left unset for
`:root`, so existing tokens and callers are unchanged.

Refs OpenCoworkAI#484
cssColorToHex() handles hex, rgb(), hsl() and oklch(). A color with an alpha
below 1 or in another format is reported, not approximated. oklch() goes
OKLab -> linear sRGB -> gamma; a channel outside sRGB is clamped to [0, 1]
and the result says so.

The oklch test vectors were computed on a separate route (CSS Color 4
reference matrices through XYZ) and include the Tailwind v4 palette steps
whose hex values Tailwind documents, in-gamut, out-of-gamut, achromatic
and values a fraction of a step outside sRGB.

Refs OpenCoworkAI#484
serializeDesignMd() writes the frontmatter fields validateDesignMd accepts
in the canonical order, then the body sections in the canonical section
order (the ordering formatDesignMdForPrompt already uses). Hex colors stay
quoted strings and long values are not folded. It sits next to the parser
because yaml is a dependency of packages/shared, not of packages/core.

Refs OpenCoworkAI#484
mapTokensToDesignFields() turns scanned tokens into the fields
validateDesignMd accepts: colors (sRGB hex), typography (font families),
rounded and spacing. Font sizes, line heights and shadows have no
frontmatter field and come back as Typography and Elevation & Depth
sections. A var() is replaced with the scanned token it names, with a cycle
guard and the CSS fallback.

Nothing is logged. The result carries where every value came from and a
structured report: clamped colors with their original value, values DESIGN.md
cannot hold (alpha, other color formats, other units), unresolved var(),
values left out because they sit in another theme block, and names already
taken by an earlier value.

Refs OpenCoworkAI#484
@github-actions github-actions Bot added docs Documentation area:core packages/core (generation orchestration) labels Oct 14, 2026
Comment thread packages/core/src/brand/color.ts Fixed

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] DIMENSION and UNITLESS reject valid CSS dimensions, so real tokens are dropped — packages/core/src/brand/designFields.ts:44-45. Both patterns require at least one digit before the decimal point and match units case-sensitively, so --radius: .5rem, --space-4: .5, --spacing: -.5rem and --radius: 1REM are classified as unsupported / dimension. That outcome is reported rather than silently changed, so it is not a data-loss bug, but .5 is a valid CSS <number> and CSS units are ASCII case-insensitive, so these are valid — and common in hand-written token files — so they will never import. packages/core/src/brand/color.ts:20 already accepts a leading-dot mantissa for its own numbers, so the two modules disagree on what a number is.
    Suggested fix: accept a leading-dot mantissa and match units case-insensitively in both regexes, then add matching it.each cases to designFields.test.ts:

    const DIMENSION = /^-?(?:\d+(?:\.\d+)?|\.\d+)(?:px|em|rem)$/i;
    const UNITLESS = /^-?(?:\d+(?:\.\d+)?|\.\d+)$/;
  • [Nit] serializeDesignMd() in packages/shared/src/design-md.ts casts every key of ALLOWED_TOP_LEVEL to keyof DesignMdFrontmatter, so the two lists can drift silently: a name added to ALLOWED_TOP_LEVEL but not to the interface compiles, reads undefined, and yaml then omits it. If ALLOWED_TOP_LEVEL is the canonical field set, derive the interface from it or add a type-level subset check so the cast cannot hide a mismatch.

Questions

  • None; the scope matches the PR body (step 2 of 3, Refs #484) and the diff stays inside it.

Summary

  • Review mode: initial
  • No blockers. The change is additive and well insulated: nothing calls mapTokensToDesignFields / serializeDesignMd yet, block is optional and unset on every existing token and caller, and the --font-* classifier change is covered by a new test without disturbing the existing extractor fixtures.
  • Constraints: no new dependencies (yaml was already a packages/shared dependency), no console.*, no provider SDK imports. Returning a report instead of mutating values quietly matches the project rule against silent user-visible fallbacks.
  • Linked issue: Refs #484 is the correct link for a partial slice; the PR does not claim to close the issue, so there is nothing to validate against acceptance criteria.
  • Residual observations: the :root under @media (prefers-color-scheme: dark) and .dark class gaps are pre-existing extractor limits and are disclosed in the PR body; the optional block field on DesignTokenV1 keeps schemaVersion: 1 and is backward compatible, so no migration is required.
  • Testing gap: only the dimension-format cases noted above are uncovered.

Testing

  • Not run (automation). Vitest coverage in the diff is otherwise thorough: color.test.ts vectors cross-checked against an independent conversion route, designFields.test.ts covering each report path plus an end-to-end Tailwind v4 stylesheet re-validated with validateDesignMd, design-md.test.ts for serializer round-trip/ordering, and extractor regression tests.

Open-CoDesign Bot

CodeQL flagged the function pattern in cssColorToHex: the lazy body between
two `\s*` can match whitespace in several ways, so `rgb(` followed by many
tabs backtracks quadratically. The numeric pattern `\d+\.?\d*` has the same
ambiguity on a long run of digits. The input is CSS from a folder the user
picked, which can be someone else's repository.

The body is now matched greedily and split afterwards, and the number
pattern is `\d+(?:\.\d*)?`. Tests run 100,000-character inputs for each case.
A value that names the same token twice at every level takes 2^levels steps
to resolve in full, so a few lines of CSS could stall the scan. A chain is
now followed at most 32 levels deep and a resolved value may reach 4,096
characters; past either limit the token is reported as unresolved with the
reason `too-complex`.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Nit] UnresolvedReason is exported from designFields.ts but left out of both barrels, while its sibling UnsupportedReason is re-exported — packages/core/src/brand/index.ts:3-10 and packages/core/src/index.ts:39-46. A caller that wants to switch on report.unresolved[].reason cannot name the union from the public entry point, and the asymmetry looks accidental.
    Suggested fix: add UnresolvedReason next to UnsupportedReason in both export type { ... } blocks.

Residual observations

  • Carried over from the previous review and still open: DIMENSION / UNITLESS at packages/core/src/brand/designFields.ts:59-60 still require a digit before the decimal point and match units case-sensitively, so --radius: .5rem, --space-4: .5 and --radius: 1REM land in report.unsupported as dimension. This push did not touch it, so it is not re-filed as a new finding, but the previous fix still applies: /^-?(?:\d+(?:\.\d+)?|\.\d+)(?:px|em|rem)$/i plus the matching unitless form, with it.each cases in designFields.test.ts.
  • The previous Nit on serializeDesignMd in packages/shared/src/design-md.ts (casting every ALLOWED_TOP_LEVEL key to keyof DesignMdFrontmatter) is likewise unchanged.
  • The new bounds in this push read correctly: resolveVars is depth-capped via chain.length > MAX_VAR_DEPTH and expansion-capped via out.length > MAX_RESOLVED_LENGTH, the greedy FUNCTION_RE has no nested quantifiers, and the reworked NUMBER alternation stays linear on the long-input cases. The chain is copied per branch, so a repeated (non-cyclic) reference is not misreported as a cycle — consistent with the new MAX_VAR_DEPTH + 8 and doubling-shadow tests. No new issue found in the added lines.

Questions

  • None.

Summary

  • Review mode: follow-up after new commits
  • The delta since 86ccc5b is the ReDoS-hardening commit plus its tests; it changes no in-range behavior and regresses no previously reviewed path. The earlier Minor (dimension regex) and earlier Nit (serializeDesignMd key cast) are simply untouched, not worsened.
  • Constraints: no new dependencies (yaml was already a packages/shared dep), no console.*, no provider SDK imports; the new constants are internal and the mapper stays pure, so the report-instead-of-logging approach still matches the no-silent-fallback rule. Refs #484 remains the correct partial link.
  • Nothing blocking; the PR is mergeable as step 2 with the residual items above.

Testing

  • Not run (automation). Suggested: pnpm --filter @open-codesign/core test and pnpm --filter @open-codesign/shared test locally; the added tests look proportional to the new bounds.

Open-CoDesign Bot

`.5rem`, `-.5rem`, `.5` and `1REM` are valid CSS but were reported as
unsupported dimensions, so hand-written token files never imported. The
mapper now accepts them and writes the form validateDesignMd takes (a digit
before the point, a lowercase unit): `0.5rem`, `-0.5rem`, 0.5, `1rem`.
Widening the pattern alone would have written `.5rem`, which
validateDesignMd rejects. The same length spelled two ways counts as one
value.

Refs OpenCoworkAI#484
…zer from one list

serializeDesignMd cast every allowed field name to keyof DesignMdFrontmatter,
so a name added to one list and not the other compiled and was dropped by
yaml. The fields are now one `as const` list: the allowed set is built from
it and the serializer indexes the frontmatter with it, so a name the type
lacks is a compile error. A test writes a document with every field set.

Refs OpenCoworkAI#484
A caller that switches on report.unresolved[].reason could not name the
union from the public entry point.

Refs OpenCoworkAI#484
@wudilyy999

Copy link
Copy Markdown
Contributor Author

Thanks. All three are fixed: 189c1f1 (lengths), 224c3b0 (the serializer's field list), 241fc55 (UnresolvedReason).

One note on the dimension finding. The suggested patterns alone would make the mapper write .5rem, -.5rem and 1REM as they are, and validateDesignMd rejects all three ("Expected a dimension with px, em, or rem unit"): its own validateDimension needs a digit before the point and a lowercase unit. So the mapper now accepts the CSS spellings and writes the form validateDesignMd takes (0.5rem, -0.5rem, 1rem, and the number 0.5 for unitless spacing). The new tests run each spelling through validateDesignMd, and the same length written two ways counts as one value.

The field names are now one as const list that builds the allowed set and drives the serializer, so a name the DesignMdFrontmatter type lacks is a compile error; a test writes a document with every field set. UnresolvedReason is exported from both entry points.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • No new Blocker, Major, Minor, or Nit findings in this push. The delta since f90c579 is exactly the three items the previous review left open, and all three are now addressed:
    • The UnresolvedReason barrel gap is fixed in both entry points (packages/core/src/brand/index.ts:9, packages/core/src/index.ts:45), so the union is now nameable from the public API alongside UnsupportedReason.
    • DIMENSION / UNITLESS at packages/core/src/brand/designFields.ts:59-60 now accept a leading . and lowercase the unit, with normalizeDimension canonicalizing before claim dedupes; .5rem, -.5rem, 1REM, .25EM, 8PX and bare .5 all map or number-convert instead of landing in report.unsupported, and the two-spelling case (treats the same length spelled two ways as one value) no longer produces a spurious shadowed entry.
    • The serializeDesignMd key cast is gone: FRONTMATTER_FIELDS is now an as const tuple and input.frontmatter[key] indexes it without a keyof assert (packages/shared/src/design-md.ts:26-38, :434-437), while ALLOWED_TOP_LEVEL keeps the same 8 members, so validation behavior is unchanged.

Residual observations (not filed)

  • resolveVars matches the literal var( token (packages/core/src/brand/designFields.ts:126), so uppercase VAR(--x) will not resolve; it surfaces as unresolved/unsupported rather than a silent wrong value, and the same applies to +-signed lengths which DIMENSION/UNITLESS do not accept. Both are rare enough in real stylesheets that I did not raise them as findings.
  • The text-*--line-height companion regex /^text-(.+)--line-height$/ is greedy, but it only derives a display key and any odd capture stays inside the generated section, so there is no data-loss path.

Questions

  • None.

Summary

  • Review mode: follow-up after new commits
  • All previously reported items (the UnresolvedReason export Nit, the dimension-regex Minor, the serializeDesignMd key-cast Nit) are resolved in this push, and the new normalizeDimension path is covered by focused it.each cases plus a validateDesignMd round-trip (packages/core/src/brand/designFields.test.ts:140-210). No behavior outside the newly accepted CSS spellings moved: the earlier extractor, serializer, field-name and var() tests keep their expectations.
  • Constraints still hold: no new dependencies (yaml was already a packages/shared dependency and is reused via stringifyYaml), no console.* in packages/core/packages/shared, no direct provider SDK imports, and the mapper stays pure and returns report/origins instead of logging, so the no-silent-fallback rule is respected. block is added as an optional zod field on DesignTokenV1 (packages/shared/src/design-token.ts:19-23) with a default, so no persisted shape breaks. Refs #484 is still the correct partial link, and the changeset covers core + shared at patch.
  • Nothing blocking; the PR is mergeable as step 2 of #484.

Testing

  • Not run (automation). Suggested: pnpm --filter @open-codesign/core test and pnpm --filter @open-codesign/shared test; the new dimension-normalization and frontmatter-completeness tests are proportional to the change.

Open-CoDesign Bot

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core packages/core (generation orchestration) docs Documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants