feat: create Figma Token Review Plugin#625
Conversation
…an plumbing - App scaffolding, manifest, Vite config - Shared schema + message types (common/) - Wire selection state between plugin sandbox and UI - Stub callEvaluator + scan plumbing - Design + implementation planning docs
- Wire fixtures into callEvaluator, render tabs + violation accordion - Restore summary header, row-level focus click, sort/copy - Drop stale scan results on selection change, focus offending nodes on canvas - Redesign pre-scan selection screen to match Figma spec - Match Figma container structure (button outside state group, pt/gap) - Selection-error toast + light survey asset, disable scan without frame - SuccessState for clean scan result, confirm icon swap, use-funnel - Address final review findings (focus-error channel, dead chip, toast cleanup, exhaustive switches, eager scan invalidation)
- Centralize toast, migrate to kebab-case file names, restructure for page split - Resize handling + module separation, tabs button nowrap - Violation-detail box UI tweaks, token adjust, code cleanup + format
…ry schema - Design spec + call-api scaffold for LiteLLM integration - Align Violation/Summary schema with evaluator output - Add conformanceRate to EvaluateSummary - Add missing fixture files
…leanup - Skip 🟨/🔶 prefixed nodes during extraction (continue traversing children) - Force schema-compliant Korean JSON output in system prompt - Align Conformant.nodeIds optionality with SYSTEM_PROMPT - Rename plugin/call-evaluator → plugin/extract - Naming cleanup design spec + implementation plan
…rs + common/messages - Move ui/evaluator to ui/libs/llm with Llm* family - Consolidate messaging layer into common/messages convention - Refactor src/ui structure based on features (features/*, shared/, components/) - Separate schemas, text left-align, refresh UI - ARCHITECTURE.md sync + brief-mandated 1단/2단 bullet lines - common/messages integration design + plan docs
… color evaluator - Design spec + implementation plan for det/LLM split - Foundation module wiring (loaders, evaluate/, recommend) - Token schema loaders (color/dimension/typography) - Convert oklch primitive \$value to hex - Deterministic color evaluator (raw / unknown / primitive / do-not-use / role-mismatch / fg-grade)
…s + recommend module - Deterministic evaluators: space, dimension, borderRadius, shadow, typography - Unified recommend module (T9) — value-match candidates per category
…t + 6 category tabs - Extract space/dimension/radius/shadow from Figma nodes - LLM input rubric subset builder - Rewire LLM flow for semantic-only judgment (deterministic-passed only) - Render 6 category tabs + heuristic badges - Fixes: dedup do-not-use avoid suggestions, raw color fallback returns primitive - Final review wave (PROPERTY_LABEL, isJudgments, parse error, MANIFEST)
- Post-smoke findings 1-5, restore nodeId flow for LLM heuristic violations - Unify token key format to dash-separated - groupBy preserves nodeId (= nodeIds[0]) - Third smoke findings 1-4 - Strip leading collection prefix instead of slash-to-dash convert - Relocate test files, format
…xt spec - Rescan button gated on new/reselected frame - Split scan lifecycle vs frame snapshot state - Stage-3 semantic judgment with vision context — design spec + implementation plan
…node tree - Capture frame screenshot + node tree in extract stage - Thread nodeTree into LlmInput - buildRequest emits text+image content blocks - Thread LlmContext through runLlmEvaluation + evaluateExtract - Restrict stroke property scope to border role only
…pansion - Add typography axis ViolationTypes + TEXT NodeInfo fields - Resolve textStyle fontSize alias to primitive - Fill deterministic suggested for typo-raw and typo-styled-override - recommend.ts passes typography deterministic violations through - Expose full textStyle rubric to LLM - nodeTree TEXT nodes carry characters and textStyle - Enforce axis + matchedRule on LlmTypoJudgment - Prompt requires axis + matchedRule per typography judgment
- axis→ViolationType map + matchedRule prepend + suggested filter - Pass textStyleSchema to mergeScanPayload for suggested filter - Multi-fontSize-match assertions + describe block wrap - Route typo-role-misfit and typo-viewport-misfit to Text section - Surface LLM PASS judgments in Typography tab - Skip node data but continue traversal into children - Add code1 and code2 textStyles to schema - Add text-style schema, update gitignore
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Figma token-review plugin with shared contracts and token data, Figma extraction and messaging, deterministic evaluation, LLM-assisted review, and React screens for settings, scanning, and results. ChangesFigma Token Review Plugin
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant HomePage
participant useScan
participant CodePlugin
participant extractFrame
participant evaluateExtract
participant runLlmEvaluation
participant ScanResultPage
HomePage->>useScan: start selected frame
useScan->>CodePlugin: send scan request
CodePlugin->>extractFrame: extract frame data
extractFrame->>evaluateExtract: return extract result
evaluateExtract->>runLlmEvaluation: evaluate with API key
runLlmEvaluation->>evaluateExtract: return ScanPayload
evaluateExtract->>ScanResultPage: update scan result
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (17)
apps/figma-token-review-plugin/ARCHITECTURE.md (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language identifiers to fenced code blocks.
Static analysis flags these three fenced blocks (tree diagrams) as missing a language hint (MD040).
Also applies to: 47-47, 146-146
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/ARCHITECTURE.md` at line 3, The fenced code blocks in ARCHITECTURE.md are missing language identifiers and are triggering MD040. Update the affected fenced blocks that render the tree diagrams by adding an appropriate language hint to each fence, including the other two referenced fenced blocks, while keeping the existing content unchanged.Source: Linters/SAST tools
apps/figma-token-review-plugin/vite.config.ui.ts (1)
14-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCwd-relative paths mixed with
__dirname-relative alias.
outDir/rollupOptions.inputusepath.resolve('dist')/path.resolve('index.html')(relative toprocess.cwd()), whilealiasuses__dirname. If this config is ever invoked from a different working directory (e.g., a monorepo root script), build output/input paths could resolve incorrectly.♻️ Suggested fix
build: { minify: mode === 'production', cssMinify: mode === 'production', sourcemap: mode !== 'production' ? 'inline' : false, emptyOutDir: false, - outDir: path.resolve('dist'), + outDir: path.resolve(__dirname, 'dist'), rollupOptions: { - input: path.resolve('index.html'), + input: path.resolve(__dirname, 'index.html'), }, },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/vite.config.ui.ts` around lines 14 - 22, The build config mixes cwd-relative paths with a __dirname-relative alias, so the ui Vite setup can resolve input/output incorrectly when run from a different working directory. Update the build section in vite.config.ui.ts to resolve outDir and rollupOptions.input from the same base as the alias (using __dirname consistently) so path resolution is stable regardless of where the config is invoked. Keep the existing build and rollupOptions structure, just make the path construction consistent.apps/figma-token-review-plugin/src/ui/components/hero-panel.tsx (1)
5-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded
bg-whiteinstead of a design token.Other components in this cohort (e.g.
scan-result.tsx) consistently style backgrounds via vapor$csstokens ($bg-canvas-100). Using a raw Tailwind color here is a minor inconsistency, notably ironic for a plugin whose purpose is flagging raw/non-token values.♻️ Suggested tweak
function Root({ children }: { children: ReactNode }) { return ( - <Box className="flex flex-col items-center bg-white"> + <Box className="flex flex-col items-center" $css={{ backgroundColor: '$bg-canvas-100' }}> <Box className="flex flex-col items-center gap-5 pt-30">{children}</Box> </Box> ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/ui/components/hero-panel.tsx` around lines 5 - 11, The Root component in hero-panel.tsx is using a hardcoded bg-white class instead of the shared vapor design token style used elsewhere. Update the Box wrapper in Root to use the same token-based background approach as components like scan-result.tsx, replacing the raw Tailwind color with the appropriate $css token so the styling stays consistent across the plugin UI.apps/figma-token-review-plugin/src/ui/components/resize-handler.tsx (2)
97-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueResize handle is pointer-only; no keyboard interaction path.
The element has
role="separator"but notabIndexoronKeyDown, so it cannot be reached or operated via keyboard, despite being interactive. This is a minor a11y gap for a non-critical convenience feature.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/ui/components/resize-handler.tsx` around lines 97 - 107, The resize handle in resize-handler.tsx is pointer-only even though it uses role="separator"; make it keyboard-accessible by adding a focusable tabIndex and an onKeyDown handler on the separator element. Use the existing resize logic from handlePointerDown/handlePointerMove/handlePointerUp to support keyboard resizing (for example via arrow keys), and keep the aria-orientation and aria-label in sync with the interactive behavior.
48-95: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueNo upper bound on resize; drag basis uses
window.innerWidth/Heighton every pointerdown.
MIN_WIDTH/MIN_HEIGHTclamp the lower bound but there's no corresponding max clamp, so a user could resize the plugin window arbitrarily large. Also, usingwindow.innerWidth/innerHeightasstartWidth/startHeightat drag start (rather than tracking a canonical size) is fine as long as resizes always commit synchronously before the next drag, but worth double-checking againsthandlers/resize.tsto confirm the window is resized synchronously enough that this remains accurate across rapid successive drags.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/ui/components/resize-handler.tsx` around lines 48 - 95, The resize drag logic in handlePointerMove and handlePointerUp only enforces minimum bounds, so add a maximum size clamp before queueResize and postToCode to prevent unbounded growth. Also verify the drag baseline in handlePointerDown, which currently uses window.innerWidth and window.innerHeight, remains correct with the synchronous behavior in the resize handler path; if not, switch to the canonical stored size used by the resize flow instead of reading the window size at pointerdown.apps/figma-token-review-plugin/src/ui/components/violation-card/violation-card.tsx (2)
41-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate hex-validation regex; consolidate with
utils.ts.This anchored
/^#[0-9a-fA-F]{3,8}$/check duplicates the intent ofisHexColorin./utils.ts(which is unanchored and thus behaves differently). Having two slightly different hex regexes across the same feature invites inconsistency — reuse a single, anchored validator fromutils.tshere instead of inlining a second pattern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/ui/components/violation-card/violation-card.tsx` around lines 41 - 49, The hex check in resolveHex duplicates logic that already exists in utils.ts and uses a slightly different pattern, which can lead to inconsistent behavior. Update violation-card.tsx to reuse the shared hex validator from utils.ts inside resolveHex instead of inlining the regex, and ensure the shared helper is the single source of truth for hex detection across this feature.
41-49: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
loadColorSchemais re-loaded/flattened on every render for every violation card.
resolveHexcallsloadColorSchema(schemaMode)up to twice perViolationCardrender (Lines 65-66), andScanResultPagerenders oneViolationCardper violation in a list (per the downstream snippet inscan-result.tsx). Per the loader's upstream implementation,loadColorSchemare-flattens the primitive/semantic JSON on every call. Since the schema is static permode, this work is being repeated unnecessarily on every list render/re-render, scaling with the number of violations.Consider memoizing the schema per mode (e.g., a
Map<SchemaMode, ColorSchema>cache in the loader, or hoisting the schema load to a parent and passing it down) so it's computed once per mode rather than per card per render.Also applies to: 64-66
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/ui/components/violation-card/violation-card.tsx` around lines 41 - 49, The schema load in resolveHex is being repeated for every ViolationCard render, which causes loadColorSchema(schemaMode) to re-flatten static data unnecessarily. Update resolveHex and its call sites so the ColorSchema is memoized per SchemaMode, either by caching inside loadColorSchema or by loading it once in a parent like ViolationCard/ScanResultPage and passing it down, so each mode is computed only once instead of per card render.apps/figma-token-review-plugin/src/ui/lib/loaders/color.ts (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
NSconstant across loaders.
const NS = 'io.goorm.vapor'is duplicated verbatim intypography.ts(line 4), along with a similar$extensions[NS]extraction pattern. Extracting this into a shared constant/helper (e.g. incommon/schemas.tsor a newcommon/vapor-meta.ts) would avoid drift if the namespace ever changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/ui/lib/loaders/color.ts` at line 7, The NS namespace constant is duplicated across the color and typography loaders, so centralize it in a shared helper/constant instead of redefining it in each loader. Move the `NS` value and any repeated `$extensions[NS]` access pattern into a common module such as `common/schemas.ts` or a new shared vapor meta helper, then update the relevant loader functions (including `color.ts` and `typography.ts`) to use the shared symbol.apps/figma-token-review-plugin/src/ui/lib/loaders/typography.ts (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
NSconstant (seecolor.tsline 7).Same
io.goorm.vaporextension namespace constant and extraction pattern duplicated here; worth consolidating into a shared module.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/ui/lib/loaders/typography.ts` at line 4, The typography loader defines the same NS extension namespace constant and extraction pattern already present in color.ts, so consolidate this shared value into a common module and import it here instead of duplicating it. Update the typography loader to use the shared namespace symbol (for example, the existing NS reference used by the color loader) and keep the extraction logic unchanged so both loaders source the namespace from one place.apps/figma-token-review-plugin/src/plugin/handlers/extract.ts (1)
692-696: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueScreenshot/tree-walk failures are silently swallowed.
captureScreenshot(...).catch(() => '')andwalkTree(...).catch(() => [])degradellmContextwithout any logging. Since the LLM step (typography/vision evaluation) depends on this context, a failed export or traversal silently produces a lower-quality (or empty) LLM judgment with no diagnostic trail.♻️ Suggested addition
const [screenshotB64, nodeTree] = await Promise.all([ - captureScreenshot(root as FrameNode).catch(() => ''), - walkTree(root as SceneNode).catch(() => [] as NodeInfo[]), + captureScreenshot(root as FrameNode).catch((e) => { + console.error('[extractFrame] screenshot capture failed', e); + return ''; + }), + walkTree(root as SceneNode).catch((e) => { + console.error('[extractFrame] walkTree failed', e); + return [] as NodeInfo[]; + }), ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/plugin/handlers/extract.ts` around lines 692 - 696, The screenshot and tree traversal failures in extract’s Promise.all are being swallowed, which can silently degrade llmContext without any diagnostics. Update the captureScreenshot and walkTree error handling in extract to log the caught error before falling back to an empty string or empty array, so the LLM context issues are traceable while preserving the current fallback behavior.apps/figma-token-review-plugin/src/plugin/handlers/focus.ts (1)
12-20: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winResolve node IDs in parallel.
Sequential
awaitpernodeIdis unnecessary since each lookup is independent;Promise.allwould reduce total latency, especially for larger focus sets.Proposed fix
- for (const nodeId of msg.nodeIds) { - const n = await figma.getNodeByIdAsync(nodeId); - - if (n && n.type !== 'DOCUMENT' && n.type !== 'PAGE' && 'visible' in n) { - resolved.push(n as SceneNode); - } else { - missing.push(nodeId); - } - } + const nodes = await Promise.all(msg.nodeIds.map((id) => figma.getNodeByIdAsync(id))); + nodes.forEach((n, i) => { + if (n && n.type !== 'DOCUMENT' && n.type !== 'PAGE' && 'visible' in n) { + resolved.push(n as SceneNode); + } else { + missing.push(msg.nodeIds[i]); + } + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/plugin/handlers/focus.ts` around lines 12 - 20, The node lookup loop in the focus handler is doing independent getNodeByIdAsync calls sequentially, which adds unnecessary latency. Update the focus.ts logic that builds resolved and missing so it resolves msg.nodeIds in parallel with Promise.all, then filters each result into resolved or missing while preserving the existing node-type and visibility checks.apps/figma-token-review-plugin/src/plugin/handlers/extract.test.ts (1)
85-129: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider asserting sibling traversal order.
walkTree(per the extract.ts context) uses a stack withpop(), which processes children in reverse of their declared order at each level. The test only checks set membership (toContain) forids, not order among multiple siblings, so a future accidental change to traversal order wouldn't be caught here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/plugin/handlers/extract.test.ts` around lines 85 - 129, The walkTree test currently verifies only that nodes are present, so it can miss a sibling-order regression caused by the stack/pop traversal in walkTree. Update this test in extract.test.ts to assert the exact traversal order for siblings at the same level, using the existing walkTree, root, skipped, and leaf setup so a change in declared child order or stack handling will fail. Keep the current parent/child and skipped-subtree assertions, but add an order check that distinguishes the first and second child visitation.apps/figma-token-review-plugin/src/ui/providers.tsx (1)
12-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
toastManager/timeoutprops.
ToastProvider(intoast.tsx) already hardcodestoastManager={toastManager}andtimeout={3000}after spreadingprops, so these two props passed here are silently discarded. Passing them here implies they're configurable, which is misleading — consider dropping them to avoid future drift if one of the two values changes.♻️ Proposed simplification
- <ToastProvider toastManager={toastManager} timeout={3000}> - {children} - </ToastProvider> + <ToastProvider>{children}</ToastProvider>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/ui/providers.tsx` around lines 12 - 16, The ToastProvider usage in providers.tsx is passing toastManager and timeout even though ToastProvider already hardcodes those values internally after spreading props, so these inputs are redundant and misleading. Update the providers.tsx render of ToastProvider to pass only children (and any truly configurable props), and rely on ToastProvider’s implementation in toast.tsx for toastManager and timeout so the call site stays aligned with the component’s actual behavior.apps/figma-token-review-plugin/src/ui/lib/evaluate/dimension.ts (1)
1-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNear-identical logic duplicated across dimension.ts, radius.ts, and shadow.ts.
All three evaluators share the same raw→
token-not-used/unknown-or-missing→unknown-token/else→conformant shape, differing only in thepropertyliteral and message text. Consider extracting a shared generic helper (e.g.evaluateTokenBinding(usages, schema, property, propertyLabel)) to avoid triple-maintaining this logic as rules evolve.♻️ Sketch of a shared helper
function evaluateTokenBinding<T extends { nodeId: string; name: string; token: string | null; value: unknown; tokenStatus: string }>( usages: T[], schema: TokenValueIndex, property: Property, propertyLabel: string, ): { violations: Violation[]; conformant: Conformant[] } { const violations: Violation[] = []; const conformant: Conformant[] = []; for (const u of usages) { const base = { nodeId: u.nodeId, name: u.name, property, token: u.token, value: u.value, origin: 'rule' as const, message: '', suggested: [] as string[] }; if (u.tokenStatus === 'raw') { violations.push({ ...base, type: 'token-not-used', severity: 'high', message: `${propertyLabel}에 raw value(${u.value})가 직접 입력되었습니다.` }); continue; } if (u.tokenStatus === 'unknown' || !u.token || !(u.token in schema.tokens)) { violations.push({ ...base, type: 'unknown-token', severity: 'high', message: `${propertyLabel} 스키마에 등록되지 않은 토큰입니다.` }); continue; } conformant.push({ nodeId: u.nodeId, name: u.name, property, token: u.token }); } return { violations, conformant }; }The same concern applies to
radius.tsandshadow.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/ui/lib/evaluate/dimension.ts` around lines 1 - 47, The raw/unknown/conformant branching in evaluateDimension is duplicated in the sibling evaluators, so extract it into a shared helper and reuse it. Introduce a generic function such as evaluateTokenBinding that takes usages, schema, the property literal, and the localized label/message text, then have evaluateDimension (and the matching radius.ts and shadow.ts evaluators) call it instead of maintaining near-identical loops. Keep the unique behavior in the caller only where the property-specific messages differ.apps/figma-token-review-plugin/src/ui/lib/recommend.ts (1)
203-219: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
metalookup is always undefined forprimitive-usedviolations.
primitive-usedis only ever emitted (perevaluateColor) whentokenis a primitive key that is not present inschema.semantic, soctx.colorSchema.semantic[token]here will always resolve toundefined. The code happens to work only because of themeta?.hex ?? valuefallback to the rawvaluefield. Simplify by usingvaluedirectly to avoid the misleading dead lookup.♻️ Suggested simplification
case 'primitive-used': { - // Same hex + scope semantic only (no primitive fallback) - const meta = token ? ctx.colorSchema.semantic[token] : null; - const hex = meta?.hex ?? value; + // token is a primitive key by definition, never in schema.semantic — use value (raw hex) directly + const hex = value; const allowedRoles =🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/ui/lib/recommend.ts` around lines 203 - 219, The `primitive-used` branch in `recommend.ts` has a dead `meta` lookup because `evaluateColor` only emits this violation for primitive keys that are not in `schema.semantic`. Remove the unused `ctx.colorSchema.semantic[token]` access and use `value` directly when deriving `hex`, keeping the rest of the `primitive-used` candidate filtering logic in `recommend` unchanged.apps/figma-token-review-plugin/src/ui/lib/evaluate/color.test.ts (1)
40-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSilent no-op skips instead of real test failures.
The
do-not-use(Lines 40-57),role-mismatch(Lines 59-74), andconformant(Lines 85-103) tests all useif (!key) return;when the expected schema entry isn't found. Vitest does not fail a test with zero assertions — an earlyreturnmakes these tests silently pass without exercisingevaluateColorat all if the schema ever stops containing a matching token.The test fails only if the function throws an error. An empty function doesn't throw an error.
Prefer failing loudly (e.g.
expect(key).toBeDefined()or throwing, as already done viafindBgHexinrecommend.test.ts) so schema/data regressions are caught instead of masked.♻️ Suggested pattern
- if (!doNotUseKey) return; // 스키마에 없으면 skip + expect(doNotUseKey).toBeDefined();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/ui/lib/evaluate/color.test.ts` around lines 40 - 103, The color tests are silently skipping when the expected schema token is missing because `doNotUseKey` and `fgKey` use early `return`, so regressions can pass without exercising `evaluateColor`. Update the affected tests in `evaluate/color.test.ts` to fail loudly instead of returning, using an assertion or explicit throw before calling `evaluateColor`. Apply this to the `do-not-use`, `fill` role-mismatch, and conformant cases so missing schema entries break the test rather than no-op.apps/figma-token-review-plugin/src/ui/lib/recommend.test.ts (1)
56-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame silent-skip pattern as
color.test.ts.Both
if (!testHex || !expectedPrimKey) return;(Lines 56-97) andif (!fgEntry) return;(Lines 149-175) will let these tests pass with zero assertions executed if the schema doesn't contain a matching fixture, unlikefindBgHex()above (Lines 30-40) which correctly throws. Consider using the same throwing helper pattern here for consistency and to avoid false-positive coverage.The test fails only if the function throws an error. An empty function doesn't throw an error.
Also applies to: 149-175
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/figma-token-review-plugin/src/ui/lib/recommend.test.ts` around lines 56 - 97, The tests in recommend.test.ts use silent early returns in the fallback-path cases, which can let them pass without running any assertions. Replace the `if (!testHex || !expectedPrimKey) return;` and `if (!fgEntry) return;` guards in the affected test blocks with the same throwing helper pattern used by `findBgHex()`, so the tests fail when no matching fixture is found. Keep the logic centered around `applyRecommendations`, `colorSchema`, and the helper lookup code, but ensure missing schema matches raise an error instead of skipping the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/figma-token-review-plugin/src/common/tokens/semantic-color.dark.json`:
- Around line 47-86: The dark-theme contrast background tokens are inverted: the
lower-emphasis and higher-emphasis values in the contrast group are mapped to
the wrong grays. Update the `contrast` entries so `background.contrast.100` uses
the subtler dark value and `background.contrast.200` uses the more visually
distinct value, keeping the mapping aligned with the corresponding light-theme
semantics.
In `@apps/figma-token-review-plugin/src/plugin/handlers/extract.ts`:
- Around line 408-427: The extractPaints helper in extract.ts applies the
visible check only to raw SOLID paints, but bound paint entries are always
pushed even when their corresponding paint is hidden. Update the bound-entry
loop in extractPaints to consult the matching paint’s visible flag before
calling walk and pushColor, so hidden fills/strokes are skipped consistently
with the raw-value path.
- Around line 185-201: The classifyBackground helper currently only treats solid
ancestor fills as backgrounds, so visible non-solid fills like gradients or
images are missed and text can be misclassified as transparent. Update
classifyBackground in extract.ts to consider any visible ancestor fill (not just
SOLID) as a background and return ambiguous when such a fill is present, while
preserving the existing solid white/other handling for solid fills.
- Around line 363-365: Remove the active-page switch in the extraction flow: the
PAGE lookup around pageNode and the subsequent figma.setCurrentPageAsync call in
extract.ts are unnecessary because figma.getNodeByIdAsync plus the later
extract/screenshot traversal already use dynamic-page access. Keep the pageNode
resolution if needed for context, but delete the current-page navigation so
extraction no longer moves the user away from their canvas.
In `@apps/figma-token-review-plugin/src/plugin/handlers/focus.ts`:
- Around line 12-42: The focus handler in focus.ts applies selection and
viewport changes even when the async response is stale, so copy the
latest-request guard pattern used in scan.ts before mutating
figma.currentPage.selection and calling figma.viewport.scrollAndZoomIntoView.
Use the existing requestId-based check in the same focus flow (around the
resolved/missing handling) to return early for outdated requests, so only the
most recent focus action can update the canvas.
In `@apps/figma-token-review-plugin/src/plugin/handlers/scan.ts`:
- Around line 15-26: The figma node lookup in scanHandler is outside the error
handling path, so a thrown getNodeByIdAsync error can reject without notifying
the UI. Move figma.getNodeByIdAsync(msg.frameId) into the existing try/catch in
scan.ts and, on failure, send an extract-error via postToUi with the current
requestId so the loading state can clear. Keep the activeRequestId check and the
node validation logic in scanHandler, but ensure any lookup exception is handled
there rather than bubbling to messages.ts start().
In `@apps/figma-token-review-plugin/src/ui/components/error-boundary.tsx`:
- Line 24: The reset() method in ErrorBoundary currently only clears the local
boundary state, so the same broken subtree can immediately crash again. Update
reset() to also clear or reinitialize the underlying crash source in the
relevant store(s) such as scanStore, or navigate the UI to a known-good state
before clearing the error state. Use the existing ErrorBoundary.reset method and
the scanStore dependency to locate the fix.
In `@apps/figma-token-review-plugin/src/ui/features/llm/client.ts`:
- Around line 61-86: The timeout handling in the request flow inside the LLM
client currently aborts with a custom Error reason, so the catch block in the
fetch logic never reliably maps timeout failures to LlmTimeoutError. Update the
abort/timeout handling in the client method that builds the AbortController and
calls fetchImpl so timeouts are tracked explicitly or the abort reason is
checked before rethrowing, and ensure callers still receive LlmTimeoutError for
timeout-triggered aborts while preserving non-timeout errors.
In `@apps/figma-token-review-plugin/src/ui/features/llm/merge.ts`:
- Around line 104-139: The mergeScanPayload/buildOutput path is keeping
deterministic conformant entries unchanged even when LLM FAIL heuristics add
violations for the same nodes, which leaves those nodes counted in both lists.
Update buildOutput in mergeScanPayload so it reconciles d.conformant against the
extra LLM-derived violations and removes any failed nodes from the returned
conformant list before computing summary, while keeping the merged violations
intact. Add a regression test in merge.test.ts for a deterministically
conformant node that receives an LLM FAIL verdict to verify it is excluded from
conformant.
In `@apps/figma-token-review-plugin/src/ui/features/scan/store.ts`:
- Around line 39-53: The empty-check in scanStore.result only considers color
and typography, so scans with violations in space, dimension, borderRadius, or
shadow can be incorrectly marked clean and lose their payload. Update the
result() logic to compute emptiness from every violation-bearing category on
ScanPayload, using the existing ScanPayload fields and the result() method in
scanStore so any violation in any category keeps the state as success and
preserves the payload.
In `@apps/figma-token-review-plugin/src/ui/lib/evaluate/color.ts`:
- Around line 79-80: In color.ts’s evaluator, the early return on null u.token
is incorrectly dropping usages when tokenStatus is still ok, so they never get
reported. Update the color evaluation logic to mirror the sibling evaluators
(like dimension/radius/shadow) by classifying token-null cases as unknown-token
instead of continuing silently, and verify the relevant function in this file
records the usage in the report rather than skipping it.
In `@apps/figma-token-review-plugin/src/ui/lib/loaders/dimension.ts`:
- Around line 27-34: Normalize shadow object values before they are used as map
keys by updating the stringify helper in the dimension loader so object
serialization is deterministic. The current handling in stringify falls back to
JSON.stringify for shadow-like objects, which can vary with property order;
adjust the logic used by valueToTokens to sort object keys before serialization
so equivalent shadow values always produce the same key.
In `@apps/figma-token-review-plugin/src/ui/lib/loaders/typography.ts`:
- Around line 47-68: The hierarchy rank is still coming from
Object.entries(root.textStyle) iteration order in loadTextStyleSchema, which
makes TextStyleMeta.rank and downstream rubric.textStyle[].rank behavior depend
on object key ordering. Update the typography loader to use an explicit rank
from the token data (or add validation for the expected sequence) when building
the styles map, and keep the rank tied to the TextStyleSchema/TextStyleMeta flow
rather than insertion order.
In `@apps/figma-token-review-plugin/src/ui/pages/scan-result.tsx`:
- Around line 213-238: The ViolationPanel component currently hides the empty
state when passJudgments exists, but it never renders any pass-only content, so
a scan with only passJudgments ends up blank. Update ViolationPanel to either
render a pass summary when passJudgments is present or keep showing EmptyState
whenever violations.length is 0, and make sure the logic around splitByKind,
ViolationSection, and hasPass stays consistent with the intended pass-only
typography flow.
In `@apps/figma-token-review-plugin/vite.config.plugin.ts`:
- Around line 7-10: The alias root in the Vite plugin config uses __dirname even
though this file runs as ESM, so the `resolve.alias` setup for `~` can break
during build. Update the alias root in `vite.config.plugin.ts` to use
`import.meta.dirname` inside the existing `path.resolve` call so the `~/*`
mapping stays stable. Keep the change localized to the `resolve.alias` entry in
the config.
---
Nitpick comments:
In `@apps/figma-token-review-plugin/ARCHITECTURE.md`:
- Line 3: The fenced code blocks in ARCHITECTURE.md are missing language
identifiers and are triggering MD040. Update the affected fenced blocks that
render the tree diagrams by adding an appropriate language hint to each fence,
including the other two referenced fenced blocks, while keeping the existing
content unchanged.
In `@apps/figma-token-review-plugin/src/plugin/handlers/extract.test.ts`:
- Around line 85-129: The walkTree test currently verifies only that nodes are
present, so it can miss a sibling-order regression caused by the stack/pop
traversal in walkTree. Update this test in extract.test.ts to assert the exact
traversal order for siblings at the same level, using the existing walkTree,
root, skipped, and leaf setup so a change in declared child order or stack
handling will fail. Keep the current parent/child and skipped-subtree
assertions, but add an order check that distinguishes the first and second child
visitation.
In `@apps/figma-token-review-plugin/src/plugin/handlers/extract.ts`:
- Around line 692-696: The screenshot and tree traversal failures in extract’s
Promise.all are being swallowed, which can silently degrade llmContext without
any diagnostics. Update the captureScreenshot and walkTree error handling in
extract to log the caught error before falling back to an empty string or empty
array, so the LLM context issues are traceable while preserving the current
fallback behavior.
In `@apps/figma-token-review-plugin/src/plugin/handlers/focus.ts`:
- Around line 12-20: The node lookup loop in the focus handler is doing
independent getNodeByIdAsync calls sequentially, which adds unnecessary latency.
Update the focus.ts logic that builds resolved and missing so it resolves
msg.nodeIds in parallel with Promise.all, then filters each result into resolved
or missing while preserving the existing node-type and visibility checks.
In `@apps/figma-token-review-plugin/src/ui/components/hero-panel.tsx`:
- Around line 5-11: The Root component in hero-panel.tsx is using a hardcoded
bg-white class instead of the shared vapor design token style used elsewhere.
Update the Box wrapper in Root to use the same token-based background approach
as components like scan-result.tsx, replacing the raw Tailwind color with the
appropriate $css token so the styling stays consistent across the plugin UI.
In `@apps/figma-token-review-plugin/src/ui/components/resize-handler.tsx`:
- Around line 97-107: The resize handle in resize-handler.tsx is pointer-only
even though it uses role="separator"; make it keyboard-accessible by adding a
focusable tabIndex and an onKeyDown handler on the separator element. Use the
existing resize logic from handlePointerDown/handlePointerMove/handlePointerUp
to support keyboard resizing (for example via arrow keys), and keep the
aria-orientation and aria-label in sync with the interactive behavior.
- Around line 48-95: The resize drag logic in handlePointerMove and
handlePointerUp only enforces minimum bounds, so add a maximum size clamp before
queueResize and postToCode to prevent unbounded growth. Also verify the drag
baseline in handlePointerDown, which currently uses window.innerWidth and
window.innerHeight, remains correct with the synchronous behavior in the resize
handler path; if not, switch to the canonical stored size used by the resize
flow instead of reading the window size at pointerdown.
In
`@apps/figma-token-review-plugin/src/ui/components/violation-card/violation-card.tsx`:
- Around line 41-49: The hex check in resolveHex duplicates logic that already
exists in utils.ts and uses a slightly different pattern, which can lead to
inconsistent behavior. Update violation-card.tsx to reuse the shared hex
validator from utils.ts inside resolveHex instead of inlining the regex, and
ensure the shared helper is the single source of truth for hex detection across
this feature.
- Around line 41-49: The schema load in resolveHex is being repeated for every
ViolationCard render, which causes loadColorSchema(schemaMode) to re-flatten
static data unnecessarily. Update resolveHex and its call sites so the
ColorSchema is memoized per SchemaMode, either by caching inside loadColorSchema
or by loading it once in a parent like ViolationCard/ScanResultPage and passing
it down, so each mode is computed only once instead of per card render.
In `@apps/figma-token-review-plugin/src/ui/lib/evaluate/color.test.ts`:
- Around line 40-103: The color tests are silently skipping when the expected
schema token is missing because `doNotUseKey` and `fgKey` use early `return`, so
regressions can pass without exercising `evaluateColor`. Update the affected
tests in `evaluate/color.test.ts` to fail loudly instead of returning, using an
assertion or explicit throw before calling `evaluateColor`. Apply this to the
`do-not-use`, `fill` role-mismatch, and conformant cases so missing schema
entries break the test rather than no-op.
In `@apps/figma-token-review-plugin/src/ui/lib/evaluate/dimension.ts`:
- Around line 1-47: The raw/unknown/conformant branching in evaluateDimension is
duplicated in the sibling evaluators, so extract it into a shared helper and
reuse it. Introduce a generic function such as evaluateTokenBinding that takes
usages, schema, the property literal, and the localized label/message text, then
have evaluateDimension (and the matching radius.ts and shadow.ts evaluators)
call it instead of maintaining near-identical loops. Keep the unique behavior in
the caller only where the property-specific messages differ.
In `@apps/figma-token-review-plugin/src/ui/lib/loaders/color.ts`:
- Line 7: The NS namespace constant is duplicated across the color and
typography loaders, so centralize it in a shared helper/constant instead of
redefining it in each loader. Move the `NS` value and any repeated
`$extensions[NS]` access pattern into a common module such as
`common/schemas.ts` or a new shared vapor meta helper, then update the relevant
loader functions (including `color.ts` and `typography.ts`) to use the shared
symbol.
In `@apps/figma-token-review-plugin/src/ui/lib/loaders/typography.ts`:
- Line 4: The typography loader defines the same NS extension namespace constant
and extraction pattern already present in color.ts, so consolidate this shared
value into a common module and import it here instead of duplicating it. Update
the typography loader to use the shared namespace symbol (for example, the
existing NS reference used by the color loader) and keep the extraction logic
unchanged so both loaders source the namespace from one place.
In `@apps/figma-token-review-plugin/src/ui/lib/recommend.test.ts`:
- Around line 56-97: The tests in recommend.test.ts use silent early returns in
the fallback-path cases, which can let them pass without running any assertions.
Replace the `if (!testHex || !expectedPrimKey) return;` and `if (!fgEntry)
return;` guards in the affected test blocks with the same throwing helper
pattern used by `findBgHex()`, so the tests fail when no matching fixture is
found. Keep the logic centered around `applyRecommendations`, `colorSchema`, and
the helper lookup code, but ensure missing schema matches raise an error instead
of skipping the test.
In `@apps/figma-token-review-plugin/src/ui/lib/recommend.ts`:
- Around line 203-219: The `primitive-used` branch in `recommend.ts` has a dead
`meta` lookup because `evaluateColor` only emits this violation for primitive
keys that are not in `schema.semantic`. Remove the unused
`ctx.colorSchema.semantic[token]` access and use `value` directly when deriving
`hex`, keeping the rest of the `primitive-used` candidate filtering logic in
`recommend` unchanged.
In `@apps/figma-token-review-plugin/src/ui/providers.tsx`:
- Around line 12-16: The ToastProvider usage in providers.tsx is passing
toastManager and timeout even though ToastProvider already hardcodes those
values internally after spreading props, so these inputs are redundant and
misleading. Update the providers.tsx render of ToastProvider to pass only
children (and any truly configurable props), and rely on ToastProvider’s
implementation in toast.tsx for toastManager and timeout so the call site stays
aligned with the component’s actual behavior.
In `@apps/figma-token-review-plugin/vite.config.ui.ts`:
- Around line 14-22: The build config mixes cwd-relative paths with a
__dirname-relative alias, so the ui Vite setup can resolve input/output
incorrectly when run from a different working directory. Update the build
section in vite.config.ui.ts to resolve outDir and rollupOptions.input from the
same base as the alias (using __dirname consistently) so path resolution is
stable regardless of where the config is invoked. Keep the existing build and
rollupOptions structure, just make the path construction consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 85858c8c-7956-4651-ad37-922ab8759fa9
⛔ Files ignored due to path filters (3)
apps/figma-token-review-plugin/src/ui/assets/confirm.svgis excluded by!**/*.svgapps/figma-token-review-plugin/src/ui/assets/survey.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (99)
.gitignoreapps/figma-token-review-plugin/.env.exampleapps/figma-token-review-plugin/.gitignoreapps/figma-token-review-plugin/ARCHITECTURE.mdapps/figma-token-review-plugin/eslint.config.mjsapps/figma-token-review-plugin/index.htmlapps/figma-token-review-plugin/manifest.jsonapps/figma-token-review-plugin/package.jsonapps/figma-token-review-plugin/src/common/messages.tsapps/figma-token-review-plugin/src/common/schemas.tsapps/figma-token-review-plugin/src/common/tokens/MANIFEST.jsonapps/figma-token-review-plugin/src/common/tokens/border-radius.jsonapps/figma-token-review-plugin/src/common/tokens/dimension.jsonapps/figma-token-review-plugin/src/common/tokens/primitive-color.dark.jsonapps/figma-token-review-plugin/src/common/tokens/primitive-color.light.jsonapps/figma-token-review-plugin/src/common/tokens/semantic-color.dark.jsonapps/figma-token-review-plugin/src/common/tokens/semantic-color.light.jsonapps/figma-token-review-plugin/src/common/tokens/shadow.jsonapps/figma-token-review-plugin/src/common/tokens/space.jsonapps/figma-token-review-plugin/src/common/tokens/text-style.jsonapps/figma-token-review-plugin/src/common/tokens/typography.jsonapps/figma-token-review-plugin/src/plugin/code.tsapps/figma-token-review-plugin/src/plugin/handlers/extract.test.tsapps/figma-token-review-plugin/src/plugin/handlers/extract.tsapps/figma-token-review-plugin/src/plugin/handlers/focus.tsapps/figma-token-review-plugin/src/plugin/handlers/resize.tsapps/figma-token-review-plugin/src/plugin/handlers/scan.tsapps/figma-token-review-plugin/src/plugin/handlers/selection.tsapps/figma-token-review-plugin/src/plugin/messages.tsapps/figma-token-review-plugin/src/ui/App.tsxapps/figma-token-review-plugin/src/ui/components/error-boundary.tsxapps/figma-token-review-plugin/src/ui/components/hero-panel.tsxapps/figma-token-review-plugin/src/ui/components/loader.tsxapps/figma-token-review-plugin/src/ui/components/resize-handler.tsxapps/figma-token-review-plugin/src/ui/components/toast.tsxapps/figma-token-review-plugin/src/ui/components/violation-card/index.tsapps/figma-token-review-plugin/src/ui/components/violation-card/token-comparison.tsxapps/figma-token-review-plugin/src/ui/components/violation-card/utils.tsapps/figma-token-review-plugin/src/ui/components/violation-card/violation-breadcrumb.tsxapps/figma-token-review-plugin/src/ui/components/violation-card/violation-card.tsxapps/figma-token-review-plugin/src/ui/components/violation-card/violation-detail.tsxapps/figma-token-review-plugin/src/ui/features/llm/client.tsapps/figma-token-review-plugin/src/ui/features/llm/index.tsapps/figma-token-review-plugin/src/ui/features/llm/merge.test.tsapps/figma-token-review-plugin/src/ui/features/llm/merge.tsapps/figma-token-review-plugin/src/ui/features/llm/parse.test.tsapps/figma-token-review-plugin/src/ui/features/llm/parse.tsapps/figma-token-review-plugin/src/ui/features/llm/prompt.test.tsapps/figma-token-review-plugin/src/ui/features/llm/prompt.tsapps/figma-token-review-plugin/src/ui/features/messaging/bridge.tsapps/figma-token-review-plugin/src/ui/features/messaging/evaluate.tsapps/figma-token-review-plugin/src/ui/features/messaging/focus.tsapps/figma-token-review-plugin/src/ui/features/messaging/index.tsapps/figma-token-review-plugin/src/ui/features/messaging/request-selection.tsapps/figma-token-review-plugin/src/ui/features/scan/index.tsapps/figma-token-review-plugin/src/ui/features/scan/store.tsapps/figma-token-review-plugin/src/ui/features/scan/use-scan.tsapps/figma-token-review-plugin/src/ui/features/selection/index.tsapps/figma-token-review-plugin/src/ui/features/selection/plugins.tsapps/figma-token-review-plugin/src/ui/features/selection/store.tsapps/figma-token-review-plugin/src/ui/features/selection/use-selection.tsapps/figma-token-review-plugin/src/ui/index.cssapps/figma-token-review-plugin/src/ui/lib/evaluate/color.test.tsapps/figma-token-review-plugin/src/ui/lib/evaluate/color.tsapps/figma-token-review-plugin/src/ui/lib/evaluate/dimension.test.tsapps/figma-token-review-plugin/src/ui/lib/evaluate/dimension.tsapps/figma-token-review-plugin/src/ui/lib/evaluate/radius.test.tsapps/figma-token-review-plugin/src/ui/lib/evaluate/radius.tsapps/figma-token-review-plugin/src/ui/lib/evaluate/shadow.test.tsapps/figma-token-review-plugin/src/ui/lib/evaluate/shadow.tsapps/figma-token-review-plugin/src/ui/lib/evaluate/space.test.tsapps/figma-token-review-plugin/src/ui/lib/evaluate/space.tsapps/figma-token-review-plugin/src/ui/lib/evaluate/typography.test.tsapps/figma-token-review-plugin/src/ui/lib/evaluate/typography.tsapps/figma-token-review-plugin/src/ui/lib/loaders/color.test.tsapps/figma-token-review-plugin/src/ui/lib/loaders/color.tsapps/figma-token-review-plugin/src/ui/lib/loaders/dimension.test.tsapps/figma-token-review-plugin/src/ui/lib/loaders/dimension.tsapps/figma-token-review-plugin/src/ui/lib/loaders/typography.test.tsapps/figma-token-review-plugin/src/ui/lib/loaders/typography.tsapps/figma-token-review-plugin/src/ui/lib/recommend.test.tsapps/figma-token-review-plugin/src/ui/lib/recommend.tsapps/figma-token-review-plugin/src/ui/lib/rubric.test.tsapps/figma-token-review-plugin/src/ui/lib/rubric.tsapps/figma-token-review-plugin/src/ui/lib/scope.test.tsapps/figma-token-review-plugin/src/ui/lib/scope.tsapps/figma-token-review-plugin/src/ui/main.tsxapps/figma-token-review-plugin/src/ui/pages/home.tsxapps/figma-token-review-plugin/src/ui/pages/scan-result.tsxapps/figma-token-review-plugin/src/ui/pages/success.tsxapps/figma-token-review-plugin/src/ui/providers.tsxapps/figma-token-review-plugin/src/ui/shared/create-store.tsapps/figma-token-review-plugin/src/vite-env.d.tsapps/figma-token-review-plugin/tsconfig.app.jsonapps/figma-token-review-plugin/tsconfig.jsonapps/figma-token-review-plugin/tsconfig.node.jsonapps/figma-token-review-plugin/vite.config.plugin.tsapps/figma-token-review-plugin/vite.config.ui.tsapps/figma-token-review-plugin/vitest.config.ts
| "contrast": { | ||
| "100": { | ||
| "$description": "The subtle contrast background to intentionally draw attention to content by contrasting with the surrounding page surface.", | ||
| "$extensions": { | ||
| "io.goorm.vapor": { | ||
| "when": [ | ||
| "background of non-interactive element requiring intentional visual contrast against the page", | ||
| "background of informational element that must stand out from the surrounding content" | ||
| ], | ||
| "avoid": [ | ||
| "elements requiring stronger visual separation → colors.background.contrast.200" | ||
| ], | ||
| "accessibility": { | ||
| "role": "background" | ||
| } | ||
| } | ||
| }, | ||
| "$value": "{colors.gray.800}" | ||
| }, | ||
| "200": { | ||
| "$description": "The solid high-emphasis background for elements requiring strong visual separation from the surrounding UI.", | ||
| "$extensions": { | ||
| "io.goorm.vapor": { | ||
| "when": [ | ||
| "background of element that must be visually distinct from all surrounding content", | ||
| "high-emphasis background for elements requiring immediate user attention", | ||
| "background of elements providing temporary or contextual information above the page layer" | ||
| ], | ||
| "avoid": [ | ||
| "elements requiring only subtle contrast → colors.background.contrast.100", | ||
| "interactive variant-specific backgrounds → use variant-specific background tokens" | ||
| ], | ||
| "accessibility": { | ||
| "role": "background" | ||
| } | ||
| } | ||
| }, | ||
| "$value": "{colors.gray.300}" | ||
| } | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
background.contrast.100 / .200 look reversed in the dark theme. 100 maps to gray.800 while 200 maps to gray.300; swap them so the higher-emphasis token stays more visually distinct and matches the light-theme counterpart.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/figma-token-review-plugin/src/common/tokens/semantic-color.dark.json`
around lines 47 - 86, The dark-theme contrast background tokens are inverted:
the lower-emphasis and higher-emphasis values in the contrast group are mapped
to the wrong grays. Update the `contrast` entries so `background.contrast.100`
uses the subtler dark value and `background.contrast.200` uses the more visually
distinct value, keeping the mapping aligned with the corresponding light-theme
semantics.
| // token 이 없으면 판정 불가 — 건너뜀 | ||
| if (!u.token) continue; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Silent drop instead of unknown-token for null-token usages.
When tokenStatus === 'ok' but u.token is null, this continues without recording either a violation or a conformant entry — the usage vanishes from the report. Compare this to the sibling evaluators, which explicitly classify this exact case as unknown-token:
if (u.tokenStatus === 'unknown' || !u.token || !(u.token in schema.tokens)) in dimension.ts, and radius/shadow tests explicitly assert 'token null + tokenStatus ok → unknown-token'. There's no equivalent test for color.ts, suggesting this was overlooked rather than intentional. Silently dropping usages means real color-binding gaps are hidden from the scan report instead of surfaced.
🐛 Proposed fix
- // token 이 없으면 판정 불가 — 건너뜀
- if (!u.token) continue;
+ // token 이 없으면 unknown-token 으로 기록 (다른 evaluator와 동일하게)
+ if (!u.token) {
+ violations.push({
+ ...base,
+ type: 'unknown-token',
+ severity: 'high',
+ message: '바인딩된 토큰 정보를 확인할 수 없습니다.',
+ });
+ continue;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // token 이 없으면 판정 불가 — 건너뜀 | |
| if (!u.token) continue; | |
| // token 이 없으면 unknown-token 으로 기록 (다른 evaluator와 동일하게) | |
| if (!u.token) { | |
| violations.push({ | |
| ...base, | |
| type: 'unknown-token', | |
| severity: 'high', | |
| message: '바인딩된 토큰 정보를 확인할 수 없습니다.', | |
| }); | |
| continue; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/figma-token-review-plugin/src/ui/lib/evaluate/color.ts` around lines 79
- 80, In color.ts’s evaluator, the early return on null u.token is incorrectly
dropping usages when tokenStatus is still ok, so they never get reported. Update
the color evaluation logic to mirror the sibling evaluators (like
dimension/radius/shadow) by classifying token-null cases as unknown-token
instead of continuing silently, and verify the relevant function in this file
records the usage in the report rather than skipping it.
| function stringify(v: string | object): string { | ||
| if (typeof v === 'string') return v; | ||
| if (typeof v === 'object' && v !== null && 'value' in v && 'unit' in v) { | ||
| const dv = v as DimensionValue; | ||
| return `${dv.value}${dv.unit}`; | ||
| } | ||
| return JSON.stringify(v); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n== dimension.ts ==\n'
cat -n apps/figma-token-review-plugin/src/ui/lib/loaders/dimension.ts | sed -n '1,220p'
printf '\n== search valueToTokens / stringify / shadow ==\n'
rg -n "valueToTokens|stringify\(|shadow|JSON\.stringify" apps/figma-token-review-plugin/src/ui/lib -SRepository: goorm-dev/vapor-ui
Length of output: 11854
🏁 Script executed:
set -euo pipefail
printf '\n== shadow.json ==\n'
wc -l apps/common/tokens/shadow.json
cat -n apps/common/tokens/shadow.json | sed -n '1,240p'
printf '\n== shadow loader/tests context ==\n'
cat -n apps/figma-token-review-plugin/src/ui/lib/loaders/dimension.test.ts | sed -n '1,220p'
cat -n apps/figma-token-review-plugin/src/ui/lib/evaluate/shadow.test.ts | sed -n '1,220p'Repository: goorm-dev/vapor-ui
Length of output: 237
🏁 Script executed:
set -euo pipefail
printf '\n== locate shadow.json ==\n'
fd -a 'shadow.json$' .
printf '\n== locate common/tokens files ==\n'
fd -a 'tokens' apps -t dRepository: goorm-dev/vapor-ui
Length of output: 550
🏁 Script executed:
set -euo pipefail
printf '\n== locate shadow.json ==\n'
fd -a '^shadow\.json$' .
printf '\n== locate token directories ==\n'
fd -a 'tokens' apps -t d
printf '\n== inspect shadow loader/tests if found ==\n'
for f in $(fd -a '^shadow\.json$' .); do
echo "--- $f"
wc -l "$f"
cat -n "$f" | sed -n '1,220p'
doneRepository: goorm-dev/vapor-ui
Length of output: 8372
🏁 Script executed:
set -euo pipefail
printf '\n== search for shadow token generation / normalization ==\n'
rg -n "shadow.*sort|sort.*shadow|canonical|normalize.*shadow|JSON\.stringify\(.*shadow|valueToTokens|get\(value\)" apps/figma-token-review-plugin/src -S
printf '\n== inspect recommend.ts for downstream semantics ==\n'
cat -n apps/figma-token-review-plugin/src/ui/lib/recommend.ts | sed -n '1,260p'Repository: goorm-dev/vapor-ui
Length of output: 14209
Normalize shadow values before using them as map keys. valueToTokens uses JSON.stringify(v) for shadow objects, so the same shadow content with a different property order will be treated as a different value. Sorting keys first would make duplicate detection stable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/figma-token-review-plugin/src/ui/lib/loaders/dimension.ts` around lines
27 - 34, Normalize shadow object values before they are used as map keys by
updating the stringify helper in the dimension loader so object serialization is
deterministic. The current handling in stringify falls back to JSON.stringify
for shadow-like objects, which can vary with property order; adjust the logic
used by valueToTokens to sort object keys before serialization so equivalent
shadow values always produce the same key.
| export function loadTextStyleSchema(): TextStyleSchema { | ||
| const root = textStyleRaw as { textStyle?: Record<string, Node | unknown> }; | ||
| const fontSizeTable = loadFontSizeTable(); | ||
| const order: string[] = []; | ||
| const styles: Record<string, TextStyleMeta> = {}; | ||
| const entries = Object.entries(root.textStyle ?? {}); | ||
| for (const [name, node] of entries) { | ||
| if (name.startsWith('$') || typeof node !== 'object' || !node) continue; | ||
| const meta = ((node as Node).$extensions?.[NS] ?? {}) as Record<string, unknown>; | ||
| const alias = (node as Node).$value?.fontSize; | ||
| styles[name] = { | ||
| rank: order.length, | ||
| when: Array.isArray(meta.when) ? (meta.when as string[]) : [], | ||
| avoid: Array.isArray(meta.avoid) ? (meta.avoid as string[]) : [], | ||
| description: | ||
| typeof (node as Node).$description === 'string' | ||
| ? ((node as Node).$description ?? null) | ||
| : null, | ||
| fontSize: resolveFontSize(alias, fontSizeTable), | ||
| }; | ||
| order.push(name); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant files and symbols.
git ls-files | rg 'apps/figma-token-review-plugin/src/ui/lib/loaders/typography\.ts|text-style\.json|TextStyleSchema|TextStyleMeta|rank|order'
printf '\n--- outline typography.ts ---\n'
ast-grep outline apps/figma-token-review-plugin/src/ui/lib/loaders/typography.ts --view expanded || true
printf '\n--- text-style.json locations ---\n'
fd -a 'text-style.json' apps/figma-token-review-plugin || true
printf '\n--- symbol/search hits ---\n'
rg -n "TextStyleSchema|TextStyleMeta|rank:|\\brank\\b|\\border\\b|textStyle" apps/figma-token-review-plugin/src/ui -g '!**/*.map' || trueRepository: goorm-dev/vapor-ui
Length of output: 16400
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the loader file and nearby types/consumers in small slices.
printf '\n--- typography.ts ---\n'
wc -l apps/figma-token-review-plugin/src/ui/lib/loaders/typography.ts
cat -n apps/figma-token-review-plugin/src/ui/lib/loaders/typography.ts | sed -n '1,220p'
printf '\n--- related type definitions and consumers ---\n'
rg -n "type TextStyleSchema|interface TextStyleSchema|type TextStyleMeta|interface TextStyleMeta|rank|order" apps/figma-token-review-plugin/src/ui -g '!**/*.map' -A 3 -B 3 || true
printf '\n--- text-style.json if present ---\n'
fd -a 'text-style.json' apps/figma-token-review-plugin || trueRepository: goorm-dev/vapor-ui
Length of output: 49831
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search downstream consumers and the source JSON ordering.
rg -n "loadTextStyleSchema|TextStyleSchema|TextStyleMeta|rank|order|neighbor|hierarchy|semantic" apps/figma-token-review-plugin/src/ui -g '!**/*.map' -A 2 -B 2 || true
printf '\n--- typography.ts lines ---\n'
sed -n '1,220p' apps/figma-token-review-plugin/src/ui/lib/loaders/typography.ts
printf '\n--- candidate JSON files ---\n'
fd -a 'text-style.json' .Repository: goorm-dev/vapor-ui
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read only the relevant small slices.
printf '\n--- typography.ts excerpt ---\n'
sed -n '1,140p' apps/figma-token-review-plugin/src/ui/lib/loaders/typography.ts
printf '\n--- schema/type definitions ---\n'
rg -n "type TextStyleSchema|interface TextStyleSchema|type TextStyleMeta|interface TextStyleMeta" apps/figma-token-review-plugin/src/ui -A 8 -B 2
printf '\n--- references to rank/order consumers ---\n'
rg -n "\.rank\b|\border\b|TextStyleMeta|TextStyleSchema" apps/figma-token-review-plugin/src/ui -A 4 -B 4Repository: goorm-dev/vapor-ui
Length of output: 33291
Hierarchy rank is still derived from Object.entries(root.textStyle) order. TextStyleMeta.rank and rubric.textStyle[].rank feed the hierarchy checks, so reordering text-style.json changes behavior. Make the rank explicit in the token data or validate the expected sequence instead of relying on object key order.
[medium_effort_and_high_reward]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/figma-token-review-plugin/src/ui/lib/loaders/typography.ts` around lines
47 - 68, The hierarchy rank is still coming from Object.entries(root.textStyle)
iteration order in loadTextStyleSchema, which makes TextStyleMeta.rank and
downstream rubric.textStyle[].rank behavior depend on object key ordering.
Update the typography loader to use an explicit rank from the token data (or add
validation for the expected sequence) when building the styles map, and keep the
rank tied to the TextStyleSchema/TextStyleMeta flow rather than insertion order.
| resolve: { | ||
| alias: { | ||
| '~': path.resolve(__dirname, 'src'), | ||
| }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -e
printf '\n## target file\n'
sed -n '1,120p' apps/figma-token-review-plugin/vite.config.plugin.ts
printf '\n## package files\n'
sed -n '1,220p' apps/figma-token-review-plugin/package.json
printf '\n## repo package root hints\n'
sed -n '1,220p' package.json 2>/dev/null || true
printf '\n## search for similar vite configs\n'
rg -n "defineConfig|vite.config" apps/figma-token-review-plugin -g 'vite.config*' -g 'package.json'Repository: goorm-dev/vapor-ui
Length of output: 6588
🌐 Web query:
Vite config TypeScript ESM __dirname import.meta.url docs
💡 Result:
In modern Vite environments (especially when using ES modules), you should prefer using import.meta.dirname instead of the traditional dirname variable [1][2][3]. While older versions of Vite provided a shim for dirname and filename in configuration files to support CommonJS patterns, these are considered legacy and potentially fragile [4][3]. Native ESM does not define these variables, and Vite has moved toward aligning with modern Node.js standards by encouraging the use of import.meta.dirname [1][2]. Key Recommendations: 1. Use import.meta.dirname: This is the native, standard way to access the current directory in ESM [1][2]. 2. Avoid dirname: Relying on Vite's automatic replacement of dirname or filename is discouraged for new or maintained projects, as it can lead to inconsistent behavior if the config file is processed differently or if you rely on deep-imported modules [5][3]. 3. Path Resolution: If you need to resolve paths relative to your config file, use node:path with import.meta.dirname: import { resolve } from 'node:path'; import { defineConfig } from 'vite'; export default defineConfig({ build: { lib: { entry: resolve(import.meta.dirname, 'src/main.ts'), }, }, }); This approach is cross-platform compatible and follows the latest recommended practices for ESM-based Vite configurations [1][2].
Citations:
- 1: vitejs/vite@7becf5f
- 2: refactor: use
import.meta.dirnameeverywhere vitejs/vite#21509 - 3: __dirname in ESM vite.config.ts + npx in pnpm scripts 2389-research/word-compiler#50
- 4: vitejs/vite@51e9195
- 5: Provide an unaltered/corrected import.meta object in vite.config.ts and imported files vitejs/vite#19730
🌐 Web query:
Node.js ESM __dirname undefined import.meta.url official docs
💡 Result:
In Node.js ES modules (ESM), the variables dirname and filename are not available and will be undefined if accessed [1][2][3]. Node.js provides official, direct replacements for these in the import.meta object [1][4]. For modern versions of Node.js (v20.11.0, v21.2.0 and later), you should use the following built-in properties [4]: import.meta.dirname import.meta.filename If you are using an older version of Node.js, you can derive these values using import.meta.url and the built-in node:url and node:path modules [4]: import { fileURLToPath } from 'node:url'; import { dirname } from 'node:path'; const filename = fileURLToPath(import.meta.url); const dirname = dirname(filename); Additionally, for tasks like loading relative files, the recommended pattern is to use import.meta.url directly with the URL constructor [1][2]: import { readFileSync } from 'node:fs'; const buffer = readFileSync(new URL('./data.txt', import.meta.url));
Citations:
- 1: https://nodejs.org/docs/latest-v26.x/api/esm.html
- 2: https://nodejs.org/api/esm.md
- 3: https://nodejs.org/docs/latest/api/esm.md
- 4: https://nodejs.github.io/package-examples/05-cjs-esm-migration/migrating-context-local-variables/
Use import.meta.dirname for the alias root. This config runs as ESM, so __dirname isn’t a stable anchor here and can break ~/* resolution during the plugin build. path.resolve(import.meta.dirname, 'src') is the simplest fix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/figma-token-review-plugin/vite.config.plugin.ts` around lines 7 - 10,
The alias root in the Vite plugin config uses __dirname even though this file
runs as ESM, so the `resolve.alias` setup for `~` can break during build. Update
the alias root in `vite.config.plugin.ts` to use `import.meta.dirname` inside
the existing `path.resolve` call so the `~/*` mapping stays stable. Keep the
change localized to the `resolve.alias` entry in the config.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/figma-token-review-plugin/src/ui/pages/settings.tsx`:
- Around line 85-87: The close button in SettingsPage is always rendered even
when onClose is missing, so it should be conditionally hidden. Update the
SettingsPage component to only render the Button that calls onClose when that
prop is provided, and keep the existing onClose wiring intact for the cases
where App.tsx passes it in.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3685c566-bb85-4765-99b3-4cf9547942b8
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (27)
apps/figma-token-review-plugin/manifest.jsonapps/figma-token-review-plugin/package.jsonapps/figma-token-review-plugin/src/common/messages.tsapps/figma-token-review-plugin/src/plugin/code.tsapps/figma-token-review-plugin/src/plugin/handlers/api-key.tsapps/figma-token-review-plugin/src/plugin/handlers/extract.tsapps/figma-token-review-plugin/src/plugin/handlers/focus.tsapps/figma-token-review-plugin/src/plugin/handlers/resize.tsapps/figma-token-review-plugin/src/plugin/handlers/scan.tsapps/figma-token-review-plugin/src/ui/App.tsxapps/figma-token-review-plugin/src/ui/components/resize-handler.tsxapps/figma-token-review-plugin/src/ui/features/api-key/index.tsapps/figma-token-review-plugin/src/ui/features/api-key/store.tsapps/figma-token-review-plugin/src/ui/features/api-key/use-api-key.tsapps/figma-token-review-plugin/src/ui/features/llm/client.tsapps/figma-token-review-plugin/src/ui/features/llm/index.tsapps/figma-token-review-plugin/src/ui/features/llm/merge.tsapps/figma-token-review-plugin/src/ui/features/messaging/evaluate.tsapps/figma-token-review-plugin/src/ui/features/messaging/index.tsapps/figma-token-review-plugin/src/ui/features/messaging/use-bridge.tsapps/figma-token-review-plugin/src/ui/features/messaging/use-request-api-key.tsapps/figma-token-review-plugin/src/ui/features/messaging/use-request-selection.tsapps/figma-token-review-plugin/src/ui/features/scan/store.tsapps/figma-token-review-plugin/src/ui/pages/home.tsxapps/figma-token-review-plugin/src/ui/pages/scan-result.tsxapps/figma-token-review-plugin/src/ui/pages/settings.tsxapps/figma-token-review-plugin/src/ui/providers.tsx
💤 Files with no reviewable changes (1)
- apps/figma-token-review-plugin/src/ui/features/messaging/use-request-selection.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- apps/figma-token-review-plugin/src/ui/providers.tsx
- apps/figma-token-review-plugin/manifest.json
- apps/figma-token-review-plugin/src/ui/features/messaging/index.ts
- apps/figma-token-review-plugin/src/plugin/handlers/focus.ts
- apps/figma-token-review-plugin/src/ui/components/resize-handler.tsx
- apps/figma-token-review-plugin/src/ui/pages/home.tsx
- apps/figma-token-review-plugin/package.json
- apps/figma-token-review-plugin/src/ui/features/llm/merge.ts
- apps/figma-token-review-plugin/src/common/messages.ts
- apps/figma-token-review-plugin/src/ui/features/llm/client.ts
- apps/figma-token-review-plugin/src/ui/features/messaging/evaluate.ts
- apps/figma-token-review-plugin/src/plugin/handlers/scan.ts
- apps/figma-token-review-plugin/src/plugin/handlers/extract.ts
MaxLee-dev
left a comment
MaxLee-dev
left a comment
There was a problem hiding this comment.
.env.example로 env 예시 파일을 올려주시면 감사하겠습니다.!
| "react": "^19.0.0", | ||
| "react-dom": "^19.0.0" |
There was a problem hiding this comment.
패키지 전체적인 라이브러리 버전 일치를 위해 catalog로 추가하는 것이 좋을 듯 합니다.!
| "rimraf": "^6.0.0", | ||
| "tailwindcss": "^4.0.0", | ||
| "typescript": "catalog:", | ||
| "vite": "^6.0.0", |
There was a problem hiding this comment.
혹시 vite 6버전으로 하신 이유가 있으실까요? 최신 버전인 8.* 버전 혹은 다른 vite를 사용하는 앱의 버전과 일치시키면 좋을 것 같습니다.!
| "noFallthroughCasesInSwitch": true, | ||
| "noUncheckedSideEffectImports": true | ||
| }, | ||
| "include": ["vite.config.ui.ts", "vite.config.plugin.ts"] |
There was a problem hiding this comment.
사소한 내용이지만, 여기 vite.config.ts만 타입체크 대상에서 빠져있어서 추가가 필요해 보입니다.
There was a problem hiding this comment.
vitest.config.ts 말씀하시는 거 맞죠?? 추가해둘게요!
| | `scan.ts` | in-flight scan cancel, `extract.ts` 호출, 결과 echo | | ||
| | `extract.ts` | 프레임 추출 (`extractFrame`) | | ||
| | `focus.ts` | 노드 id 리스트 resolve, viewport 이동, missing 카운트 응답 | | ||
| | `resize.ts` | `figma.ui.resize` 적용. `commit===true`일 때만 `clientStorage` persist | |
There was a problem hiding this comment.
여기 resize.ts에서 commit에 대한 정보를 넘겨주지 않아서 항상 commit = false. 그리고 초기에도 항상 기본 사이즈로 plugin이 열리고 있어서 여기 내용 수정이 필요해 보입니다.!
There was a problem hiding this comment.
아 이건 제거한 부분인데 아키텍처 문서에 반영이 안되어 있었네요! 수정했습니다!
|
|
||
| export type RequestId = string; | ||
|
|
||
| export type Envelope<T> = T & { requestId?: RequestId }; |
There was a problem hiding this comment.
여기 타입이 scan, focus와 같이 requestId가 반드시 필요한 메시지에도 requestID가 optional로 들어가고 있는 것 같습니다.
| export type Envelope<T> = T & { requestId?: RequestId }; | |
| type UiMsg = | |
| | { type: 'scan'; frameId: string; requestId: RequestId } | |
| | { type: 'focus'; nodeIds: string[]; requestId: RequestId } | |
| | { type: 'request-selection' } | |
| | { type: 'resize'; width: number; height: number } | |
| | { type: 'api-key:get' } | |
| | { type: 'api-key:set'; value: string } | |
| | { type: 'api-key:clear' }; |
이렇게 msg에서 바로 타입을 명확하게 정의하는 것도 좋아보입니다.!
| const isFail = (v: Violation): boolean => | ||
| v.severity === 'high' && (v.origin === 'rule' || v.confidence === 'HIGH'); | ||
| const high = violations.filter(isFail).length; |
There was a problem hiding this comment.
severtiy, confidence가 구분되어 있는 상황에서 confidence가 high가 아니면 전부 infos로 가게 되어 있습니다.!
severity와 confidence를 각각 다른 지표로 보고 사용자에게 보여주면 어떨까요??
| if (kind === 'other' && grade === '100') { | ||
| violations.push({ | ||
| ...base, | ||
| type: 'fg-grade-mismatch', | ||
| severity: 'high', | ||
| message: 'fg-100을 비순백 배경 위에 사용했습니다. .200 사용을 검토하세요.', | ||
| }); | ||
| continue; |
There was a problem hiding this comment.
여기 200 배경에 흰색·투명을 사용하는 건 검수하고 있지 않은 것 같습니다.!
There was a problem hiding this comment.
플러그인 작업 이후 시점에 결정되기는 했는데, foreground-100, foreground-200의 사용 케이스를 조금 수정했습니다! 그래서 foreground-xxx-200은 우선 모든 배경 위에 올라올 수 있기 때문에 유지해도 될 것 같은데 어떻게 생각하시나요??
https://goorm.slack.com/archives/C06KDQRLWP2/p1783651034173199 해당 내용 참고해주시면 될 것 같습니다!
| const isActive = selected === key; | ||
|
|
||
| return ( | ||
| <Tabs.Button key={key} value={key} $css={{ flexShrink: 0 }}> |
There was a problem hiding this comment.
여기 속성으로 인해 tab 길이가 길어지면 x축 스크롤이 생기고 있습니다. 확인 부탁드립니다.!
| if (msg.type !== 'scan') return; | ||
|
|
||
| const requestId = msg.requestId ?? null; | ||
| activeRequestId = requestId; |
There was a problem hiding this comment.
여기 activeRequestId를 활용할 수 있도록 UI 개선을 진행하면 어떨까요??
지금은 요청을 보내고 응답이 오기 전까지 새로운 요청을 다시 보낼 수 없는 플로우인 것 같습니다. 그래서 사실상 현재 activeRequestId는 중복 요청을 검수하는 목적으로 사용되고 있지 않은 것 같습니다.
검수 요청을 보내자마자 새로운 요청을 보낼 수 있도록 activeRequestId를 추가해 두신 것 같은데, 이를 활용해서 요청을 보낸 상태에서도 바로 재검사하기 버튼을 노출시켜서 검수하고자 하는 프레임을 즉각 수정할 수 있도록 개선하면 좋을 듯 합니다.!
There was a problem hiding this comment.
activeRequestId의 역할은 현재 선택된 프레임을 재검수 할 때, 다른 노드를 방문하고 재선택했는지를 검사하기 위한 플래그입니다!
다른 노드를 선택할 때는 바로바로 재검수 요청 버튼이 active 되는 게 맞지만, 한 번 검수한 노드를 다시 한 번 검수하고자 할 때는 다른 노드 방문 후 의도적으로 노드를 다시 선택해야 한다는 디자인 요구사항이 있었습니다. (참고)
이런 의도로 추가된 요소이고, 한 번 요청을 보낸 후에는 응답값을 받기 전까지 새로운 요청을 보내도록 의도하지는 않았습니다!
2 participants
Description of Changes
VAPOR-177
Summary by CodeRabbit
Features
Deterministic Validation
LLM Semantic Judgment
Plugin Capabilities & UI/UX
Non-Goals (Out of Scope)
Open Questions / Considerations
Screenshots
Checklist
Before submitting the PR, please make sure you have checked all of the following items.