Repository navigation
feat(desktop): find design-token sources in a picked folder - #486
Merged
Sun-sunshine06 merged 3 commits intoOct 13, 2026
Merged
Sun-sunshine06 merged 3 commits into
Sun-sunshine06 merged 3 commits into
Conversation
extractFromTailwindConfig, extractFromCssVars and importDtcgJson live in src/brand and are only reachable through deep imports. Re-export them from the package entry by name so the desktop main process can use them through the public entry. They are side-effect-free text parsers. Refs OpenCoworkAI#484
Add scanTokenSources, which walks a folder and runs the brand extractors
over the files that carry design tokens: tailwind.config.{js,cjs,mjs,ts},
CSS with :root, [data-theme] or @theme blocks, and DTCG tokens.json or
*.tokens.json. It returns the tokens per source file, plus the files it
skipped and whether the file cap cut the walk short, so a later preview
can show all three.
The walk shares the ignored-directory list with the existing design
system scan, never follows symlinks, and is bounded by named limits for
depth, file count and file size. Files are read as text and nothing in
the folder is executed. Nothing calls the scan yet.
Refs OpenCoworkAI#484
github-actions
Bot
added
area:desktop
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Findings
- [Minor] A single structurally-invalid token file can abort the whole scan —
readTokensonly translates aSyntaxError(fromJSON.parse) into a skippableinvalid-jsonresult, while every other throw is re-thrown (apps/desktop/src/main/design-system-tokens.ts:99-106).importDtcgJson(...)runs inside the sametry, so if it throws for valid JSON that is not DTCG-shaped (e.g. a plaintokens.jsonfrom another tool), or ifextractFromTailwindConfig/extractFromCssVarsthrow on an unusualtailwind.config.*/ CSS file,scanTokenSourcesrejects and the whole folder import fails over one unrelated file. This is the exact case the scan is meant to tolerate: it already reportsinvalid-jsonandtoo-largeas data rather than failing.
Suggested fix / next action: either broaden the recoverable branch to cover extractor/shape failures (add askippedreason such asinvalid-tokens) so a bad file only excludes itself, or confirm and document thatimportDtcgJsonand the extractors only throw for genuine I/O errors. Add a test for (a) valid JSON with a non-DTCG shape and (b) a malformedtailwind.config.js, since the current tests only cover'{ not json'(apps/desktop/src/main/design-system-tokens.test.ts).
Questions
- Can
importDtcgJsonthrow anything other thanSyntaxErrorfor in-size files? Theerror instanceof SyntaxErrornarrowing implies a deliberate choice to re-throw everything else, but the intent isn't obvious from the diff.
Summary
- Review mode: initial
- The re-export and the scan are small, focused, and consistent with the project constraints I could check in the public repo: no new dependencies, no direct provider SDK import, all LLM/parsing work stays in the existing
packages/coreextractors,IGNORED_DIRSis only widened fromconsttoexport const(apps/desktop/src/main/design-system.ts:9), and nothing referencesscanTokenSourcesyet, so there is no startup-path cost. Symlink and size limits are enforced and tested. - Highest-severity item is the Minor above. No Blocker or Major issues found.
- Issue validation for #484 could not be performed in this run (no issue fetch available); the PR uses
Refs #484and describes itself as PR 1 of 4, which is the correct link style for a partial slice, so no closure claim is being over-asserted. - Residual risk:
skippedcan list any over-cap.cssfile (every.cssis a candidate viasourceKind, and the size check runs before token detection), so a large token-less stylesheet may appear as a skipped token source in the PR 3 preview. Worth a look when the preview surfaces this list, but not a merge concern here. - Deferring the changeset to PR 3 is reasonable as long as a changeset covering the new
@open-codesign/coreexports is added before the next version/release.
Testing
- Not run (automation). The added
design-system-tokens.test.tscovers ordering, both CSS block forms, ignored dirs, depth/file/size caps, invalid JSON, text-only config parsing, and symlink non-following. Gap: no coverage for non-SyntaxErrorfailures from the DTCG importer or the extractors (see finding above).
Open-CoDesign Bot
Pin what the token scan does with a file that is not what its name says: valid JSON that is not shaped like DTCG tokens, and a Tailwind config or CSS file with unbalanced braces. The extractors return no tokens for all of them, so the scan lists no source and does not fail. Say in the code why only malformed JSON marks a file as skipped: read errors have to surface, and importDtcgJson accepts any parsed value. Refs OpenCoworkAI#484
Contributor
There was a problem hiding this comment.
Findings
No new findings. The single Minor from the previous review is addressed:
- The prior concern was that
readTokensnarrows recovery toSyntaxError, so a token file that makesimportDtcgJsonthrow for another reason would reject the whole scan (apps/desktop/src/main/design-system-tokens.ts:99-108). The intent is now documented atapps/desktop/src/main/design-system-tokens.ts:100-101:importDtcgJsonaccepts any parsed value, so only malformed JSON is treated as skippable, and read errors propagate deliberately. - New tests exercise exactly the cases the prior review asked for: valid JSON that is not DTCG-shaped (
[1, 2, 3],null, apackage.json-shaped object) and a malformedtailwind.config.jsplus unbalanced CSS both return{ sources: [], skipped: [], truncated: false }(apps/desktop/src/main/design-system-tokens.test.ts:126-147). These assertions would fail ifimportDtcgJsonor the extractors threw for those inputs, so the tests validate the documented behavior rather than merely restating it.
Questions
None. The prior open question — whether importDtcgJson can throw anything other than SyntaxError for in-size files — is answered by the added comment and the new non-DTCG-shape tests.
Summary
- Review mode: follow-up after new commits
- Scope is unchanged from the prior review: re-export three
packages/corebrand extractors by name (noexport *) and add an unusedscanTokenSourceswalker. No new dependencies, no direct provider SDK imports, all parsing stays in the existingpackages/coreextractors, andIGNORED_DIRSis only widened fromconsttoexport const(apps/desktop/src/main/design-system.ts:9). Nothing referencesscanTokenSources, so there is no startup-path cost and the lazy-load constraint is respected. - The diff since the previously reviewed commit is test-only plus a two-line comment; no behavioral change beyond the recoverable branch now being backed by tests.
- Residual risk (unchanged, non-blocking):
readTokensstill re-throws any non-SyntaxErrorfromextractFromTailwindConfig/extractFromCssVarson the config/CSS paths (apps/desktop/src/main/design-system-tokens.ts:98,:111-112), so an unexpected extractor throw would abort the scan for the whole folder. There is no evidence the extractors throw on in-size text today; if the PR 3 preview ever surfaces such a case, add a dedicatedskippedreason (e.g.invalid-tokens) instead of failing the import. - Residual risk (unchanged, non-blocking):
skippedcan list any over-cap.cssfile, because every.cssis a candidate and the size check runs before token detection, so a large token-less stylesheet can appear as a skipped token source. Worth re-checking when PR 3 renders the list. - Linked issue: the PR uses
Refs #484and describes itself as PR 1 of 4, which is the correct link style for a partial slice; there is no closure claim to validate. The issue #484 body was not available in this run, so I could not re-check its acceptance criteria against this slice. - Deferring the changeset to PR 3 is reasonable as long as the new
@open-codesign/coreexport addition is covered before the next version bump.
Testing
- Not run (automation). The added
design-system-tokens.test.tscovers ordering, both CSS block forms, ignored dirs, depth/file/size caps, invalid JSON, non-DTCG-shaped JSON, malformed config/CSS, text-only config parsing, and symlink non-following. No Playwright coverage is expected for an unused main-process helper.
Open-CoDesign Bot
1 task done
5 tasks
9 of 15 tasks
Sun-sunshine06
pushed a commit
that referenced
this pull request
Oct 20, 2026
## 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 #484. There is no theme picker. ## Type of change - [ ] Bug fix - [x] New feature - [ ] Refactor (no behavior change) - [ ] Documentation - [ ] Build / CI / tooling - [ ] Breaking change ## Linked issue Refs #484 ## Checklist - [x] I checked the linked issue / relevant context before starting - [x] `pnpm lint && pnpm typecheck && pnpm test` passes locally - [x] Added/updated tests for the change - [x] Added a changeset (`pnpm changeset`) if user-visible - [ ] Updated docs if behavior changed ## Principles - [x] 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. - [x] Upgradeability: no pi internals; the mapping and the conversion are pure functions over plain values. - [x] No bloat: no new dependencies; the color math is one small module; the serializer reuses the parser's `yaml` and its section order. - [x] 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First slice of #484: re-export the three brand token extractors from the core entry, and add
scanTokenSources, which finds the files in a folder that carry design tokens and runs the extractors over them. Nothing calls the scan yet. PR 3 of the series wires it to the import action and loads it withawait import(...)at the IPC handler, so it stays off the startup path (CLAUDE.md constraint 5). The two changes are separate commits so the re-export and the scan can be reviewed apart.packages/core/src/index.ts):extractFromTailwindConfig,extractFromCssVarsandimportDtcgJsonby name, noexport *. They are side-effect-free text parsers, and the scan is not reachable from core's entry.apps/desktop/src/main/design-system-tokens.ts): returns the tokens per source file fortailwind.config.{js,cjs,mjs,ts}, CSS containing:root,[data-theme]or@theme, andtokens.json/*.tokens.json. It also reports the files it skipped (over the size cap, invalid JSON) and whether the file cap cut the walk short, as data for the preview and not as log lines.What the scan does and does not do:
node_modules,.git, build output and the rest of the existing ignore list (IGNORED_DIRS, now exported fromdesign-system.ts), and never follows symlinks.DEFAULT_TOKEN_SCAN_LIMITS): depth 6, 10,000 files looked at, 256,000 bytes per file.The existing "Link design system repo" scan (
scanDesignSystem) is unchanged, and the two write to different stores. The existing scan keeps an app-scoped list of candidate values in the app config. The import this series builds writes<workspace>/DESIGN.md, per design, and will be its own action with its own IPC channel (PR 3); it does not read or replace the snapshot.One difference from the plan on #484: the walk is new and the existing one is not reused. The existing walk stops after 160 candidate files in directory order and keeps only a prompt summary, so on a larger repo it can stop before it reaches the token files, and it has no tests. This scan shares its ignore list (
IGNORED_DIRS) and has its own limits, named constants that tests override. They are separate from the snapshot's caps, so the two scans do not share a budget.Run on real folders: open-cowork (3 sources, 39 tokens) and this repo (6 sources, 190 tokens), about 12 ms each, neither truncated.
Type of change
Linked issue
Refs #484 (PR 1 of 4)
Checklist
pnpm lint && pnpm typecheck && pnpm testpasses locallypnpm changeset) if user-visibleNothing is user-visible until the scan is wired in, so the changeset comes with that PR. The docs page is PR 4.
Principles
exportonIGNORED_DIRS.Checks
pnpm -r typecheck,pnpm lint,pnpm test: 10 tasks passed; core 560 tests, shared 326, desktop 181 files / 2,640 tests.design-system-tokens.test.ts(11 tests): a Tailwind config,:rootCSS,@themeCSS and DTCG JSON in one folder, in a stable order;@themeand:rootin one file; ignored directories; depth limit; file cap with the truncation flag; size cap; invalid JSON; valid JSON that is not DTCG-shaped; a malformed config and CSS; a config that is parsed and not run; symlinks to a directory and to a file are not followed.