[wudilyy999]

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, and the length limit never applies when the result stays empty (24 tokens and 1,159 bytes of CSS took 9 seconds, as the review of this PR found). So a name that has expanded is kept for the rest of the mapping and reused by every later reference, and the work follows the size of the input. Only a success is kept: it depends on the name alone, because a cycle needs a loop in the names and a name that expanded reaches none. Each entry also keeps how many levels the name nests, so the depth limit counts them when the name is reused deeper in a chain; a name nothing defines still takes the fallback of each reference.

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 718 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 (58): 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), a fan-out that resolves to an empty value (22 levels, through the real CSS extractor, in under 250 ms; it took about 3.5 seconds before names were kept), the same depth limit when a name is reused in a chain listed either way round, a cycle found after other names were expanded, and a separate fallback for each reference to a name nothing defines, 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. For the name cache, removing it, keeping names nothing defines, not checking the depth on a reuse, and not counting the levels each fail a test. I also compared the mapper with the version before the cache on 600 random variable graphs, 227 of which reach the depth limit: the results are identical.

[github-actions]

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

[github-actions]

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

[wudilyy999]

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]

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

[Sun-sunshine06]

I checked the latest head (241fc55) and ran the core tests (709), shared tests (334), and both packages' typechecks successfully. I reproduced one additional issue that I would like addressed before merging.

[P2] Bound variable-expansion work independently of the output length

In resolveVars, lines 124–131, the depth and output-length limits do not bound the number of recursive calls. A variable that references the preceding variable twice is expanded twice each time. If the leaf resolves to an empty fallback, the output never reaches MAX_RESOLVED_LENGTH, so the work grows exponentially while staying within MAX_VAR_DEPTH.

Reproduction through the real CSS extractor and mapper:

const declarations = ['--shadow-l0: var(--absent,);'];
for (let i = 1; i <= 23; i++) {
  declarations.push(
    `--shadow-l${i}: var(--shadow-l${i - 1})var(--shadow-l${i - 1});`,
  );
}
await writeFile(path, `:root { ${declarations.join('\n')} }`);
const tokens = await extractFromCssVars(path);
const result = mapTokensToDesignFields([{ path, tokens }]);

This is only 24 tokens / 1,159 bytes of CSS. Local measurements were about 65 ms at depth 16, 959 ms at depth 20, and 9,245 ms at depth 23. At depth 23 the resulting section was only 229 characters and report.unresolved was empty. The empty fallback is the key difference from the existing test whose output grows past the length limit. The mapper has no UI caller yet, but this exported synchronous operation would block its caller when connected.

Suggested change:

• Add a shared expansion-step budget covering recursive branches, returning too-complex when exhausted. If the intended bound is per import, do not reset it for each recursive branch or top-level token.
• Alternatively, memoize successfully resolved variables so repeated references reuse their results, while preserving path-sensitive cycle detection and the existing depth/length limits. A work budget alongside caching also gives an explicit upper bound.
• Add regression coverage for the empty-fallback fan-out above, checking that it either resolves efficiently or reports too-complex. Retain the existing cycle, depth, and growing-output tests.

The rest of the mapping/serialization scope looks consistent with step 2 of #484; no UI work is requested here.

[github-actions]

Findings

• No Blocker, Major, Minor, or Nit findings in this push. The delta since 241fc55 is the Expansion.resolved memo layer plus its tests, and I could not construct a case where it changes behavior versus the un-memoized resolver.

Residual observations (not filed)

• Expansion.resolved caches only successes keyed by name, and the depth limit is reconstructed arithmetically as chain.length + 1 + known.below > MAX_VAR_DEPTH (packages/core/src/brand/designFields.ts:142-160). That is sound: below is a property of a name's value (cycles are never cached, so a failed expansion cannot poison later lookups), the cache lookup runs after the chain.includes(name) guard, and a cached value contains no var() left to re-enter a cycle. The new cases in packages/core/src/brand/designFields.test.ts assert order-independence (shallowest-first vs deepest-first), cycle detection after prior expansions, per-reference fallbacks, and both fan-out failure modes. No wrong-value or data-loss path found.
• The previously noted residual limits still stand and are unchanged by this push: resolveVars matches the literal var( token (packages/core/src/brand/designFields.ts:120), so an uppercase VAR(--x) is reported rather than resolved; DIMENSION/UNITLESS (:59-60) do not accept a leading +. Both surface as unresolved/unsupported, never a silent wrong value.

Questions

• None.

Summary

• Review mode: follow-up after new commits
• The new push replaces un-memoized var() expansion with a per-name cache so a fan-out such as --b: var(--a) var(--a) at every level resolves in linear time instead of 2^levels; the below bookkeeping reproduces the old entry check (packages/core/src/brand/designFields.ts:93-166). The added tests are proportional to the risk: linear-time timing, order independence, cycle-after-expansion, per-reference fallbacks, and both fan-out failure modes (too-complex, undefined).
• All items from the previous review remain resolved; nothing new is blocking. Refs #484 is still the correct partial link for step 2, the changeset covers @open-codesign/core and @open-codesign/shared at patch (.changeset/design-tokens-to-design-md.md), and the project constraints hold: no new dependencies (stringifyYaml reuses the existing yaml dep in packages/shared), no console.* in core/shared, and the mapper stays pure, returning origins/report instead of logging.
• 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 fan-out tests assert a 250 ms wall-clock ceiling; the work is microseconds at these sizes so the margin is large, but they are the ones to watch if CI is slow.

Open-CoDesign Bot

[github-actions]

Findings

• No Blocker, Major, Minor, or Nit findings in this push. I re-reviewed the full diff (color conversion, var() resolver + memo, mapper, serializer, extractor changes) and could not construct a wrong-value, data-loss, or silent-fallback path.

Residual observations (not filed)

• resolveVars still matches the literal var( token (packages/core/src/brand/designFields.ts:120), so an uppercase VAR(--x) is reported as unresolved rather than resolved. Non-blocking: it surfaces in report.unresolved, never a silent drop.
• DIMENSION/UNITLESS (packages/core/src/brand/designFields.ts:59-60) do not accept a leading +, so spacing: +0.5rem is reported unsupported. Same reporting behavior, no silent change.
• The fan-out tests in packages/core/src/brand/designFields.test.ts assert a 250 ms wall-clock ceiling via within(...). The margin is large at these sizes (microseconds), but it is the one timing-sensitive assertion to watch on a slow CI runner.
• Not a requirement for this slice, but worth noting the two extractors now diverge on font families: packages/core/src/brand/tailwindExtractor.ts:192 classifies any --font-* (minus weight) as fontFamily, while packages/core/src/brand/cssVarExtractor.ts:12 still only recognizes explicit font-sans|font-mono|font-serif|font-family|typeface. A :root { --font-display: ... } is not picked up as a font family. This predates the PR; flag only if you want the two paths aligned.

Questions

• None.

Summary

• Review mode: follow-up after new commits
• Prior review findings remain resolved; nothing new is blocking. The --font-* classifier fix (tailwindExtractor.ts), the optional DesignToken.block field (design-token.ts:20-24, set only for [data-theme] blocks in cssVarExtractor.ts:73), the linear-time color parser (color.ts), the pure mapper returning origins/report instead of logging (designFields.ts), and serializeDesignMd reusing the existing yaml dep in packages/shared all match the stated intent and the tests are proportional to the risk.
• Constraints hold: no new prod dependencies, no console.* in core/shared, the mapper is pure and reports every value it could not hold (report.unsupported/unresolved/otherThemes/shadowed), and serializeDesignMd emits a document that validateDesignMd accepts. Refs #484 is the correct partial link for step 2 (PR body states IPC/preview/write is step 3), and the changeset covers @open-codesign/core + @open-codesign/shared at patch.
• No release/distribution files changed, so no packaging/checksum path to verify.
• Mergeable as step 2 of #484; the residual items above are pre-existing or cosmetic and need not block.

Testing

• Not run (automation). Suggested: pnpm --filter @open-codesign/core test and pnpm --filter @open-codesign/shared test. If CI flakiness appears, the within(250, ...) fan-out cases are the first to inspect.

Open-CoDesign Bot