refactor: remove Figma MCP analysis path#125
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR removes Figma MCP as a direct design-data source across code, tests, CLI, docs, and MCP tool schema, consolidating analysis inputs to a single Changes
Sequence Diagram(s)(Section intentionally omitted — changes are removals/refactors without a new multi-component sequential flow requiring visualization.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Figma MCP get_metadata returns collapsed nodes (1 node vs 291 from REST API), making MCP-based analysis meaningless (S/100% vs D/59% for same design). Removed: - src/core/adapters/figma-mcp-adapter.ts — MCP XML parser + enrichment - src/core/adapters/tailwind-parser.ts — Tailwind → Figma property converter - src/core/engine/design-data-parser.ts — MCP XML parsing (unused after removal) - MCP server: designData/designContext params from analyze tool - CLI: --mcp option from init, Figma MCP references from docs - docs/MCP-VS-CLI.md, calibrate-loop-deep command, mcp-vs-cli command - Skill: rewritten to CLI-based workflow (no Figma MCP dependency) Kept: - canicode MCP server (src/mcp/) — analyze via REST API input - CLI, REST API integration, design-tree, all rules Closes #124 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
let-sunny
force-pushed
the
refactor/remove-figma-mcp-analysis
branch
from
8129142 to
e198c5f
Compare
Clean up stale references to the removed Figma MCP analysis path: - .mcpb: remove designData/fileKey/fileName/customRulesPath params, MCP example - src/cli/index.ts: remove --mcp help text - CLAUDE.md: remove Figma MCP note and simplify calibration description - docs/CALIBRATION.md: remove calibrate-loop-deep reference - .claude/: remove Figma MCP mentions from calibration commands/agents - README.md: remove "Or use MCP" rate limit advice Made-with: Cursor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.mcpb (2)
4-4:⚠️ Potential issue | 🟡 MinorRule count inconsistency with SKILL.md.
The description states "39 rules in 6 categories" but
.claude/skills/canicode/SKILL.mdline 61 states "29 rules across 5 categories" (Structure, Token, Component, Naming, Behavior). The README.md also confirms 29 rules across 5 dimensions.📝 Proposed fix
- "description": "Analyze Figma design structures for development-friendliness and AI-friendliness. Scores designs across 39 rules in 6 categories (layout, design tokens, components, naming, AI readability, handoff risk) and generates detailed HTML reports with actionable issues.", + "description": "Analyze Figma design structures for development-friendliness and AI-friendliness. Scores designs across 29 rules in 5 categories (structure, token, component, naming, behavior) and generates detailed HTML reports with actionable issues.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.mcpb at line 4, The package description in the .mcpb file claims "39 rules in 6 categories" but SKILL.md and README state "29 rules across 5 categories"; update the "description" value in the .mcpb diff to match the canonical text used in .claude/skills/canicode/SKILL.md and README (e.g., change to "Analyze Figma design structures for development-friendliness and AI-friendliness. Scores designs across 29 rules in 5 categories (Structure, Token, Component, Naming, Behavior) and generates detailed HTML reports with actionable issues.") so all three places are consistent.
99-99:⚠️ Potential issue | 🟡 MinorSame rule count inconsistency in docs tool description.
This also references "39 rule IDs" which should be updated to match the actual rule count.
📝 Proposed fix
- "description": "Get documentation on config overrides, custom rules, all 39 rule IDs with default scores, and example configurations.", + "description": "Get documentation on config overrides, custom rules, all 29 rule IDs with default scores, and example configurations.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.mcpb at line 99, Update the "description" string in the .mcpb file so the hard-coded "39 rule IDs" matches the actual rule count; locate the JSON property named "description" (the line containing "Get documentation on config overrides, custom rules, all 39 rule IDs with default scores, and example configurations.") and change the numeric value to the correct count or make it dynamic/omit the number so docs stay accurate.src/mcp/server.ts (1)
98-104:⚠️ Potential issue | 🟡 MinorTelemetry
sourceis hardcoded—should distinguish fixture vs URL.The
sourcefield is always"url", but theinputparameter can be either a Figma URL or a local fixture path. The MCP tool description explicitly states: "Provide a Figma URL or fixture path," and the internalloadFilefunction already distinguishes between them usingisJsonFile(input) || isFixtureDir(input). The hardcoded source causes inaccurate analytics segmentation.📊 Proposed fix
Add the import:
+import { isJsonFile, isFixtureDir } from "../core/engine/loader.js";Then update the event at line 103:
trackEvent(EVENTS.ANALYSIS_COMPLETED, { nodeCount: result.nodeCount, issueCount: result.issues.length, grade: scores.overall.grade, percentage: scores.overall.percentage, - source: "url", + source: isJsonFile(input) || isFixtureDir(input) ? "fixture" : "figma", });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mcp/server.ts` around lines 98 - 104, The telemetry event currently hardcodes source:"url" in the trackEvent call (EVENTS.ANALYSIS_COMPLETED); change it to compute the source from the original input used to load the file by reusing the same detection logic as loadFile (i.e., use isJsonFile(input) || isFixtureDir(input) to set source to "fixture" when true, otherwise "url"), keeping the trackEvent call and event payload keys the same and referencing the existing input parameter and the helper functions isJsonFile/isFixtureDir to locate where to change it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli/commands/init.ts`:
- Around line 8-10: The CLI input schema InitOptionsSchema is defined inside the
command module; extract it into a shared contracts module and import it instead:
create a new contracts file that exports InitOptionsSchema (a z.object with
token: z.string().optional()), remove the local InitOptionsSchema declaration in
the init command, and update the init command to import and use the exported
InitOptionsSchema for validation so external CLI input follows the contracts/
validation architecture.
---
Outside diff comments:
In @.mcpb:
- Line 4: The package description in the .mcpb file claims "39 rules in 6
categories" but SKILL.md and README state "29 rules across 5 categories"; update
the "description" value in the .mcpb diff to match the canonical text used in
.claude/skills/canicode/SKILL.md and README (e.g., change to "Analyze Figma
design structures for development-friendliness and AI-friendliness. Scores
designs across 29 rules in 5 categories (Structure, Token, Component, Naming,
Behavior) and generates detailed HTML reports with actionable issues.") so all
three places are consistent.
- Line 99: Update the "description" string in the .mcpb file so the hard-coded
"39 rule IDs" matches the actual rule count; locate the JSON property named
"description" (the line containing "Get documentation on config overrides,
custom rules, all 39 rule IDs with default scores, and example configurations.")
and change the numeric value to the correct count or make it dynamic/omit the
number so docs stay accurate.
In `@src/mcp/server.ts`:
- Around line 98-104: The telemetry event currently hardcodes source:"url" in
the trackEvent call (EVENTS.ANALYSIS_COMPLETED); change it to compute the source
from the original input used to load the file by reusing the same detection
logic as loadFile (i.e., use isJsonFile(input) || isFixtureDir(input) to set
source to "fixture" when true, otherwise "url"), keeping the trackEvent call and
event payload keys the same and referencing the existing input parameter and the
helper functions isJsonFile/isFixtureDir to locate where to change it.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 085aace7-19b4-448d-a72d-68a84d0d80d3
📒 Files selected for processing (20)
.claude/agents/calibration/converter.md.claude/commands/calibrate-loop-deep.md.claude/commands/calibrate-loop.md.claude/commands/mcp-vs-cli.md.claude/skills/canicode/SKILL.md.mcpbCLAUDE.mdREADME.mddocs/CALIBRATION.mddocs/MCP-VS-CLI.mdsrc/cli/commands/init.tssrc/cli/docs.tssrc/cli/index.tssrc/core/adapters/figma-mcp-adapter.tssrc/core/adapters/index.tssrc/core/adapters/tailwind-parser.test.tssrc/core/adapters/tailwind-parser.tssrc/core/engine/design-data-parser.test.tssrc/core/engine/design-data-parser.tssrc/mcp/server.ts
💤 Files with no reviewable changes (10)
- docs/CALIBRATION.md
- src/core/adapters/index.ts
- docs/MCP-VS-CLI.md
- src/core/engine/design-data-parser.test.ts
- src/core/engine/design-data-parser.ts
- src/core/adapters/tailwind-parser.test.ts
- .claude/commands/mcp-vs-cli.md
- src/core/adapters/figma-mcp-adapter.ts
- src/core/adapters/tailwind-parser.ts
- .claude/commands/calibrate-loop-deep.md
Comment on lines
8
to
+10
| const InitOptionsSchema = z.object({ | ||
| token: z.string().optional(), | ||
| mcp: z.boolean().optional(), | ||
| }).refine( | ||
| (opts) => !(opts.token && opts.mcp), | ||
| { message: "--token and --mcp are mutually exclusive. Choose one." } | ||
| ); | ||
| }); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Move CLI input schema to contracts/ to satisfy validation architecture rules.
InitOptionsSchema validates external CLI input, but it is defined locally in the command module. Please define/import it from contracts/ instead of declaring it here.
♻️ Proposed refactor
import type { CAC } from "cac";
-import { z } from "zod";
+import { InitOptionsSchema } from "../../core/contracts/init-options.js";
import {
initAiready, getConfigPath, getReportsDir,
} from "../../core/engine/config-store.js";
-
-const InitOptionsSchema = z.object({
- token: z.string().optional(),
-});// src/core/contracts/init-options.ts
import { z } from "zod";
export const InitOptionsSchema = z.object({
token: z.string().optional(),
});As per coding guidelines: "Validate all external inputs with Zod schemas defined in contracts/ directory".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/commands/init.ts` around lines 8 - 10, The CLI input schema
InitOptionsSchema is defined inside the command module; extract it into a shared
contracts module and import it instead: create a new contracts file that exports
InitOptionsSchema (a z.object with token: z.string().optional()), remove the
local InitOptionsSchema declaration in the init command, and update the init
command to import and use the exported InitOptionsSchema for validation so
external CLI input follows the contracts/ validation architecture.
- .mcpb: "39 rules in 6 categories" → "29 rules in 5 categories" - .mcpb: "39 rule IDs" → "29 rule IDs" - server.ts: telemetry source distinguishes fixture vs figma URL Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Remove the Figma MCP → canicode analysis path. -1,659 lines.
canicode MCP server is kept — only the Figma MCP data input is removed.
Why
Figma MCP
get_metadatareturns collapsed nodes — same design produces 1 node (MCP) vs 291 nodes (CLI). Analysis via Figma MCP is meaningless:What's removed
src/core/adapters/figma-mcp-adapter.tssrc/core/adapters/tailwind-parser.tssrc/core/engine/design-data-parser.tssrc/mcp/server.tsdesignData,designContext,fileKey,fileNameparamsdocs/MCP-VS-CLI.md.claude/commands/calibrate-loop-deep.md.claude/commands/mcp-vs-cli.md.claude/skills/canicode/SKILL.mdsrc/cli/commands/init.ts--mcpoption removedsrc/cli/docs.tssrc/cli/index.ts--mcphelp line removed.mcpbdesignData,designContext,customRulesPathparams removedCLAUDE.mdREADME.mddocs/CALIBRATION.mdcalibrate-loop-deepexample removed.claude/agents/calibration/converter.mdWhat's kept
src/mcp/) — still servesanalyze,list-rules,visual-compare,version,docstools via REST APIinputparamTest plan
grep -rn "Figma MCP\|get_metadata\|designData\|designContext" . --include="*.ts" --include="*.md" --include="*.json" | grep -v node_modules | grep -v worktrees— cleanCloses #124
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Refactor
Chores