From d401dd27d8320f023f3d7ad665c220087956a259 Mon Sep 17 00:00:00 2001 From: MaxLee-dev Date: Mon, 7 Sep 2026 11:25:57 +0900 Subject: [PATCH 1/9] test(icon-extractor): check SVG-to-JSX output with vitest The SVGR + svgo config decides how every icon rasterizes, and nothing checked it. Six cases pin the parts that silently change rendering: mono icons follow the consumer's colour, colour icons keep Figma's palette, the root `fill="none"` that keeps strokes hollow survives, and mask ids are prefixed per icon so two icons on one page cannot collide. Verified by mutation: dropping the `blackFollowsCurrentColor` svgo plugin turns the first case red. --- pnpm-lock.yaml | 3 + scripts/icon-extractor/package.json | 10 ++- .../tests/svgr-transformer.test.ts | 81 +++++++++++++++++++ scripts/icon-extractor/tsconfig.json | 2 +- scripts/icon-extractor/vitest.config.ts | 14 ++++ 5 files changed, 105 insertions(+), 5 deletions(-) create mode 100644 scripts/icon-extractor/tests/svgr-transformer.test.ts create mode 100644 scripts/icon-extractor/vitest.config.ts diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 07ade649f..41df7a920 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -979,6 +979,9 @@ importers: typescript: specifier: 'catalog:' version: 5.9.3 + vitest: + specifier: 'catalog:' + version: 3.2.7(@types/debug@4.1.12)(@types/node@22.20.1)(@vitest/browser@3.2.7)(happy-dom@20.11.2)(jiti@2.7.0)(jsdom@29.0.2(@noble/hashes@1.8.0))(lightningcss@1.33.0)(sass@1.102.0)(tsx@4.23.13)(yaml@2.9.0) scripts/ts-api-extractor: dependencies: diff --git a/scripts/icon-extractor/package.json b/scripts/icon-extractor/package.json index 924b60431..b325e11cf 100644 --- a/scripts/icon-extractor/package.json +++ b/scripts/icon-extractor/package.json @@ -6,12 +6,13 @@ "type": "module", "scripts": { "extract": "tsx --env-file-if-exists=.env ./src/cli.ts", - "format": "prettier --write \"./src/**/*.{ts,md}\"", - "format:check": "prettier --check \"./src/**/*.{ts,md}\"", - "lint": "eslint ./src", + "format": "prettier --write \"./{src,tests}/**/*.{ts,md}\"", + "format:check": "prettier --check \"./{src,tests}/**/*.{ts,md}\"", + "lint": "eslint ./src ./tests", "notify:slack": "tsx ./src/slack/notify.ts", "sync-icons:basic": "pnpm extract --type=basic", "sync-icons:symbol": "pnpm extract --type=symbol", + "test": "vitest --run", "typecheck": "tsc --noEmit" }, "dependencies": { @@ -30,6 +31,7 @@ "@types/node": "^22.20.1", "eslint": "catalog:", "tsx": "^4.23.13", - "typescript": "catalog:" + "typescript": "catalog:", + "vitest": "catalog:" } } diff --git a/scripts/icon-extractor/tests/svgr-transformer.test.ts b/scripts/icon-extractor/tests/svgr-transformer.test.ts new file mode 100644 index 000000000..ce18a81d5 --- /dev/null +++ b/scripts/icon-extractor/tests/svgr-transformer.test.ts @@ -0,0 +1,81 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; + +import { svgToIconComponent } from '~/transformer/svgr-transformer'; + +/** Shaped like Figma's export: root fill="none", black children, document-global mask id. */ +const MONO_SVG = ` + + + + + + +`; + +const COLOR_SVG = ` + + + +`; + +const convert = (svg: string, iconName: string, isColorIcon = false) => + svgToIconComponent({ svg, iconName, isColorIcon }); + +test('mono icons follow the consumer colour', async () => { + const out = await convert(MONO_SVG, 'SampleIcon'); + + assert.match(out, /fill="currentColor"/, 'black fill must become currentColor'); + assert.match(out, /stroke="currentColor"/, 'black stroke must become currentColor'); + assert.doesNotMatch(out, /"(black|#000|#000000)"/i, 'no literal black may survive'); +}); + +test('the root keeps fill="none" so stroked shapes stay hollow', async () => { + const out = await convert(MONO_SVG, 'SampleIcon'); + + // The attribute order is SVGR's, so match the root element rather than a fixed string. + const root = /]*)>/.exec(out); + assert.ok(root, 'output must wrap the SVG in IconBase'); + assert.match(root[1], /fill="none"/, 'dropping this fills every stroke-only icon solid'); +}); + +test('colour icons keep the Figma palette', async () => { + const out = await convert(COLOR_SVG, 'SampleColorIcon', true); + + assert.match(out, /fill="#D22730"/i); + // `black` is a real palette value here, not a placeholder for currentColor. + assert.doesNotMatch(out, /currentColor/); +}); + +test('ids are namespaced per icon so two icons on one page cannot collide', async () => { + const first = await convert(MONO_SVG, 'FirstIcon'); + const second = await convert(MONO_SVG, 'SecondIcon'); + + const idOf = (out: string) => /id="([^"]+)"/.exec(out)?.[1]; + assert.match(String(idOf(first)), /^vapor-icons-mono-FirstIcon/); + assert.match(String(idOf(second)), /^vapor-icons-mono-SecondIcon/); + assert.notEqual(idOf(first), idOf(second)); + // The reference has to move with the definition. + assert.match(first, new RegExp(`url\\(#${idOf(first)}\\)`)); + + const colour = await convert(COLOR_SVG, 'FirstIcon', true); + assert.doesNotMatch(colour, /vapor-icons-mono-/); +}); + +test('IconBase owns the size: viewBox stays, width/height go', async () => { + const out = await convert(MONO_SVG, 'SampleIcon'); + + assert.match(out, /viewBox="0 0 16 16"/, 'without viewBox the icon cannot scale'); + assert.doesNotMatch(out, /width="16"/); + assert.doesNotMatch(out, /height="16"/); +}); + +test('the component is an IconBase wrapper that forwards props', async () => { + const out = await convert(MONO_SVG, 'SampleIcon'); + + assert.match(out, /import IconBase from '~\/components\/icon-base';/); + assert.match(out, /const SampleIcon = \(props: IconProps\)/); + assert.match(out, /\{\.\.\.props\}/); + assert.match(out, /export default SampleIcon;/); + assert.doesNotMatch(out, / must be replaced, not nested'); +}); diff --git a/scripts/icon-extractor/tsconfig.json b/scripts/icon-extractor/tsconfig.json index 398afac72..4b689baba 100644 --- a/scripts/icon-extractor/tsconfig.json +++ b/scripts/icon-extractor/tsconfig.json @@ -11,5 +11,5 @@ "lib": ["ES2023"], "types": ["node"] }, - "include": ["src", "icon-extractor.config.json"] + "include": ["src", "tests", "icon-extractor.config.json"] } diff --git a/scripts/icon-extractor/vitest.config.ts b/scripts/icon-extractor/vitest.config.ts new file mode 100644 index 000000000..1983db0be --- /dev/null +++ b/scripts/icon-extractor/vitest.config.ts @@ -0,0 +1,14 @@ +import path from 'node:path'; +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + resolve: { + alias: { + '~': path.resolve(__dirname, './src'), + }, + }, + test: { + environment: 'node', + include: ['tests/**/*.test.ts'], + }, +}); From 76ba8da7446d4fb5cbf34a5e7485e0cc70ebbe5f Mon Sep 17 00:00:00 2001 From: MaxLee-dev Date: Mon, 7 Sep 2026 11:29:06 +0900 Subject: [PATCH 2/9] feat(icon-extractor): add the Figma visual parity check Nothing checked that a synced icon actually draws the same as its Figma source. `parity:fetch` pulls Figma's own PNG per icon at scale 4, `parity:render` draws our component at the same size in Chromium, and `parity:compare` counts differing pixels with pixelmatch, failing above the gate. Both sides rasterize the vector directly, so no resampling enters the measurement. Only monochrome icons are gated. Figma and Chromium antialias the colour icons differently enough (worst 164 px) that no real signal survives, so those are reported and left ungated. compare.ts writes report.json, report.md and a self-contained report.html with the Figma / code / diff PNGs of each failure inlined. The canvas size, includeAA, threshold and gate were measured across all 594 mono icons; lib.ts records what each measurement was and why the value cannot move without redoing it. Current state: 594 mono, worst 2, none failing. --- .gitignore | 3 + .vscode/settings.json | 3 + pnpm-lock.yaml | 45 ++++ scripts/icon-extractor/package.json | 11 + scripts/icon-extractor/src/parity/.gitignore | 1 + scripts/icon-extractor/src/parity/compare.ts | 200 ++++++++++++++++++ .../src/parity/fetch-baseline.ts | 125 +++++++++++ scripts/icon-extractor/src/parity/lib.ts | 69 ++++++ scripts/icon-extractor/src/parity/render.ts | 86 ++++++++ 9 files changed, 543 insertions(+) create mode 100644 scripts/icon-extractor/src/parity/.gitignore create mode 100644 scripts/icon-extractor/src/parity/compare.ts create mode 100644 scripts/icon-extractor/src/parity/fetch-baseline.ts create mode 100644 scripts/icon-extractor/src/parity/lib.ts create mode 100644 scripts/icon-extractor/src/parity/render.ts diff --git a/.gitignore b/.gitignore index 3c6a09028..a4eecc5d6 100644 --- a/.gitignore +++ b/.gitignore @@ -58,3 +58,6 @@ docs/plans/ # Worktrees .worktrees + +# Icon parity check cache (baselines, renders, diffs) +scripts/icon-extractor/src/parity/.cache/ diff --git a/.vscode/settings.json b/.vscode/settings.json index de387b770..79d50b9f7 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -9,6 +9,9 @@ "source.fixAll.eslint": "explicit" }, "js/ts.tsdk.path": "node_modules/typescript/lib", + "files.watcherExclude": { + "**/scripts/icon-extractor/src/parity/.cache/**": true + }, "json.schemas": [ { "url": "https://cdn.jsdelivr.net/npm/tsup/schema.json", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 41df7a920..9e3ee1189 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -970,9 +970,33 @@ importers: '@types/node': specifier: ^22.20.1 version: 22.20.1 + '@types/pngjs': + specifier: ^6.0.5 + version: 6.0.5 + '@types/react': + specifier: 'catalog:' + version: 19.2.18 + '@types/react-dom': + specifier: 'catalog:' + version: 19.2.5(@types/react@19.2.18) eslint: specifier: 'catalog:' version: 9.39.5(jiti@2.7.0) + pixelmatch: + specifier: ^7.2.0 + version: 7.2.0 + playwright: + specifier: ^1.62.1 + version: 1.62.1 + pngjs: + specifier: ^7.0.0 + version: 7.0.0 + react: + specifier: 'catalog:' + version: 19.2.8 + react-dom: + specifier: 'catalog:' + version: 19.2.8(react@19.2.8) tsx: specifier: ^4.23.13 version: 4.23.13 @@ -4685,6 +4709,9 @@ packages: '@types/node@25.5.0': resolution: {integrity: sha512-jp2P3tQMSxWugkCUKLRPVUpGaL5MVFwF8RDuSRztfwgN1wmqJeMSbKlnEtQqU8UrhTmzEmZdu2I6v2dpp7XIxw==} + '@types/pngjs@6.0.5': + resolution: {integrity: sha512-0k5eKfrA83JOZPppLtS2C7OUtyNAl2wKNxfyYl9Q5g9lPkgBl/9hNyAu6HuEH2J4XmIv2znEpkDd0SaZVxW6iQ==} + '@types/prismjs@1.26.5': resolution: {integrity: sha512-AUZTa7hQ2KY5L7AmtSiqxlhWxb4ina0yd8hNbl4TWuqnv/pFP0nDMb3YrfSBf4hJVGLh2YEIBfKaBW/9UEl6IQ==} @@ -8304,6 +8331,10 @@ packages: resolution: {integrity: sha512-TfySrs/5nm8fQJDcBDuUng3VOUKsd7S+zqvbOTiGXHfxX4wK31ard+hoNuvkicM/2YFzlpDgABOevKSsB4G/FA==} engines: {node: '>= 6'} + pixelmatch@7.2.0: + resolution: {integrity: sha512-xhcb4yHu9sM/G7foGzoLtXYcC0zHEaOXXjRKhGup0fw78Nf2Tkiapv4EQyMzrbcmQPsllAI7DbFY2UT7PlI9Pg==} + hasBin: true + pkg-dir@3.0.0: resolution: {integrity: sha512-/E57AYkoeQ25qkxMj5PBOVgF8Kiu/h7cYS30Z5+R7WaiCCBfLq58ZI/dSeaEKb9WVJV5n/03QwrN3IeWIFllvw==} engines: {node: '>=6'} @@ -8325,6 +8356,10 @@ packages: engines: {node: '>=20'} hasBin: true + pngjs@7.0.0: + resolution: {integrity: sha512-LKWqWJRhstyYo9pGvgor/ivk2w94eSjE3RGVuzLGlr3NmD8bf7RcYGze1mNdEHRP6TRP6rMuDHk5t44hnTRyow==} + engines: {node: '>=14.19.0'} + portfinder@1.0.38: resolution: {integrity: sha512-rEwq/ZHlJIKw++XtLAO8PPuOQA/zaPJOZJ37BVuN97nLpMJeuDVLVGRwbFoBgLudgdTMP2hdRJP++H+8QOA3vg==} engines: {node: '>= 10.12'} @@ -14145,6 +14180,10 @@ snapshots: undici-types: 7.18.2 optional: true + '@types/pngjs@6.0.5': + dependencies: + '@types/node': 22.20.1 + '@types/prismjs@1.26.5': {} '@types/react-dom@19.2.5(@types/react@19.2.18)': @@ -18778,6 +18817,10 @@ snapshots: pirates@4.0.7: {} + pixelmatch@7.2.0: + dependencies: + pngjs: 7.0.0 + pkg-dir@3.0.0: dependencies: find-up: 3.0.0 @@ -18800,6 +18843,8 @@ snapshots: optionalDependencies: fsevents: 2.3.2 + pngjs@7.0.0: {} + portfinder@1.0.38: dependencies: async: 3.2.6 diff --git a/scripts/icon-extractor/package.json b/scripts/icon-extractor/package.json index b325e11cf..60cbc29e9 100644 --- a/scripts/icon-extractor/package.json +++ b/scripts/icon-extractor/package.json @@ -10,6 +10,9 @@ "format:check": "prettier --check \"./{src,tests}/**/*.{ts,md}\"", "lint": "eslint ./src ./tests", "notify:slack": "tsx ./src/slack/notify.ts", + "parity:compare": "tsx ./src/parity/compare.ts", + "parity:fetch": "tsx --env-file-if-exists=.env ./src/parity/fetch-baseline.ts", + "parity:render": "tsx ./src/parity/render.ts", "sync-icons:basic": "pnpm extract --type=basic", "sync-icons:symbol": "pnpm extract --type=symbol", "test": "vitest --run", @@ -29,7 +32,15 @@ "@repo/typescript-config": "workspace:*", "@types/lodash-es": "^4.17.12", "@types/node": "^22.20.1", + "@types/pngjs": "^6.0.5", + "@types/react": "catalog:", + "@types/react-dom": "catalog:", "eslint": "catalog:", + "pixelmatch": "^7.2.0", + "playwright": "^1.62.1", + "pngjs": "^7.0.0", + "react": "catalog:", + "react-dom": "catalog:", "tsx": "^4.23.13", "typescript": "catalog:", "vitest": "catalog:" diff --git a/scripts/icon-extractor/src/parity/.gitignore b/scripts/icon-extractor/src/parity/.gitignore new file mode 100644 index 000000000..ceddaa37f --- /dev/null +++ b/scripts/icon-extractor/src/parity/.gitignore @@ -0,0 +1 @@ +.cache/ diff --git a/scripts/icon-extractor/src/parity/compare.ts b/scripts/icon-extractor/src/parity/compare.ts new file mode 100644 index 000000000..d65941cc7 --- /dev/null +++ b/scripts/icon-extractor/src/parity/compare.ts @@ -0,0 +1,200 @@ +import fs from 'node:fs/promises'; +import path from 'node:path'; +import process from 'node:process'; +import pc from 'picocolors'; +import pixelmatch from 'pixelmatch'; +import { PNG } from 'pngjs'; + +import type { Manifest } from './lib'; +import { CACHE_DIR, DIFF_GATE, MANIFEST, PIXELMATCH_OPTIONS, flags, onlyFilter } from './lib'; + +type Row = { + name: string; + diffPixels: number; + isColorIcon: boolean; + width: number; + height: number; + failed: boolean; +}; + +const args = flags(); +const threshold = Number(args.threshold ?? DIFF_GATE); +// Colour icons are rasterizer noise, not signal — gate them only to exercise the failure path. +const gateColor = args['gate-color'] === 'true'; +const keep = onlyFilter(args.only); + +const baselineDir = path.join(CACHE_DIR, 'baseline'); +const codeDir = path.join(CACHE_DIR, 'render'); +const diffDir = path.join(CACHE_DIR, 'diff'); +await fs.mkdir(diffDir, { recursive: true }); + +const manifest: Manifest = JSON.parse(await fs.readFile(MANIFEST, 'utf8').catch(() => '{}')); + +// The expected name list comes from the *baseline* (what Figma actually has), never from the +// render output: enumerating our renders let a missing one vanish silently as +// `813 compared, skipped: []`. +const names = (await fs.readdir(baselineDir)) + .filter((f) => f.endsWith('.png')) + .map((f) => f.slice(0, -4)) + .filter(keep) + .sort(); +if (!names.length) { + throw new Error( + `no baseline in ${baselineDir}${args.only ? ` matching --only=${args.only}` : ''} — run parity:fetch first`, + ); +} + +const rows: Row[] = []; +const skipped: string[] = []; +const missing: string[] = []; +const unclassified: string[] = []; + +for (const name of names) { + const codeFile = path.join(codeDir, `${name}.png`); + if (!(await fs.stat(codeFile).catch(() => null))) { + missing.push(`${name} (no code render)`); + continue; + } + const entry = manifest[name]; + if (!entry) { + unclassified.push(name); + continue; + } + + const figma = PNG.sync.read(await fs.readFile(path.join(baselineDir, `${name}.png`))); + const code = PNG.sync.read(await fs.readFile(codeFile)); + if (figma.width !== code.width || figma.height !== code.height) { + skipped.push(`${name} (${figma.width}x${figma.height} vs ${code.width}x${code.height})`); + continue; + } + + const a = new Uint8Array(figma.data); + const b = new Uint8Array(code.data); + const diff = new PNG({ width: figma.width, height: figma.height }); + const diffPixels = pixelmatch(a, b, diff.data, figma.width, figma.height, { + ...PIXELMATCH_OPTIONS, + diffMask: true, + }); + + await fs.writeFile(path.join(diffDir, `${name}.png`), PNG.sync.write(diff)); + + rows.push({ + name, + diffPixels, + isColorIcon: entry.isColorIcon, + width: figma.width, + height: figma.height, + failed: (gateColor || !entry.isColorIcon) && diffPixels > threshold, + }); +} + +rows.sort((x, y) => y.diffPixels - x.diffPixels); +const failures = rows.filter((row) => row.failed); +const mono = rows.filter((row) => !row.isColorIcon); +const color = rows.filter((row) => row.isColorIcon); +const report = { + threshold, + pixelmatch: PIXELMATCH_OPTIONS, + total: rows.length, + expected: names.length, + failed: failures.length, + skipped, + missing, + unclassified, + rows, +}; +await fs.writeFile(path.join(CACHE_DIR, 'report.json'), JSON.stringify(report, null, 2)); + +const worst = (group: Row[]) => Math.max(0, ...group.map((row) => row.diffPixels)); +const lines = [ + `# Icon parity report`, + '', + `Our render vs Figma's own PNG, diff measured in pixelmatch pixels.`, + '', + `- gated: **${mono.length} mono** icons, fail above ${threshold} diff pixels — ` + + `${failures.length} failing, worst ${worst(mono)}`, + `- ungated: ${color.length} colour icons (Figma and Chromium disagree on these), ` + + `worst ${worst(color)}`, + ...(skipped.length ? [`- ${skipped.length} skipped (size mismatch)`] : []), + ...(missing.length ? [`- **${missing.length} MISSING** renders`] : []), + ...(unclassified.length ? [`- **${unclassified.length} not in manifest**`] : []), + '', + `| Icon | kind | diff px | size |`, + `| --- | --- | ---: | --- |`, + ...rows + .slice(0, 60) + .map( + (row) => + `| ${row.name}${row.failed ? ' **FAIL**' : ''} | ` + + `${row.isColorIcon ? 'colour' : 'mono'} | ${row.diffPixels} | ` + + `${row.width}x${row.height} |`, + ), +]; +for (const [label, list] of [ + ['skipped', skipped], + ['missing', missing], + ['unclassified', unclassified], +] as const) { + if (list.length) lines.push('', `### ${label}`, ...list.map((item) => `- ${item}`)); +} +await fs.writeFile(path.join(CACHE_DIR, 'report.md'), lines.join('\n') + '\n'); + +// Self-contained HTML (images inlined) so a CI artifact opens with no extra files. +// Only failing rows carry images (an explicit --only shows every requested icon); the plain +// lists below cover the other failure causes. +const dataUri = async (file: string) => + `data:image/png;base64,${(await fs.readFile(file)).toString('base64')}`; +const cell = async (dir: string, name: string) => + ``; +const htmlRows = await Promise.all( + (args.only ? rows : failures).map( + async (row) => + `${row.name}
${row.diffPixels} px` + + (await cell(baselineDir, row.name)) + + (await cell(codeDir, row.name)) + + (await cell(diffDir, row.name)) + + '', + ), +); +const htmlList = (label: string, list: string[]) => + list.length + ? `

${label} (${list.length})

` + : ''; +await fs.writeFile( + path.join(CACHE_DIR, 'report.html'), + `Icon parity report + +

Icon parity report

+

${failures.length} of ${mono.length} mono icons over ${threshold} diff pixels.

+${htmlRows.join('')}
FigmaCodeDiff
+${htmlList('missing renders', missing)}${htmlList('skipped (size mismatch)', skipped)}${htmlList('not in manifest', unclassified)}`, +); + +console.log( + `${rows.length}/${names.length} compared — mono ${mono.length} (worst ${worst(mono)}), ` + + `colour ${color.length} (worst ${worst(color)}, ungated), ` + + `${pc.red(String(failures.length))} over ${threshold}` + + (skipped.length ? `, ${skipped.length} skipped` : '') + + (missing.length ? `, ${pc.red(`${missing.length} missing`)}` : ''), +); +console.log(`report: ${path.join(CACHE_DIR, 'report.md')}`); +console.log(`diffs: ${diffDir} (${rows.length} PNGs)`); +if (failures.length) { + console.error( + pc.red( + `mono icons over ${threshold} diff pixels:\n ` + + failures.map((row) => `${row.name} (${row.diffPixels})`).join('\n '), + ), + ); +} +if (missing.length) console.error(pc.red(`missing renders:\n ${missing.join('\n ')}`)); +if (unclassified.length) { + console.error(pc.red(`not in manifest (re-run parity:fetch):\n ${unclassified.join('\n ')}`)); +} +if (failures.length || missing.length || skipped.length || unclassified.length) { + process.exitCode = 1; +} diff --git a/scripts/icon-extractor/src/parity/fetch-baseline.ts b/scripts/icon-extractor/src/parity/fetch-baseline.ts new file mode 100644 index 000000000..a57e38021 --- /dev/null +++ b/scripts/icon-extractor/src/parity/fetch-baseline.ts @@ -0,0 +1,125 @@ +/** + * Step 1 — download Figma's own raster for every icon. + * + * Usage: + * tsx src/parity/fetch-baseline.ts [--type=basic|symbol] [--only=Name,Name] [--limit=N] [--scale=N] + * + * The baseline is a PNG on purpose: this check asks whether our components render the way FIGMA + * renders them, so the reference has to come out of Figma's rasterizer. Comparing against Figma's + * SVG export instead would only re-test svgo, which tests/svgr-transformer.test.ts covers + * offline and for free. + * + * Already-downloaded icons are skipped, so changing `--scale` means clearing `.cache/baseline` + * first — otherwise the previous scale's files are reused and the run silently mixes sizes. + */ +import fs from 'node:fs/promises'; +import path from 'node:path'; +import process from 'node:process'; +import pLimit from 'p-limit'; +import pc from 'picocolors'; + +import { getImage } from '~/api/figma-client'; +import { ICON_TYPE_NAMES, colorFrameIds, figma, iconTypes, isIconType } from '~/config'; +import { fetchIconNodes } from '~/downloader/svg-downloader'; +import { normalizeIconName } from '~/utils/icon-name'; + +import type { Manifest } from './lib'; +import { CACHE_DIR, MANIFEST, flags, onlyFilter } from './lib'; + +// Figma's images endpoint has no documented node-count cap, but percent-encoding the `:` in node +// ids pushed a 814-node query past 8KB and returned HTTP 414 (measured). Batch to stay under it. +const BATCH_SIZE = 200; + +const args = flags(); +const type = args.type ?? 'basic'; +const limit = args.limit ? Number(args.limit) : Infinity; +const keep = onlyFilter(args.only); +// 4 is Figma's documented maximum, so 64px is all the resolution this check can get. +const scale = args.scale ? Number(args.scale) : 4; + +if (!process.env.FIGMA_TOKEN) { + console.error(pc.red('FIGMA_TOKEN is not set.')); + process.exit(1); +} +if (!isIconType(type)) { + console.error(pc.red(`--type must be one of ${ICON_TYPE_NAMES.join(', ')}`)); + process.exit(1); +} +// Figma's documented range. Out of range comes back as HTTP 400 with no per-node detail. +if (!Number.isFinite(scale) || scale < 0.01 || scale > 4) { + console.error(pc.red(`--scale must be between 0.01 and 4 (got ${args.scale})`)); + process.exit(1); +} + +const outDir = path.join(CACHE_DIR, 'baseline'); +await fs.mkdir(outDir, { recursive: true }); + +const iconType = iconTypes[type]; +// Colour-ness is the parent frame, not the icon name — same rule as the sync itself. +const colorFrames = colorFrameIds(iconType); +const components = await fetchIconNodes({ + fileKey: figma.fileKey, + frameIds: iconType.frames.map((frame) => frame.id), +}); +components.sort((a, b) => normalizeIconName(a.name).localeCompare(normalizeIconName(b.name))); + +const wanted = components + .filter((node) => keep(normalizeIconName(node.name))) + .slice(0, limit) + .map((node) => ({ + id: node.id, + name: normalizeIconName(node.name), + isColorIcon: colorFrames.has(node.parentId), + })); +if (!wanted.length) { + console.error(pc.red(`no ${type} icon matched --only=${args.only}`)); + process.exit(1); +} + +// compare.ts only gates mono icons, so it needs Figma's own answer to "is this a colour icon". +// Rewritten every run, merged across --type runs so a basic run does not drop the symbol flags. +const manifest: Manifest = JSON.parse(await fs.readFile(MANIFEST, 'utf8').catch(() => '{}')); +for (const icon of wanted) manifest[icon.name] = { id: icon.id, isColorIcon: icon.isColorIcon }; +await fs.writeFile(MANIFEST, JSON.stringify(manifest, null, 2)); + +// Already-downloaded icons are skipped so re-runs are cheap. +const missing: typeof wanted = []; +for (const icon of wanted) { + const file = path.join(outDir, `${icon.name}.png`); + if (!(await fs.stat(file).catch(() => null))) missing.push(icon); +} +console.log(`${wanted.length} icons, ${missing.length} to download (scale ${scale})`); + +const urls = new Map(); +for (let i = 0; i < missing.length; i += BATCH_SIZE) { + const batch = missing.slice(i, i + BATCH_SIZE); + const { images } = await getImage({ + fileKey: figma.fileKey, + nodeIds: batch.map((icon) => icon.id), + format: 'png', + // Figma renders PNGs at the node bbox; scale=4 on a 16px icon gives 64px. + scale, + }); + for (const icon of batch) { + // HTTP 200 does not mean every node rendered — nulls are per-node failures. + const url = images[icon.id]; + if (!url) console.warn(pc.yellow(`render failed: ${icon.name}`)); + else urls.set(icon.name, url); + } +} + +const concurrency = pLimit(10); +await Promise.all( + [...urls].map(([name, url]) => + concurrency(async () => { + const response = await fetch(url); + if (!response.ok) throw new Error(`${name}: ${response.status} ${response.statusText}`); + await fs.writeFile( + path.join(outDir, `${name}.png`), + Buffer.from(await response.arrayBuffer()), + ); + }), + ), +); + +console.log(pc.green(`baseline ready: ${outDir}`)); diff --git a/scripts/icon-extractor/src/parity/lib.ts b/scripts/icon-extractor/src/parity/lib.ts new file mode 100644 index 000000000..6ddaf8c09 --- /dev/null +++ b/scripts/icon-extractor/src/parity/lib.ts @@ -0,0 +1,69 @@ +/** + * Shared settings for the icon parity check: fetch a Figma PNG per icon, render our component + * to a PNG the same size in Chromium, and count differing pixels with pixelmatch. + * + * The four values below (canvas size, includeAA, threshold, gate) were measured across all 594 + * mono icons, not guessed. Each carries the measurement that fixed it — read those before + * changing one, because the failure they guard against is silent: a gate that is too loose + * passes a shifted icon without a word. + */ +import path from 'node:path'; +import process from 'node:process'; +import { fileURLToPath } from 'node:url'; + +const PARITY_DIR = path.dirname(fileURLToPath(import.meta.url)); +/** Override to hold a cache fetched at another `--scale` without clobbering the main one. */ +const CACHE_DIR = process.env.PARITY_CACHE_DIR ?? path.join(PARITY_DIR, '.cache'); + +/** Written by fetch-baseline, read by compare — which icons are colour, straight from Figma. */ +const MANIFEST = path.join(CACHE_DIR, 'manifest.json'); + +type Manifest = Record; + +/** + * `includeAA: true` reads backwards: `false` is pixelmatch's default and turns its anti-aliasing + * detector ON. A thin line moved by 1px changes only edge pixels, which that detector always + * classifies as anti-aliasing and drops — 253 of 594 mono icons could move a full pixel and + * still score ≤1. `true` skips the detector and counts every pixel over the threshold. + * + * `threshold: 0.3` is an alpha difference of 79 for these icons (mono icons are pure black, so + * pixelmatch's YIQ distance collapses to the alpha delta). 99.8% of rasterizer-noise pixels fall + * below it and 62% of the pixels a 1px shift moves fall above it. Lower and noise leaks in; + * higher (0.4) and the smallest real shifts score 2, colliding with noise. + */ +const PIXELMATCH_OPTIONS = { threshold: 0.3, includeAA: true } as const; + +/** + * Fail above this many differing pixels. Measured 2026-09-07 over all 594 mono icons: a correct + * icon scores at most 2 (DividerOutlineIcon), and the smallest defect worth catching — the source + * shifted 0.25 units, i.e. 1px on the 64px canvas — scores at least 4 (MinusOutlineIcon). The + * gate sits at the noise ceiling rather than halfway between: a silent miss costs more than a + * false alarm you can see in the PR. + * + * 64×64 is the only canvas where those two ranges separate. At 16px a 0.25-unit shift flips no + * pixel at all, and at 32px the best setting leaves signal min 1 against noise max 1. It is also + * Figma's maximum `scale`, and both sides rasterize the vector directly — no resampling. + * + * Re-measure if Chromium, Playwright, the icon bundle or Figma's renderer changes. + */ +const DIFF_GATE = 2; + +/** `--flag=value` / `--flag` parsing. No dependency needed. */ +function flags(): Record { + const out: Record = {}; + for (const arg of process.argv.slice(2)) { + const m = /^--([^=]+)(?:=(.*))?$/.exec(arg); + if (m) out[m[1]] = m[2] ?? 'true'; + } + return out; +} + +/** `--only=A,B` keeps just those icon names; without the flag everything passes. */ +function onlyFilter(only: string | undefined): (name: string) => boolean { + if (!only) return () => true; + const wanted = new Set(only.split(',')); + return (name) => wanted.has(name); +} + +export type { Manifest }; +export { CACHE_DIR, DIFF_GATE, MANIFEST, PARITY_DIR, PIXELMATCH_OPTIONS, flags, onlyFilter }; diff --git a/scripts/icon-extractor/src/parity/render.ts b/scripts/icon-extractor/src/parity/render.ts new file mode 100644 index 000000000..3cd231379 --- /dev/null +++ b/scripts/icon-extractor/src/parity/render.ts @@ -0,0 +1,86 @@ +import type { ComponentType } from 'react'; +import { createElement } from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; + +import fs from 'node:fs/promises'; +import path from 'node:path'; +import pc from 'picocolors'; +import type { Browser, Page } from 'playwright'; +import { chromium } from 'playwright'; +import { PNG } from 'pngjs'; + +import { REPO_ROOT } from '~/utils/file-system'; + +import { CACHE_DIR, flags, onlyFilter } from './lib'; + +const ICON_BUNDLE = path.join(REPO_ROOT, 'packages/icons/dist/index.js'); + +const DEFAULT_COLOR = '#000'; + +type RenderProps = { + width: number; + height: number; + style: { color: string; transform?: string }; +}; + +async function rasterize(page: Page, svg: string, width: number, height: number): Promise { + await page.setViewportSize({ width, height }); + await page.setContent( + `${svg}`, + ); + return page.screenshot({ omitBackground: true }); +} + +async function main() { + const args = flags(); + const color = args.color ?? DEFAULT_COLOR; + const keep = onlyFilter(args.only); + + const baselineDir = path.join(CACHE_DIR, 'baseline'); + const codeDir = path.join(CACHE_DIR, 'render'); + await fs.mkdir(codeDir, { recursive: true }); + + const icons = (await import(ICON_BUNDLE)) as Record>; + const names = (await fs.readdir(baselineDir)) + .filter((file) => file.endsWith('.png')) + .map((file) => file.slice(0, -4)) + .filter(keep) + .sort(); + + const browser: Browser = await chromium.launch(); + const page = await browser.newPage(); + let rendered = 0; + const missing: string[] = []; + + for (const name of names) { + const Icon = icons[name]; + if (typeof Icon !== 'function') { + missing.push(name); + continue; + } + + const baseline = PNG.sync.read(await fs.readFile(path.join(baselineDir, `${name}.png`))); + const markup = renderToStaticMarkup( + createElement(Icon, { + width: baseline.width, + height: baseline.height, + style: { color }, + }), + ); + await fs.writeFile( + path.join(codeDir, `${name}.png`), + await rasterize(page, markup, baseline.width, baseline.height), + ); + rendered++; + } + + await browser.close(); + if (missing.length) { + console.warn( + pc.yellow(`no component exported for ${missing.length}: ${missing.join(', ')}`), + ); + } + console.log(pc.green(`rendered ${rendered} icons -> ${codeDir}`)); +} + +await main(); From 1621c655ad5995f4f6ecbc5b43d5c0dc354295c6 Mon Sep 17 00:00:00 2001 From: MaxLee-dev Date: Mon, 7 Sep 2026 11:31:28 +0900 Subject: [PATCH 3/9] ci(icons): gate icon changes on visual parity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parity check had no CI entry point, so a change to the SVGR or svgo config could alter how every icon rasterizes and nothing would notice. `icon-parity.yml` owns the check: it builds the bundle, fetches Figma's PNGs, renders in Chromium, compares, uploads report.* as an artifact on failure and leaves a sticky comment on the PR. Three entry points share that one job so the gate value and the report can never drift apart: - pull_request, on the paths that can change how an icon draws - workflow_call, from the Figma sync - workflow_dispatch, to exercise the check on demand The sync workflow cannot rely on the pull_request trigger instead: it opens and pushes its PR with GITHUB_TOKEN and GitHub raises no workflow events for those. It therefore calls the reusable workflow and passes the PR number, which required exposing `has_changes` and `pr_number` as job outputs. Slack notification moves to its own job so a parity failure reaches it too. A dispatch from a branch other than main no longer closes the open sync PR — it compared Figma against that branch, so "no changes" says nothing about main. That run now reports parity onto the open PR instead. Fork PRs are skipped: without FIGMA_TOKEN there is no baseline to compare against. --- .github/workflows/icon-parity.yml | 111 +++++++++++++++++++++++++ .github/workflows/sync-figma-icons.yml | 47 ++++++++++- 2 files changed, 154 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/icon-parity.yml diff --git a/.github/workflows/icon-parity.yml b/.github/workflows/icon-parity.yml new file mode 100644 index 000000000..ab007ebaf --- /dev/null +++ b/.github/workflows/icon-parity.yml @@ -0,0 +1,111 @@ +name: '[CI] Icon Parity' + +# Renders every icon component in Chromium and compares it to Figma's own PNG. +# Gate values (64px canvas, pixelmatch threshold 0.3 / includeAA, fail above 2 diff px) +# are measured, not guessed — scripts/icon-extractor/src/parity/CALIBRATION.md. +# +# Two entry points share this one job so the gate and the report live in one place: +# - pull_request: any change that can alter how an icon rasterizes +# - workflow_call: the weekly Figma sync, on the PR it just opened + +on: + workflow_dispatch: + workflow_call: + inputs: + pr-number: + description: 'PR to comment the report on. Empty means report to the log only.' + type: string + required: false + pull_request: + paths: + - packages/icons/src/** + - scripts/icon-extractor/src/parity/** + - scripts/icon-extractor/src/transformer/** + - scripts/icon-extractor/src/config.ts + - scripts/icon-extractor/icon-extractor.config.json + - .github/workflows/icon-parity.yml + +permissions: + contents: read + pull-requests: write + +concurrency: + group: icon-parity-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + parity: + name: Icon Parity + runs-on: ubuntu-latest + # Forks get no FIGMA_TOKEN, so there is no baseline to compare against. + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository + env: + CACHE: scripts/icon-extractor/src/parity/.cache + steps: + - name: Checkout branch + uses: actions/checkout@v4 + + - name: Install + uses: ./.github/composite/install + + - name: Build icons + run: pnpm --filter @vapor-ui/icons build + + - name: Install Chromium + run: pnpm --filter @repo/icon-extractor exec playwright install --with-deps chromium + + # Colour icons are fetched and reported but not gated: Figma and Chromium antialias + # them differently enough (worst 164 px) to drown any real signal. + - name: Compare against Figma + id: compare + continue-on-error: true + env: + FIGMA_TOKEN: ${{ secrets.FIGMA_TOKEN }} + run: | + pnpm --filter @repo/icon-extractor parity:fetch --type=basic + pnpm --filter @repo/icon-extractor parity:fetch --type=symbol + pnpm --filter @repo/icon-extractor parity:render + pnpm --filter @repo/icon-extractor parity:compare + + # report.html inlines the Figma / code / diff PNG of every failing icon, so the + # artifact is one file that opens in a browser with nothing else attached. + - name: Upload report + id: report + if: steps.compare.outcome == 'failure' + uses: actions/upload-artifact@v4 + with: + name: icon-parity-report + path: ${{ env.CACHE }}/report.* + # .cache is hidden, and upload-artifact excludes hidden paths since v4.4. + include-hidden-files: true + if-no-files-found: error + + - name: Comment on PR + if: steps.compare.outcome == 'failure' && (inputs.pr-number || github.event.pull_request.number) + uses: marocchino/sticky-pull-request-comment@v2 + with: + number: ${{ inputs.pr-number || github.event.pull_request.number }} + header: icon-parity + message: | + ## ⚠️ 아이콘 시각 검증 실패 + + 아이콘이 Figma 렌더링과 다릅니다. 판정 기준은 `scripts/icon-extractor/src/parity/CALIBRATION.md`에 있습니다. + + 🖼️ **[시각 diff 보기](${{ steps.report.outputs.artifact-url }})** — 내려받아 `report.html`을 열면 실패한 아이콘마다 Figma / 코드 / diff가 나란히 보입니다. + + [워크플로 실행 보기](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) + + - name: Report summary + if: always() + run: | + # The full table is in the artifact; the summary carries the headline counts. + # GitHub caps a summary at 1 MB and the report lists at most 60 rows. + if [[ -f "$CACHE/report.md" ]]; then + head -c 60000 "$CACHE/report.md" >> "$GITHUB_STEP_SUMMARY" + else + echo 'No parity report — the comparison never ran.' >> "$GITHUB_STEP_SUMMARY" + fi + + - name: Fail if parity failed + if: steps.compare.outcome == 'failure' + run: exit 1 diff --git a/.github/workflows/sync-figma-icons.yml b/.github/workflows/sync-figma-icons.yml index 9828c8dc1..379b10bc8 100644 --- a/.github/workflows/sync-figma-icons.yml +++ b/.github/workflows/sync-figma-icons.yml @@ -18,6 +18,9 @@ env: jobs: sync-figma-icons: runs-on: ubuntu-latest + outputs: + has_changes: ${{ steps.commit.outputs.has_changes }} + pr_number: ${{ steps.create_pr.outputs.number }} steps: - name: Checkout branch uses: actions/checkout@v4 @@ -60,10 +63,13 @@ jobs: echo "deleted_icons=$DELETED_SYMBOL" >> $GITHUB_OUTPUT - name: Create branch and commit changes + id: commit run: | # Check if there are changes in the icons directory if [[ -n $(git status --porcelain packages/icons/src/) ]]; then + # GITHUB_ENV for the later steps in this job, GITHUB_OUTPUT for the parity job. echo "has_changes=true" >> $GITHUB_ENV + echo "has_changes=true" >> $GITHUB_OUTPUT git config --local user.email "action@github.com" git config --local user.name "GitHub Action" @@ -254,15 +260,48 @@ jobs: # No changes means main already matches Figma, so an open PR left over # from an earlier run is stale and must not be merged. STALE_PR=$(gh pr list --head "$BRANCH_NAME" --base main --state open --json number --jq '.[0].number // empty') - if [[ -n "$STALE_PR" ]]; then + # Only main can judge staleness — a dispatch from another branch compared + # Figma against that branch, so "no changes" says nothing about the sync PR. + if [[ -n "$STALE_PR" && "${{ github.ref_name }}" == "main" ]]; then gh pr close "$STALE_PR" --comment "Closing: main already matches Figma, so this sync is no longer valid." + elif [[ -n "$STALE_PR" ]]; then + # Feature-branch dispatch: let the parity report land on the open sync PR. + echo "number=$STALE_PR" >> $GITHUB_OUTPUT fi fi + # The gate, the report and the PR comment all live in icon-parity.yml so this entry point + # and the pull_request one can never drift apart. + # + # This call cannot be dropped in favour of that pull_request trigger: the sync PR is opened + # and pushed with GITHUB_TOKEN, and GitHub raises no workflow events for those, so the sync + # PR would otherwise never be checked. For the same reason there is no double run here. + # + # Manual dispatch always runs it, so the check can be exercised without a Figma diff. + parity: + name: Parity + needs: sync-figma-icons + if: needs.sync-figma-icons.outputs.has_changes == 'true' || github.event_name == 'workflow_dispatch' + uses: ./.github/workflows/icon-parity.yml + secrets: inherit + with: + pr-number: ${{ needs.sync-figma-icons.outputs.pr_number }} + + notify: + name: Notify Slack + needs: [sync-figma-icons, parity] + # A skipped parity job is not a failure, so this only fires on a real one. + if: failure() + runs-on: ubuntu-latest + steps: + - name: Checkout branch + uses: actions/checkout@v4 + + - name: Install + uses: ./.github/composite/install + - name: Notify Slack on Failure - if: failure() - run: | - WORKFLOW_STATUS=failure pnpm --filter @repo/icon-extractor notify:slack + run: WORKFLOW_STATUS=failure pnpm --filter @repo/icon-extractor notify:slack env: SLACK_GDS_ALARM_WEBHOOK_URL: ${{ secrets.SLACK_GDS_ALARM_WEBHOOK_URL }} GITHUB_REPOSITORY: ${{ github.repository }} From e862dc9113b03c190c6f0a3e819a3f8eec190821 Mon Sep 17 00:00:00 2001 From: MaxLee-dev Date: Wed, 9 Sep 2026 09:19:08 +0900 Subject: [PATCH 4/9] ci(icons): host the parity report on S3 instead of an artifact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - an artifact link means download-and-unzip before anyone sees a diff; one HTML object on the bucket website opens straight from the PR comment - comment now posts on pass too, with a counts table read from report.json rather than a re-grepped markdown report — so report.md is gone - S3 upload needs OIDC `id-token`, which a called workflow cannot exceed, so the sync-icons caller has to request it as well --- .github/workflows/icon-parity.yml | 78 +++++++++++++------- .github/workflows/sync-figma-icons.yml | 9 +++ scripts/icon-extractor/src/parity/compare.ts | 50 +++---------- 3 files changed, 73 insertions(+), 64 deletions(-) diff --git a/.github/workflows/icon-parity.yml b/.github/workflows/icon-parity.yml index ab007ebaf..3be30c87f 100644 --- a/.github/workflows/icon-parity.yml +++ b/.github/workflows/icon-parity.yml @@ -25,9 +25,12 @@ on: - scripts/icon-extractor/icon-extractor.config.json - .github/workflows/icon-parity.yml +# This block replaces the default permissions rather than adding to them, so `contents` and +# `pull-requests` have to stay listed alongside the OIDC `id-token` the S3 upload needs. permissions: contents: read pull-requests: write + id-token: write concurrency: group: icon-parity-${{ github.event.pull_request.number || github.ref }} @@ -67,45 +70,68 @@ jobs: pnpm --filter @repo/icon-extractor parity:render pnpm --filter @repo/icon-extractor parity:compare - # report.html inlines the Figma / code / diff PNG of every failing icon, so the - # artifact is one file that opens in a browser with nothing else attached. - - name: Upload report - id: report - if: steps.compare.outcome == 'failure' - uses: actions/upload-artifact@v4 + # compare.ts already computed these, so read them back instead of re-deriving them + # from the rendered report. + - name: Read counts + id: counts + if: always() + run: | + if [[ ! -f "$CACHE/report.json" ]]; then + echo 'ran=false' >> "$GITHUB_OUTPUT" + echo 'No parity report — the comparison never ran.' >> "$GITHUB_STEP_SUMMARY" + exit 0 + fi + { + echo 'ran=true' + jq -r '"total=\(.total)", + "failed=\(.failed)", + "threshold=\(.threshold)", + "mono=\(.mono.count)", + "mono_worst=\(.mono.worst)", + "colour=\(.colour.count)", + "colour_worst=\(.colour.worst)"' "$CACHE/report.json" + } >> "$GITHUB_OUTPUT" + + - name: Configure AWS credentials + if: steps.counts.outputs.ran == 'true' + uses: aws-actions/configure-aws-credentials@v4 with: - name: icon-parity-report - path: ${{ env.CACHE }}/report.* - # .cache is hidden, and upload-artifact excludes hidden paths since v4.4. - include-hidden-files: true - if-no-files-found: error + role-to-assume: ${{ secrets.AWS_IAM_ROLE_NAME }} + role-session-name: ${{ secrets.AWS_IAM_ROLE_SESSION_NAME }} + aws-region: ${{ secrets.AWS_REGION }} + + # report.html inlines the Figma / code / diff PNG of every failing icon, so one object + # is the whole report. `index.html` is the bucket website's default document. + - name: Upload report to S3 + id: s3 + if: steps.counts.outputs.ran == 'true' + run: | + BRANCH=$(echo "${{ github.head_ref || github.ref_name }}" | tr '/' '-') + KEY="icon-parity/$BRANCH/index.html" + aws s3 cp "$CACHE/report.html" "s3://${{ secrets.BUCKET_NAME }}/$KEY" + URL="http://${{ secrets.BUCKET_NAME }}/$KEY" + echo "url=$URL" >> "$GITHUB_OUTPUT" + # A dispatch run has no PR to comment on, so the link has to land somewhere. + echo "[Icon parity report]($URL)" >> "$GITHUB_STEP_SUMMARY" - name: Comment on PR - if: steps.compare.outcome == 'failure' && (inputs.pr-number || github.event.pull_request.number) + if: steps.counts.outputs.ran == 'true' && (inputs.pr-number || github.event.pull_request.number) uses: marocchino/sticky-pull-request-comment@v2 with: number: ${{ inputs.pr-number || github.event.pull_request.number }} header: icon-parity message: | - ## ⚠️ 아이콘 시각 검증 실패 + ${{ steps.compare.outcome == 'failure' && '🚫 **아이콘 시각 검증 실패**' || '✅ **아이콘 시각 검증 통과**' }} - 아이콘이 Figma 렌더링과 다릅니다. 판정 기준은 `scripts/icon-extractor/src/parity/CALIBRATION.md`에 있습니다. + | 전체 | 게이트 대상 (mono) | 실패 | mono 최대 diff | 리포트 | + | :--- | :--- | :--- | :--- | :--- | + | ${{ steps.counts.outputs.total }} | ${{ steps.counts.outputs.mono }} | ${{ steps.counts.outputs.failed }} | ${{ steps.counts.outputs.mono_worst }} px | [열기 ↗︎](${{ steps.s3.outputs.url }}) | - 🖼️ **[시각 diff 보기](${{ steps.report.outputs.artifact-url }})** — 내려받아 `report.html`을 열면 실패한 아이콘마다 Figma / 코드 / diff가 나란히 보입니다. + 실패한 아이콘마다 Figma / 코드 / diff가 나란히 보입니다. 컬러 아이콘 ${{ steps.counts.outputs.colour }}개는 게이트 밖입니다 (최대 ${{ steps.counts.outputs.colour_worst }} px). + 판정 기준 `> ${{ steps.counts.outputs.threshold }} diff px`의 근거는 `scripts/icon-extractor/src/parity/lib.ts` 주석에 있습니다. [워크플로 실행 보기](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) - - name: Report summary - if: always() - run: | - # The full table is in the artifact; the summary carries the headline counts. - # GitHub caps a summary at 1 MB and the report lists at most 60 rows. - if [[ -f "$CACHE/report.md" ]]; then - head -c 60000 "$CACHE/report.md" >> "$GITHUB_STEP_SUMMARY" - else - echo 'No parity report — the comparison never ran.' >> "$GITHUB_STEP_SUMMARY" - fi - - name: Fail if parity failed if: steps.compare.outcome == 'failure' run: exit 1 diff --git a/.github/workflows/sync-figma-icons.yml b/.github/workflows/sync-figma-icons.yml index 379b10bc8..7c0db51b3 100644 --- a/.github/workflows/sync-figma-icons.yml +++ b/.github/workflows/sync-figma-icons.yml @@ -6,6 +6,15 @@ on: # Run every Thursday at 4 PM KST (07:00 UTC) - cron: '0 7 * * 4' +# The called parity workflow uploads to S3 over OIDC, and a called workflow cannot be granted +# more than its caller holds — `id-token` is never in the default token, so it has to be asked +# for here. Opening this block replaces the defaults, so the write scopes this job's own +# commit / PR steps rely on have to be listed too. +permissions: + contents: write + pull-requests: write + id-token: write + concurrency: # Fixed group, not keyed on github.ref: every run targets the same BRANCH_NAME # regardless of the ref it was dispatched from, so runs must serialize. diff --git a/scripts/icon-extractor/src/parity/compare.ts b/scripts/icon-extractor/src/parity/compare.ts index d65941cc7..146cdb7f1 100644 --- a/scripts/icon-extractor/src/parity/compare.ts +++ b/scripts/icon-extractor/src/parity/compare.ts @@ -92,12 +92,19 @@ rows.sort((x, y) => y.diffPixels - x.diffPixels); const failures = rows.filter((row) => row.failed); const mono = rows.filter((row) => !row.isColorIcon); const color = rows.filter((row) => row.isColorIcon); +const worst = (group: Row[]) => Math.max(0, ...group.map((row) => row.diffPixels)); + +// The headline counts live here, not only in the rendered page: the workflow reads them straight +// out of this file for the PR comment. Re-parsing a rendered report to recover numbers we already +// have is how the Playwright job ended up grepping its own markdown. const report = { threshold, pixelmatch: PIXELMATCH_OPTIONS, total: rows.length, expected: names.length, failed: failures.length, + mono: { count: mono.length, worst: worst(mono) }, + colour: { count: color.length, worst: worst(color) }, skipped, missing, unclassified, @@ -105,42 +112,8 @@ const report = { }; await fs.writeFile(path.join(CACHE_DIR, 'report.json'), JSON.stringify(report, null, 2)); -const worst = (group: Row[]) => Math.max(0, ...group.map((row) => row.diffPixels)); -const lines = [ - `# Icon parity report`, - '', - `Our render vs Figma's own PNG, diff measured in pixelmatch pixels.`, - '', - `- gated: **${mono.length} mono** icons, fail above ${threshold} diff pixels — ` + - `${failures.length} failing, worst ${worst(mono)}`, - `- ungated: ${color.length} colour icons (Figma and Chromium disagree on these), ` + - `worst ${worst(color)}`, - ...(skipped.length ? [`- ${skipped.length} skipped (size mismatch)`] : []), - ...(missing.length ? [`- **${missing.length} MISSING** renders`] : []), - ...(unclassified.length ? [`- **${unclassified.length} not in manifest**`] : []), - '', - `| Icon | kind | diff px | size |`, - `| --- | --- | ---: | --- |`, - ...rows - .slice(0, 60) - .map( - (row) => - `| ${row.name}${row.failed ? ' **FAIL**' : ''} | ` + - `${row.isColorIcon ? 'colour' : 'mono'} | ${row.diffPixels} | ` + - `${row.width}x${row.height} |`, - ), -]; -for (const [label, list] of [ - ['skipped', skipped], - ['missing', missing], - ['unclassified', unclassified], -] as const) { - if (list.length) lines.push('', `### ${label}`, ...list.map((item) => `- ${item}`)); -} -await fs.writeFile(path.join(CACHE_DIR, 'report.md'), lines.join('\n') + '\n'); - -// Self-contained HTML (images inlined) so a CI artifact opens with no extra files. -// Only failing rows carry images (an explicit --only shows every requested icon); the plain +// Self-contained HTML (images inlined) so the one file the workflow uploads to S3 opens on its +// own. Only failing rows carry images (an explicit --only shows every requested icon); the plain // lists below cover the other failure causes. const dataUri = async (file: string) => `data:image/png;base64,${(await fs.readFile(file)).toString('base64')}`; @@ -169,7 +142,8 @@ td{padding:.5rem;background:repeating-conic-gradient(#eee 0 25%,#fff 0 50%) 0 0/ img{display:block;width:128px;height:128px;image-rendering:pixelated}

Icon parity report

-

${failures.length} of ${mono.length} mono icons over ${threshold} diff pixels.

+

${failures.length} of ${mono.length} mono icons over ${threshold} diff pixels (worst ${worst(mono)}).
+${color.length} colour icons are reported but not gated — Figma and Chromium disagree on these (worst ${worst(color)}).

${htmlRows.join('')}
FigmaCodeDiff
${htmlList('missing renders', missing)}${htmlList('skipped (size mismatch)', skipped)}${htmlList('not in manifest', unclassified)}`, ); @@ -181,7 +155,7 @@ console.log( (skipped.length ? `, ${skipped.length} skipped` : '') + (missing.length ? `, ${pc.red(`${missing.length} missing`)}` : ''), ); -console.log(`report: ${path.join(CACHE_DIR, 'report.md')}`); +console.log(`report: ${path.join(CACHE_DIR, 'report.html')}`); console.log(`diffs: ${diffDir} (${rows.length} PNGs)`); if (failures.length) { console.error( From c2c0ceb53ed0fb0ef1583c9cd7d05506cf1b946a Mon Sep 17 00:00:00 2001 From: MaxLee-dev Date: Wed, 9 Sep 2026 09:41:55 +0900 Subject: [PATCH 5/9] fix(ci): restore the PR number output for the icon sync workflow - `Create Pull Request` had lost its step `id`, so the `pr_number` job output silently evaluated to empty and the parity report never reached the sync PR - emit `number` on the changed-PR paths too, not just the no-changes path - temporarily pin `parity:compare` to three icons so a green run still renders Figma / code / diff images, confirming the S3 report works --- .github/workflows/icon-parity.yml | 6 +++++- .github/workflows/sync-figma-icons.yml | 15 +++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/.github/workflows/icon-parity.yml b/.github/workflows/icon-parity.yml index 3be30c87f..db30f723e 100644 --- a/.github/workflows/icon-parity.yml +++ b/.github/workflows/icon-parity.yml @@ -68,7 +68,11 @@ jobs: pnpm --filter @repo/icon-extractor parity:fetch --type=basic pnpm --filter @repo/icon-extractor parity:fetch --type=symbol pnpm --filter @repo/icon-extractor parity:render - pnpm --filter @repo/icon-extractor parity:compare + # TEMPORARY — remove once the S3 report is confirmed to render its images. + # `--only` draws every requested row whether it passes or fails, so a green run + # still produces a page with Figma / code / diff images on it. + pnpm --filter @repo/icon-extractor parity:compare \ + --only=DividerOutlineIcon,ImagePackOutlineIcon,AccessibilityOutlineIcon # compare.ts already computed these, so read them back instead of re-deriving them # from the rendered report. diff --git a/.github/workflows/sync-figma-icons.yml b/.github/workflows/sync-figma-icons.yml index 7c0db51b3..67cde0db2 100644 --- a/.github/workflows/sync-figma-icons.yml +++ b/.github/workflows/sync-figma-icons.yml @@ -192,6 +192,10 @@ jobs: fi - name: Create Pull Request + # The `pr_number` job output reads this id. A missing step id is not an error in + # Actions — the expression just evaluates to empty — so losing it silently stopped + # the parity report from ever reaching the sync PR. + id: create_pr env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | @@ -255,13 +259,20 @@ jobs: EXISTING_PR=$(gh pr list --head "$BRANCH_NAME" --base main --state open --json number --jq '.[0].number // empty') if [[ -n "$EXISTING_PR" ]]; then gh pr edit "$EXISTING_PR" --body-file pr_body.txt + PR_NUMBER="$EXISTING_PR" else - gh pr create \ + # `gh pr create` prints the new PR's URL, whose last segment is its number. + PR_URL=$(gh pr create \ --title "feat: sync icons from Figma" \ --body-file pr_body.txt \ --base main \ - --head "$BRANCH_NAME" + --head "$BRANCH_NAME") + PR_NUMBER="${PR_URL##*/}" fi + # The parity job needs this to know where to comment. Emitting it only on + # the no-changes path left the normal case — a PR that just changed — with + # no number at all. + echo "number=$PR_NUMBER" >> $GITHUB_OUTPUT # Clean up temporary file rm -f pr_body.txt From 8b805f45a75e197958598ccd822caf01168aac2c Mon Sep 17 00:00:00 2001 From: MaxLee-dev Date: Wed, 9 Sep 2026 09:50:49 +0900 Subject: [PATCH 6/9] ci(icons): key the parity report on the run and trim its comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - the report key now carries `run_id`: a branch-only key let a second run overwrite the page an earlier PR comment still links to, so that comment quietly started showing a different run's result - drop the two sentences under the table — "실패한 아이콘마다 …" is false on a green run, and the gate rationale belongs in lib.ts, not in every comment - stop extracting the outputs those sentences used; plumbing nothing reads is worse than no plumbing - comment as the same bot as the visual-regression one, which lands on the same PR; the default token stays reserved for machine work - restore the full compare now that S3 is confirmed to serve the images Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/icon-parity.yml | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/.github/workflows/icon-parity.yml b/.github/workflows/icon-parity.yml index db30f723e..d83e13d9d 100644 --- a/.github/workflows/icon-parity.yml +++ b/.github/workflows/icon-parity.yml @@ -68,11 +68,7 @@ jobs: pnpm --filter @repo/icon-extractor parity:fetch --type=basic pnpm --filter @repo/icon-extractor parity:fetch --type=symbol pnpm --filter @repo/icon-extractor parity:render - # TEMPORARY — remove once the S3 report is confirmed to render its images. - # `--only` draws every requested row whether it passes or fails, so a green run - # still produces a page with Figma / code / diff images on it. - pnpm --filter @repo/icon-extractor parity:compare \ - --only=DividerOutlineIcon,ImagePackOutlineIcon,AccessibilityOutlineIcon + pnpm --filter @repo/icon-extractor parity:compare # compare.ts already computed these, so read them back instead of re-deriving them # from the rendered report. @@ -89,11 +85,8 @@ jobs: echo 'ran=true' jq -r '"total=\(.total)", "failed=\(.failed)", - "threshold=\(.threshold)", "mono=\(.mono.count)", - "mono_worst=\(.mono.worst)", - "colour=\(.colour.count)", - "colour_worst=\(.colour.worst)"' "$CACHE/report.json" + "mono_worst=\(.mono.worst)"' "$CACHE/report.json" } >> "$GITHUB_OUTPUT" - name: Configure AWS credentials @@ -111,7 +104,11 @@ jobs: if: steps.counts.outputs.ran == 'true' run: | BRANCH=$(echo "${{ github.head_ref || github.ref_name }}" | tr '/' '-') - KEY="icon-parity/$BRANCH/index.html" + # Keyed on the run, not the branch alone: a branch-only key means a second run + # today overwrites the page that an earlier PR comment still links to, so that + # comment quietly starts showing a different run's result. `run_id` is unique + # per run and already monotonic, so it sorts chronologically for free. + KEY="icon-parity/$BRANCH/${{ github.run_id }}/index.html" aws s3 cp "$CACHE/report.html" "s3://${{ secrets.BUCKET_NAME }}/$KEY" URL="http://${{ secrets.BUCKET_NAME }}/$KEY" echo "url=$URL" >> "$GITHUB_OUTPUT" @@ -131,10 +128,11 @@ jobs: | :--- | :--- | :--- | :--- | :--- | | ${{ steps.counts.outputs.total }} | ${{ steps.counts.outputs.mono }} | ${{ steps.counts.outputs.failed }} | ${{ steps.counts.outputs.mono_worst }} px | [열기 ↗︎](${{ steps.s3.outputs.url }}) | - 실패한 아이콘마다 Figma / 코드 / diff가 나란히 보입니다. 컬러 아이콘 ${{ steps.counts.outputs.colour }}개는 게이트 밖입니다 (최대 ${{ steps.counts.outputs.colour_worst }} px). - 판정 기준 `> ${{ steps.counts.outputs.threshold }} diff px`의 근거는 `scripts/icon-extractor/src/parity/lib.ts` 주석에 있습니다. - [워크플로 실행 보기](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) + # Same author as the visual-regression comment — both land on the same PR, and + # the repo already reserves the default token for machine work rather than + # anything a reviewer reads. + GITHUB_TOKEN: ${{ secrets.VAPOR_BOT_TOKEN }} - name: Fail if parity failed if: steps.compare.outcome == 'failure' From aa1e35189af61028d86b03f557a613dc3615be63 Mon Sep 17 00:00:00 2001 From: MaxLee-dev Date: Wed, 9 Sep 2026 10:01:36 +0900 Subject: [PATCH 7/9] ci(icons): upload the parity report only when it has rows to show MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A green run drew an empty table, and the comment linked to it anyway, so "열기 ↗︎" promised a diff that was not there. - compare.ts records `rendered`, the row count the page will draw - the workflow assumes the role and uploads only when that is non-zero, so a passing run skips both; the comment cell falls back to "—" - the condition is "has rows", not "failed", so it still holds once the page also carries the icons a sync changed Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/icon-parity.yml | 14 +++++++++----- scripts/icon-extractor/src/parity/compare.ts | 10 +++++++--- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/.github/workflows/icon-parity.yml b/.github/workflows/icon-parity.yml index d83e13d9d..948847aa6 100644 --- a/.github/workflows/icon-parity.yml +++ b/.github/workflows/icon-parity.yml @@ -77,7 +77,9 @@ jobs: if: always() run: | if [[ ! -f "$CACHE/report.json" ]]; then - echo 'ran=false' >> "$GITHUB_OUTPUT" + # rendered must be set here too: an unset output is '', and '' != '0' + # would send the upload after a page that was never written. + printf 'ran=false\nrendered=0\n' >> "$GITHUB_OUTPUT" echo 'No parity report — the comparison never ran.' >> "$GITHUB_STEP_SUMMARY" exit 0 fi @@ -86,11 +88,13 @@ jobs: jq -r '"total=\(.total)", "failed=\(.failed)", "mono=\(.mono.count)", - "mono_worst=\(.mono.worst)"' "$CACHE/report.json" + "mono_worst=\(.mono.worst)", + "rendered=\(.rendered)"' "$CACHE/report.json" } >> "$GITHUB_OUTPUT" + # A green run draws no rows, so there is no page worth an assume-role and an upload. - name: Configure AWS credentials - if: steps.counts.outputs.ran == 'true' + if: steps.counts.outputs.rendered != '0' uses: aws-actions/configure-aws-credentials@v4 with: role-to-assume: ${{ secrets.AWS_IAM_ROLE_NAME }} @@ -101,7 +105,7 @@ jobs: # is the whole report. `index.html` is the bucket website's default document. - name: Upload report to S3 id: s3 - if: steps.counts.outputs.ran == 'true' + if: steps.counts.outputs.rendered != '0' run: | BRANCH=$(echo "${{ github.head_ref || github.ref_name }}" | tr '/' '-') # Keyed on the run, not the branch alone: a branch-only key means a second run @@ -126,7 +130,7 @@ jobs: | 전체 | 게이트 대상 (mono) | 실패 | mono 최대 diff | 리포트 | | :--- | :--- | :--- | :--- | :--- | - | ${{ steps.counts.outputs.total }} | ${{ steps.counts.outputs.mono }} | ${{ steps.counts.outputs.failed }} | ${{ steps.counts.outputs.mono_worst }} px | [열기 ↗︎](${{ steps.s3.outputs.url }}) | + | ${{ steps.counts.outputs.total }} | ${{ steps.counts.outputs.mono }} | ${{ steps.counts.outputs.failed }} | ${{ steps.counts.outputs.mono_worst }} px | ${{ steps.counts.outputs.rendered != '0' && format('[열기 ↗︎]({0})', steps.s3.outputs.url) || '—' }} | [워크플로 실행 보기](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) # Same author as the visual-regression comment — both land on the same PR, and diff --git a/scripts/icon-extractor/src/parity/compare.ts b/scripts/icon-extractor/src/parity/compare.ts index 146cdb7f1..c938177b6 100644 --- a/scripts/icon-extractor/src/parity/compare.ts +++ b/scripts/icon-extractor/src/parity/compare.ts @@ -93,6 +93,10 @@ const failures = rows.filter((row) => row.failed); const mono = rows.filter((row) => !row.isColorIcon); const color = rows.filter((row) => row.isColorIcon); const worst = (group: Row[]) => Math.max(0, ...group.map((row) => row.diffPixels)); +// What the HTML page will actually draw. An explicit --only shows every requested icon; otherwise +// only failures carry images. The workflow uploads the page only when this is non-empty, so a +// green run never hands a reviewer a link to an empty table. +const drawn = args.only ? rows : failures; // The headline counts live here, not only in the rendered page: the workflow reads them straight // out of this file for the PR comment. Re-parsing a rendered report to recover numbers we already @@ -103,6 +107,7 @@ const report = { total: rows.length, expected: names.length, failed: failures.length, + rendered: drawn.length, mono: { count: mono.length, worst: worst(mono) }, colour: { count: color.length, worst: worst(color) }, skipped, @@ -113,14 +118,13 @@ const report = { await fs.writeFile(path.join(CACHE_DIR, 'report.json'), JSON.stringify(report, null, 2)); // Self-contained HTML (images inlined) so the one file the workflow uploads to S3 opens on its -// own. Only failing rows carry images (an explicit --only shows every requested icon); the plain -// lists below cover the other failure causes. +// own. The plain lists below cover the failure causes that have no image to show. const dataUri = async (file: string) => `data:image/png;base64,${(await fs.readFile(file)).toString('base64')}`; const cell = async (dir: string, name: string) => ``; const htmlRows = await Promise.all( - (args.only ? rows : failures).map( + drawn.map( async (row) => `${row.name}
${row.diffPixels} px` + (await cell(baselineDir, row.name)) + From b38f60b214d609dedc4ea26afd97d2bc01c3995f Mon Sep 17 00:00:00 2001 From: MaxLee-dev Date: Wed, 9 Sep 2026 10:03:25 +0900 Subject: [PATCH 8/9] feat(icon-extractor): draw the icons a sync changed on the parity report A green sync PR left a reviewer with nothing to look at, even though the icons in it had just been regenerated from Figma. - `--show=A,B` adds rows to the page without narrowing what gets compared; `--only` cannot do this job, since it drops every other icon out of the gate - the sync workflow passes the icons it created or updated, so the report opens with them next to Figma; deleted ones are left out, having no component - failures lead the table and the page is capped at 60 rows, so a regeneration that marks all 814 icons changed cannot bury a failure or produce a page of several megabytes - rows without FAIL say why they are there, and empty entries in the list are dropped so the workflow can stitch it from four step outputs Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/icon-parity.yml | 9 +++++- .github/workflows/sync-figma-icons.yml | 4 +++ scripts/icon-extractor/src/parity/compare.ts | 33 +++++++++++++++++--- scripts/icon-extractor/src/parity/lib.ts | 24 ++++++++++++-- 4 files changed, 61 insertions(+), 9 deletions(-) diff --git a/.github/workflows/icon-parity.yml b/.github/workflows/icon-parity.yml index 948847aa6..ee4b98ddd 100644 --- a/.github/workflows/icon-parity.yml +++ b/.github/workflows/icon-parity.yml @@ -16,6 +16,10 @@ on: description: 'PR to comment the report on. Empty means report to the log only.' type: string required: false + changed-icons: + description: 'Comma-separated icons to draw on the report even when they pass.' + type: string + required: false pull_request: paths: - packages/icons/src/** @@ -68,7 +72,10 @@ jobs: pnpm --filter @repo/icon-extractor parity:fetch --type=basic pnpm --filter @repo/icon-extractor parity:fetch --type=symbol pnpm --filter @repo/icon-extractor parity:render - pnpm --filter @repo/icon-extractor parity:compare + # --show adds rows to the page; it does not narrow the gate. Empty on the + # pull_request path, where the diff itself already says what changed. + pnpm --filter @repo/icon-extractor parity:compare \ + --show="${{ inputs.changed-icons }}" # compare.ts already computed these, so read them back instead of re-deriving them # from the rendered report. diff --git a/.github/workflows/sync-figma-icons.yml b/.github/workflows/sync-figma-icons.yml index 67cde0db2..f25052b24 100644 --- a/.github/workflows/sync-figma-icons.yml +++ b/.github/workflows/sync-figma-icons.yml @@ -30,6 +30,9 @@ jobs: outputs: has_changes: ${{ steps.commit.outputs.has_changes }} pr_number: ${{ steps.create_pr.outputs.number }} + # New and updated icons get drawn on the parity report so the sync PR shows them next + # to Figma. Deleted ones are left out — there is no component left to render. + changed_icons: ${{ format('{0},{1},{2},{3}', steps.sync_basic.outputs.new_icons, steps.sync_basic.outputs.updated_icons, steps.sync_symbol.outputs.new_icons, steps.sync_symbol.outputs.updated_icons) }} steps: - name: Checkout branch uses: actions/checkout@v4 @@ -306,6 +309,7 @@ jobs: secrets: inherit with: pr-number: ${{ needs.sync-figma-icons.outputs.pr_number }} + changed-icons: ${{ needs.sync-figma-icons.outputs.changed_icons }} notify: name: Notify Slack diff --git a/scripts/icon-extractor/src/parity/compare.ts b/scripts/icon-extractor/src/parity/compare.ts index c938177b6..ce67cc5e3 100644 --- a/scripts/icon-extractor/src/parity/compare.ts +++ b/scripts/icon-extractor/src/parity/compare.ts @@ -6,7 +6,15 @@ import pixelmatch from 'pixelmatch'; import { PNG } from 'pngjs'; import type { Manifest } from './lib'; -import { CACHE_DIR, DIFF_GATE, MANIFEST, PIXELMATCH_OPTIONS, flags, onlyFilter } from './lib'; +import { + CACHE_DIR, + DIFF_GATE, + MANIFEST, + PIXELMATCH_OPTIONS, + flags, + nameSet, + onlyFilter, +} from './lib'; type Row = { name: string; @@ -22,6 +30,10 @@ const threshold = Number(args.threshold ?? DIFF_GATE); // Colour icons are rasterizer noise, not signal — gate them only to exercise the failure path. const gateColor = args['gate-color'] === 'true'; const keep = onlyFilter(args.only); +// `--show=A,B` adds rows to the page without narrowing what gets compared. The sync workflow +// passes the icons it just changed, so a reviewer sees them next to Figma even on a green run. +// `--only` cannot do this job: it drops every other icon out of the gate as well. +const shown = args.show ? nameSet(args.show) : new Set(); const baselineDir = path.join(CACHE_DIR, 'baseline'); const codeDir = path.join(CACHE_DIR, 'render'); @@ -94,9 +106,17 @@ const mono = rows.filter((row) => !row.isColorIcon); const color = rows.filter((row) => row.isColorIcon); const worst = (group: Row[]) => Math.max(0, ...group.map((row) => row.diffPixels)); // What the HTML page will actually draw. An explicit --only shows every requested icon; otherwise -// only failures carry images. The workflow uploads the page only when this is non-empty, so a -// green run never hands a reviewer a link to an empty table. -const drawn = args.only ? rows : failures; +// failures first, then whatever --show asked for. The workflow uploads the page only when this is +// non-empty, so a green run never hands a reviewer a link to an empty table. +// +// Failures lead so the cap can never hide one: a sync that regenerates every icon marks all 814 as +// changed, and a page with 814 rows of inlined PNGs is several megabytes. +const MAX_DRAWN = 60; +const candidates = args.only + ? rows + : [...failures, ...rows.filter((row) => !row.failed && shown.has(row.name))]; +const drawn = candidates.slice(0, MAX_DRAWN); +const folded = candidates.length - drawn.length; // The headline counts live here, not only in the rendered page: the workflow reads them straight // out of this file for the PR comment. Re-parsing a rendered report to recover numbers we already @@ -126,7 +146,8 @@ const cell = async (dir: string, name: string) => const htmlRows = await Promise.all( drawn.map( async (row) => - `${row.name}
${row.diffPixels} px` + + `${row.name}${row.failed ? ' FAIL' : ''}` + + `
${row.diffPixels} px` + (await cell(baselineDir, row.name)) + (await cell(codeDir, row.name)) + (await cell(diffDir, row.name)) + @@ -148,7 +169,9 @@ img{display:block;width:128px;height:128px;image-rendering:pixelated}

Icon parity report

${failures.length} of ${mono.length} mono icons over ${threshold} diff pixels (worst ${worst(mono)}).
${color.length} colour icons are reported but not gated — Figma and Chromium disagree on these (worst ${worst(color)}).

+${shown.size ? '

Rows without FAIL are icons this sync changed. They are here for review, not because anything is wrong with them.

' : ''} ${htmlRows.join('')}
FigmaCodeDiff
+${folded ? `

${folded} more row(s) not drawn — the page is capped at ${MAX_DRAWN}.

` : ''} ${htmlList('missing renders', missing)}${htmlList('skipped (size mismatch)', skipped)}${htmlList('not in manifest', unclassified)}`, ); diff --git a/scripts/icon-extractor/src/parity/lib.ts b/scripts/icon-extractor/src/parity/lib.ts index 6ddaf8c09..ea8999824 100644 --- a/scripts/icon-extractor/src/parity/lib.ts +++ b/scripts/icon-extractor/src/parity/lib.ts @@ -58,12 +58,30 @@ function flags(): Record { return out; } -/** `--only=A,B` keeps just those icon names; without the flag everything passes. */ +/** + * `--only=A,B` keeps just those icon names; without the flag everything passes. + * + * Empty entries are dropped so a caller can pass a list stitched from several sources — + * `format('{0},{1}', ...)` in a workflow yields `A,,B` when one source is empty. + */ +function nameSet(list: string): Set { + return new Set(list.split(',').filter(Boolean)); +} + function onlyFilter(only: string | undefined): (name: string) => boolean { if (!only) return () => true; - const wanted = new Set(only.split(',')); + const wanted = nameSet(only); return (name) => wanted.has(name); } export type { Manifest }; -export { CACHE_DIR, DIFF_GATE, MANIFEST, PARITY_DIR, PIXELMATCH_OPTIONS, flags, onlyFilter }; +export { + CACHE_DIR, + DIFF_GATE, + MANIFEST, + PARITY_DIR, + PIXELMATCH_OPTIONS, + flags, + nameSet, + onlyFilter, +}; From 344a59c948a6eeffc54c6b2170b000f65f457df9 Mon Sep 17 00:00:00 2001 From: MaxLee-dev Date: Thu, 10 Sep 2026 09:36:25 +0900 Subject: [PATCH 9/9] fix(ci): pass workflow context through env and fail loudly on parity gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit 리뷰 4건 반영. - `${{ }}`는 bash가 파싱하기 전에 러너가 수행하는 텍스트 치환이라 큰따옴표 안에서도 `$(...)`가 실행된다. 브랜치명·아이콘 목록·ref 이름을 전부 스텝 `env`로 넘겼다. 특히 S3 업로드 스텝은 AWS 세션을 들고 있어 노출 대상이 자격증명이었다. - `--threshold`에 숫자가 아닌 값이 오면 `NaN`이 되고 `diffPixels > NaN`은 항상 false라 게이트가 말없이 꺼졌다. 검증을 넣어 거부한다. - Figma 렌더 실패를 경고로 흘리면 그 아이콘의 PNG가 baseline에 안 생기고, compare가 baseline을 기준으로 검사 대상을 정하므로 게이트에서 조용히 빠졌다. - 워크플로우 주석이 없는 CALIBRATION.md를 가리키고 있어 실제 근거가 있는 lib.ts로 바꿨다. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/icon-parity.yml | 15 +++++++++++--- .github/workflows/sync-figma-icons.yml | 20 ++++++++++--------- scripts/icon-extractor/src/parity/compare.ts | 5 +++++ .../src/parity/fetch-baseline.ts | 9 ++++++++- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/.github/workflows/icon-parity.yml b/.github/workflows/icon-parity.yml index ee4b98ddd..44a49e92e 100644 --- a/.github/workflows/icon-parity.yml +++ b/.github/workflows/icon-parity.yml @@ -2,7 +2,8 @@ name: '[CI] Icon Parity' # Renders every icon component in Chromium and compares it to Figma's own PNG. # Gate values (64px canvas, pixelmatch threshold 0.3 / includeAA, fail above 2 diff px) -# are measured, not guessed — scripts/icon-extractor/src/parity/CALIBRATION.md. +# are measured, not guessed — each value carries its measurement in the comments of +# scripts/icon-extractor/src/parity/lib.ts. # # Two entry points share this one job so the gate and the report live in one place: # - pull_request: any change that can alter how an icon rasterizes @@ -68,6 +69,10 @@ jobs: continue-on-error: true env: FIGMA_TOKEN: ${{ secrets.FIGMA_TOKEN }} + # Passed through env, never interpolated into the script: `${{ }}` is a text + # substitution the runner performs before bash parses the file, so a value + # holding `$(...)` would run as code even inside double quotes. + CHANGED_ICONS: ${{ inputs.changed-icons }} run: | pnpm --filter @repo/icon-extractor parity:fetch --type=basic pnpm --filter @repo/icon-extractor parity:fetch --type=symbol @@ -75,7 +80,7 @@ jobs: # --show adds rows to the page; it does not narrow the gate. Empty on the # pull_request path, where the diff itself already says what changed. pnpm --filter @repo/icon-extractor parity:compare \ - --show="${{ inputs.changed-icons }}" + --show="$CHANGED_ICONS" # compare.ts already computed these, so read them back instead of re-deriving them # from the rendered report. @@ -113,8 +118,12 @@ jobs: - name: Upload report to S3 id: s3 if: steps.counts.outputs.rendered != '0' + env: + # A branch name may contain `$`, `(` and backticks, so interpolating it into + # the script would hand this step — which holds the AWS session — arbitrary code. + BRANCH_REF: ${{ github.head_ref || github.ref_name }} run: | - BRANCH=$(echo "${{ github.head_ref || github.ref_name }}" | tr '/' '-') + BRANCH=$(echo "$BRANCH_REF" | tr '/' '-') # Keyed on the run, not the branch alone: a branch-only key means a second run # today overwrites the page that an earlier PR comment still links to, so that # comment quietly starts showing a different run's result. `run_id` is unique diff --git a/.github/workflows/sync-figma-icons.yml b/.github/workflows/sync-figma-icons.yml index f25052b24..b59313afe 100644 --- a/.github/workflows/sync-figma-icons.yml +++ b/.github/workflows/sync-figma-icons.yml @@ -201,17 +201,19 @@ jobs: id: create_pr env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # Every value below comes in through env rather than `${{ }}` inside the script: + # the runner substitutes those as text before bash parses the file, so a value + # holding `$(...)` would run as code even inside double quotes. + NEW_BASIC_ICONS: ${{ steps.sync_basic.outputs.new_icons }} + NEW_SYMBOL_ICONS: ${{ steps.sync_symbol.outputs.new_icons }} + UPDATED_BASIC_ICONS: ${{ steps.sync_basic.outputs.updated_icons }} + UPDATED_SYMBOL_ICONS: ${{ steps.sync_symbol.outputs.updated_icons }} + DELETED_BASIC_ICONS: ${{ steps.sync_basic.outputs.deleted_icons }} + DELETED_SYMBOL_ICONS: ${{ steps.sync_symbol.outputs.deleted_icons }} + REF_NAME: ${{ github.ref_name }} run: | # Check if the branch was pushed (indicates changes were found) if [[ "${has_changes:-}" == "true" ]]; then - # Prepare icon lists for PR body - NEW_BASIC_ICONS="${{ steps.sync_basic.outputs.new_icons }}" - NEW_SYMBOL_ICONS="${{ steps.sync_symbol.outputs.new_icons }}" - UPDATED_BASIC_ICONS="${{ steps.sync_basic.outputs.updated_icons }}" - UPDATED_SYMBOL_ICONS="${{ steps.sync_symbol.outputs.updated_icons }}" - DELETED_BASIC_ICONS="${{ steps.sync_basic.outputs.deleted_icons }}" - DELETED_SYMBOL_ICONS="${{ steps.sync_symbol.outputs.deleted_icons }}" - # Determine version bump type for PR body if [[ -n "$NEW_BASIC_ICONS" || -n "$NEW_SYMBOL_ICONS" ]]; then VERSION_BUMP="Minor" @@ -285,7 +287,7 @@ jobs: STALE_PR=$(gh pr list --head "$BRANCH_NAME" --base main --state open --json number --jq '.[0].number // empty') # Only main can judge staleness — a dispatch from another branch compared # Figma against that branch, so "no changes" says nothing about the sync PR. - if [[ -n "$STALE_PR" && "${{ github.ref_name }}" == "main" ]]; then + if [[ -n "$STALE_PR" && "$REF_NAME" == "main" ]]; then gh pr close "$STALE_PR" --comment "Closing: main already matches Figma, so this sync is no longer valid." elif [[ -n "$STALE_PR" ]]; then # Feature-branch dispatch: let the parity report land on the open sync PR. diff --git a/scripts/icon-extractor/src/parity/compare.ts b/scripts/icon-extractor/src/parity/compare.ts index ce67cc5e3..c546495a6 100644 --- a/scripts/icon-extractor/src/parity/compare.ts +++ b/scripts/icon-extractor/src/parity/compare.ts @@ -27,6 +27,11 @@ type Row = { const args = flags(); const threshold = Number(args.threshold ?? DIFF_GATE); +// A non-numeric --threshold parses to NaN, and `diffPixels > NaN` is always false — the gate +// would pass everything without a word, which is the one failure lib.ts warns about. +if (!Number.isFinite(threshold) || threshold < 0) { + throw new Error(`--threshold must be a non-negative number (got ${args.threshold})`); +} // Colour icons are rasterizer noise, not signal — gate them only to exercise the failure path. const gateColor = args['gate-color'] === 'true'; const keep = onlyFilter(args.only); diff --git a/scripts/icon-extractor/src/parity/fetch-baseline.ts b/scripts/icon-extractor/src/parity/fetch-baseline.ts index a57e38021..a724d4cb9 100644 --- a/scripts/icon-extractor/src/parity/fetch-baseline.ts +++ b/scripts/icon-extractor/src/parity/fetch-baseline.ts @@ -91,6 +91,7 @@ for (const icon of wanted) { console.log(`${wanted.length} icons, ${missing.length} to download (scale ${scale})`); const urls = new Map(); +const renderFailed: string[] = []; for (let i = 0; i < missing.length; i += BATCH_SIZE) { const batch = missing.slice(i, i + BATCH_SIZE); const { images } = await getImage({ @@ -103,10 +104,16 @@ for (let i = 0; i < missing.length; i += BATCH_SIZE) { for (const icon of batch) { // HTTP 200 does not mean every node rendered — nulls are per-node failures. const url = images[icon.id]; - if (!url) console.warn(pc.yellow(`render failed: ${icon.name}`)); + if (!url) renderFailed.push(icon.name); else urls.set(icon.name, url); } } +// A warning would let the run go green with a smaller baseline, and compare.ts enumerates the +// baseline to decide what to check — so a failed render would quietly drop that icon's gate. +if (renderFailed.length) { + console.error(pc.red(`figma rendered no image for: ${renderFailed.join(', ')}`)); + process.exit(1); +} const concurrency = pLimit(10); await Promise.all(