Enhance sync-design-systems script with new features by krataratha · Pull Request #213 · nexu-io/open-design

krataratha · 2026-05-01T11:35:05Z

Added command-line flags for dry-run, brand filtering, and manifest generation. Enhanced category mapping and file handling for design systems.

lefarcen · 2026-05-01T11:35:37Z

Hi @krataratha! 🎉
Thanks for the contribution — enhancing the sync-design-systems script with dry-run, filtering, and manifest generation looks like a solid usability boost.
I will run a deep review and get back to you within 24h.

Thanks for making open-design better!
— open-design team

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bcdba95220

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-05-01T11:36:58Z

+
+if (FILTER) {
+  console.log(`Filtered brand: ${FILTER}`);
+}  vercel: 'Developer Tools', warp: 'Developer Tools',


Remove leftover category literal so script can parse

The block after the FILTER log contains stray object-literal entries (vercel: ...) that are no longer inside CATEGORY, so Node throws a syntax error before any logic runs. Invoking the script (for example node --experimental-strip-types scripts/sync-design-systems.ts --dry-run) fails immediately with ERR_INVALID_TYPESCRIPT_SYNTAX, which makes this sync tool unusable in all environments.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-05-01T11:36:58Z

 const SRC = process.argv[2] || '/tmp/package/templates';

-const CATEGORY = {
-  // AI & LLM
-  claude: 'AI & LLM', cohere: 'AI & LLM', elevenlabs: 'AI & LLM',
-  minimax: 'AI & LLM', 'mistral.ai': 'AI & LLM', ollama: 'AI & LLM',
-  'opencode.ai': 'AI & LLM', replicate: 'AI & LLM', runwayml: 'AI & LLM',
-  'together.ai': 'AI & LLM', voltagent: 'AI & LLM', 'x.ai': 'AI & LLM',
-  // Developer Tools
-  cursor: 'Developer Tools', expo: 'Developer Tools', lovable: 'Developer Tools',
-  raycast: 'Developer Tools', superhuman: 'Developer Tools',
-  vercel: 'Developer Tools', warp: 'Developer Tools',
+const args = process.argv.slice(3);


Parse CLI flags without requiring a positional SRC arg

SRC is always read from process.argv[2] while flags are read from process.argv.slice(3), so calling the script with only flags (for example --dry-run or --brand) treats the flag as the source directory and ignores the option. Because the header still documents the source path as optional, this silently breaks the new flag behavior unless callers always pass an explicit path first.

Useful? React with 👍 / 👎.

Copilot

Pull request overview

This PR updates the sync-design-systems script to support additional CLI functionality for controlling sync behavior and outputs, and adjusts how categories and files are handled when generating design-systems/*/DESIGN.md.

Changes:

Added CLI flags for --dry-run, --brand <slug>, and --manifest.
Introduced manifest parsing and output generation (sync-manifest.json) for synced items.
Updated/expanded category mapping and markdown handling (category insertion).

Comments suppressed due to low confidence (1)

scripts/sync-design-systems.ts:183

The file appears to contain a leftover fragment from the previous CATEGORY object / script implementation after the new loop ends (e.g., } vercel: ...). This makes the TypeScript file syntactically invalid and also results in two competing implementations in the same file. Please remove the trailing old block (CATEGORY + main/readManifest/etc.) or refactor so there is only one coherent entrypoint.

console.log(`Synced ${generated.length} design systems`);

if (FILTER) {
  console.log(`Filtered brand: ${FILTER}`);
}  vercel: 'Developer Tools', warp: 'Developer Tools',
  // Backend & Data
  clickhouse: 'Backend & Data', composio: 'Backend & Data',
  hashicorp: 'Backend & Data', mongodb: 'Backend & Data',
  posthog: 'Backend & Data', sanity: 'Backend & Data',

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-05-01T11:37:17Z

+    return data.map((item: any) => ({
+      brand: item.brand || slugify(item.name || 'unknown'),
+      file: item.file || `${item.brand || 'unknown'}.md`,
+      description: item.description || ''
+    }));


In parseTemplates(), the default file is computed from item.brand, not from the derived brand value. If brand is missing but name is present, brand becomes slugify(item.name) but file falls back to unknown.md, causing false “Missing source” skips. Consider computing brand once and using it consistently for the default file name.

Suggested change

return data.map((item: any) => ({

brand: item.brand || slugify(item.name || 'unknown'),

file: item.file || `${item.brand || 'unknown'}.md`,

description: item.description || ''

}));

return data.map((item: any) => {

const brand = item.brand || slugify(item.name || 'unknown');

return {

brand,

file: item.file || `${brand}.md`,

description: item.description || ''

};

});

Copilot · 2026-05-01T11:37:17Z

 const ROOT = path.resolve(__dirname, '..');
 const SRC = process.argv[2] || '/tmp/package/templates';

-const CATEGORY = {
-  // AI & LLM
-  claude: 'AI & LLM', cohere: 'AI & LLM', elevenlabs: 'AI & LLM',
-  minimax: 'AI & LLM', 'mistral.ai': 'AI & LLM', ollama: 'AI & LLM',
-  'opencode.ai': 'AI & LLM', replicate: 'AI & LLM', runwayml: 'AI & LLM',
-  'together.ai': 'AI & LLM', voltagent: 'AI & LLM', 'x.ai': 'AI & LLM',
-  // Developer Tools
-  cursor: 'Developer Tools', expo: 'Developer Tools', lovable: 'Developer Tools',
-  raycast: 'Developer Tools', superhuman: 'Developer Tools',
-  vercel: 'Developer Tools', warp: 'Developer Tools',
+const args = process.argv.slice(3);
+
+const hasFlag = (flag: string) => args.includes(flag);
+
+const getArgValue = (flag: string) => {
+  const i = args.indexOf(flag);
+  return i >= 0 ? args[i + 1] : undefined;
+};
+
+const DRY_RUN = hasFlag('--dry-run');
+const FILTER = getArgValue('--brand');
+const EXPORT_MANIFEST = hasFlag('--manifest');
+


SRC is taken from process.argv[2], so running the script with only flags (e.g. node ... sync-design-systems.ts --dry-run) will incorrectly treat --dry-run as the templates path and none of the flags will be parsed (since args starts at argv[3]). Consider parsing argv so the first non-flag positional arg becomes SRC (or treat argv[2] starting with - as “no SRC provided”).

Copilot · 2026-05-01T11:37:18Z

+    .toLowerCase()
+    .trim()
+    .replace(/\s+/g, '-')
+    .replace(/[^\w.-]/g, '');


slugify() currently preserves dots (.). For brands like mistral.ai, x.ai, opencode.ai, etc., this will generate folder names like design-systems/mistral.ai/…, but the repo’s existing convention uses hyphens (e.g. design-systems/mistral-ai/, x-ai/, opencode-ai/). Consider replacing dots with - during slugification (or reusing the existing slugOf behavior) to avoid creating duplicate/incorrect directories.

Suggested change

.replace(/[^\w.-]/g, '');

.replace(/\./g, '-')

.replace(/[^\w-]/g, '');

lefarcen

Hey @krataratha, thanks for adding CLI flags — the --dry-run and --brand filter are genuinely useful!

However, this PR has several issues that need addressing before merge:

BREAKING: Removed categories — The original script had Automotive (BMW, Tesla, Ferrari, etc.) and several Media brands (Apple, IBM, Nvidia, etc.). This PR deletes them without explanation. Are these brands no longer supported?
BREAKING: Lost manifest description — The original code inserted > ${description} into each DESIGN.md. This PR removes it. That's user-visible content loss.
Silent error handling — parseTemplates() has an empty catch {} block (line 124). If manifest.json is malformed, users see "No templates found" with zero diagnostic info. The original threw explicit errors like "manifest.json must contain an array".
Type safety regression — Uses any in .map((item: any) => ...) (line 120). The original had full runtime validation for each manifest entry.

See inline comments for specifics.

lefarcen · 2026-05-01T11:37:45Z

+  cohere: 'AI & LLM',
+  elevenlabs: 'AI & LLM',
+  minimax: 'AI & LLM',
+  'mistral.ai': 'AI & LLM',


P1 BREAKING: Missing categories. The original script had:

Automotive: bmw, bugatti, ferrari, lamborghini, renault, tesla

Media & Consumer: apple, ibm, nvidia, pinterest, playstation, spacex, theverge, uber, vodafone, wired

E-Commerce: starbucks

This PR removes all of them. If they're intentionally deprecated, document it in the commit message. If not, add them back.

Also: you've duplicated some entries (e.g. stripe: 'Fintech' at line 64, but the original had it under 'Fintech & Crypto'). Merge conflicts or intentional rename?

lefarcen · 2026-05-01T11:37:45Z

+      file: item.file || `${item.brand || 'unknown'}.md`,
+      description: item.description || ''
+    }));
+  } catch {


P2 Type safety: Using any here. The original had explicit runtime checks:

if ( typeof entry === 'object' && entry !== null && 'brand' in entry && typeof entry.brand === 'string' && 'file' in entry && typeof entry.file === 'string' && 'description' in entry && typeof entry.description === 'string' ) { return entry; } throw new Error('manifest.json contains an invalid entry');

Consider restoring that validation — silent coercion with || 'unknown' hides upstream data bugs.

lefarcen · 2026-05-01T11:37:45Z

+    return [];
+  }
+};
+


P2 Error handling: Empty catch block. If manifest.json is missing or malformed (invalid JSON, wrong schema), users see "No templates found." with no hint.

The original printed:

Could not read manifest.json under /tmp/...: <actual error> Did you extract the getdesign tarball? See scripts/sync-design-systems.ts header.

Consider:

catch (err) { console.error(`Failed to parse manifest.json: ${err instanceof Error ? err.message : err}`); console.error('Check that you ran: curl -sL $(npm view getdesign dist.tarball) | tar -xz'); return []; }

lefarcen · 2026-05-01T11:37:45Z

+const parseTemplates = (): ManifestEntry[] => {
+  const manifestPath = path.join(SRC, 'manifest.json');
+
+  try {


P1 BREAKING: Lost description line. The original inserted:

`> Category: ${cat}`, `> ${description}`, // ← this line

Your code only inserts > Category: ${category}. That means every DESIGN.md loses the one-line description from the manifest (e.g. "AI assistant that helps you code"). Is that intentional? If so, document it. If not, restore it:

const insertCategory = (markdown: string, category: string, description: string) => { ... lines.splice(1, 0, '', `> Category: ${category}`, `> ${description}`, ''); ... }

lefarcen · 2026-05-01T11:37:45Z

+  return i >= 0 ? args[i + 1] : undefined;
+};
+
+const DRY_RUN = hasFlag('--dry-run');


P3 Edge case: getArgValue('--brand') doesn't handle --brand=slug syntax (only space-separated). Consider:

const getArgValue = (flag: string) => { const i = args.findIndex(a => a === flag || a.startsWith(`${flag}=`)); if (i < 0) return undefined; const arg = args[i]; return arg.includes('=') ? arg.split('=')[1] : args[i + 1]; };

Not blocking — just a UX nit for CLI users who copy-paste --brand=nike.

lefarcen · 2026-05-01T11:41:05Z

Quick addition to my review above: @copilot spotted a P1 syntax error at line 179 — there's a stray } followed by leftover CATEGORY entries from the old code. This makes the entire file invalid TypeScript.

Author: please remove everything from line 179 (} vercel: ...) through the end of the old CATEGORY block + main() call. The script should end after your new console.log() statements around line 178.

This is on top of the BREAKING issues (missing categories + lost description) I flagged inline.

Enhance sync-design-systems script with new features

bcdba95

Added command-line flags for dry-run, brand filtering, and manifest generation. Enhanced category mapping and file handling for design systems.

Copilot AI review requested due to automatic review settings May 1, 2026 11:35

Copilot started reviewing on behalf of krataratha May 1, 2026 11:35 View session

lefarcen added the enhancement New feature or request label May 1, 2026

chatgpt-codex-connector Bot reviewed May 1, 2026

View reviewed changes

Copilot AI reviewed May 1, 2026

View reviewed changes

lefarcen requested changes May 1, 2026

View reviewed changes

	.replace(/[^\w.-]/g, '');
	.replace(/\./g, '-')
	.replace(/[^\w-]/g, '');

Conversation

krataratha commented May 1, 2026

Uh oh!

lefarcen commented May 1, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 1, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

lefarcen left a comment

Choose a reason for hiding this comment

Uh oh!

lefarcen May 1, 2026

Choose a reason for hiding this comment

Uh oh!

lefarcen May 1, 2026

Choose a reason for hiding this comment

Uh oh!

lefarcen May 1, 2026

Choose a reason for hiding this comment

Uh oh!

lefarcen May 1, 2026

Choose a reason for hiding this comment

Uh oh!

lefarcen May 1, 2026

Choose a reason for hiding this comment

Uh oh!

lefarcen commented May 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants