-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: split CLI index.ts into per-command modules (#47) #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,217 @@ | ||
| import { existsSync, mkdirSync } from "node:fs"; | ||
| import { writeFile } from "node:fs/promises"; | ||
| import { resolve, dirname } from "node:path"; | ||
| import type { CAC } from "cac"; | ||
|
|
||
| import type { RuleConfig, RuleId } from "../../core/contracts/rule.js"; | ||
| import { analyzeFile } from "../../core/engine/rule-engine.js"; | ||
| import { loadFile, isJsonFile, isFixtureDir } from "../../core/engine/loader.js"; | ||
| import { | ||
| getFigmaToken, getReportsDir, ensureReportsDir, | ||
| } from "../../core/engine/config-store.js"; | ||
| import { calculateScores, formatScoreSummary, buildResultJson } from "../../core/engine/scoring.js"; | ||
| import { getConfigsWithPreset, RULE_CONFIGS, type Preset } from "../../core/rules/rule-config.js"; | ||
| import { ruleRegistry } from "../../core/rules/rule-registry.js"; | ||
| import { loadCustomRules } from "../../core/rules/custom/custom-rule-loader.js"; | ||
| import { loadConfigFile, mergeConfigs } from "../../core/rules/custom/config-loader.js"; | ||
| import { generateHtmlReport } from "../../core/report-html/index.js"; | ||
| import { trackEvent, trackError, EVENTS } from "../../core/monitoring/index.js"; | ||
| import { pickRandomScope, countNodes, MAX_NODES_WITHOUT_SCOPE } from "../helpers.js"; | ||
|
|
||
| interface AnalyzeOptions { | ||
| preset?: Preset; | ||
| output?: string; | ||
| token?: string; | ||
| api?: boolean; | ||
| screenshot?: boolean; | ||
| customRules?: string; | ||
| config?: string; | ||
| noOpen?: boolean; | ||
| json?: boolean; | ||
| } | ||
|
|
||
| export function registerAnalyze(cli: CAC): void { | ||
| cli | ||
| .command("analyze <input>", "Analyze a Figma file or JSON fixture") | ||
| .option("--preset <preset>", "Analysis preset (relaxed | dev-friendly | ai-ready | strict)") | ||
| .option("--output <path>", "HTML report output path") | ||
| .option("--token <token>", "Figma API token (or use FIGMA_TOKEN env var)") | ||
| .option("--api", "Load via Figma REST API (requires FIGMA_TOKEN)") | ||
| .option("--screenshot", "Include screenshot comparison in report (requires ANTHROPIC_API_KEY)") | ||
| .option("--custom-rules <path>", "Path to custom rules JSON file") | ||
| .option("--config <path>", "Path to config JSON file (override rule scores/settings)") | ||
| .option("--no-open", "Don't open report in browser after analysis") | ||
| .option("--json", "Output JSON results to stdout (same format as MCP)") | ||
| .example(" canicode analyze https://www.figma.com/design/ABC123/MyDesign") | ||
| .example(" canicode analyze https://www.figma.com/design/ABC123/MyDesign --api --token YOUR_TOKEN") | ||
| .example(" canicode analyze ./fixtures/my-design --output report.html") | ||
| .example(" canicode analyze ./fixtures/my-design --custom-rules ./my-rules.json") | ||
| .example(" canicode analyze ./fixtures/my-design --config ./my-config.json") | ||
| .action(async (input: string, options: AnalyzeOptions) => { | ||
| const analysisStart = Date.now(); | ||
| trackEvent(EVENTS.ANALYSIS_STARTED, { source: isJsonFile(input) || isFixtureDir(input) ? "fixture" : "figma" }); | ||
| try { | ||
| // Check init | ||
| if (!options.token && !getFigmaToken() && !isJsonFile(input) && !isFixtureDir(input)) { | ||
| throw new Error( | ||
| "canicode is not configured. Run 'canicode init --token YOUR_TOKEN' first." | ||
| ); | ||
| } | ||
|
|
||
| // Validate --screenshot requirements | ||
| if (options.screenshot) { | ||
| const anthropicKey = process.env["ANTHROPIC_API_KEY"]; | ||
| if (!anthropicKey) { | ||
| throw new Error( | ||
| "ANTHROPIC_API_KEY required for --screenshot mode. Set it in .env or environment." | ||
| ); | ||
| } | ||
| console.log("Screenshot comparison mode enabled (coming soon).\n"); | ||
| } | ||
|
|
||
| // Load file | ||
| const { file, nodeId } = await loadFile(input, options.token); | ||
|
|
||
| // Scope enforcement for large files | ||
| const totalNodes = countNodes(file.document); | ||
| let effectiveNodeId = nodeId; | ||
|
|
||
| if (!effectiveNodeId && totalNodes > MAX_NODES_WITHOUT_SCOPE) { | ||
| if (isJsonFile(input) || isFixtureDir(input)) { | ||
| // Fixture: auto-pick a random suitable FRAME | ||
| const picked = pickRandomScope(file.document); | ||
| if (picked) { | ||
| effectiveNodeId = picked.id; | ||
| console.log(`\nAuto-scoped to "${picked.name}" (${picked.id}, ${countNodes(picked)} nodes) — file too large (${totalNodes} nodes) for unscoped analysis.`); | ||
| } else { | ||
| console.warn(`\nWarning: Could not find a suitable scope in fixture. Analyzing all ${totalNodes} nodes.`); | ||
| } | ||
| } else { | ||
| // Figma URL: require explicit node-id | ||
| throw new Error( | ||
| `Too many nodes (${totalNodes}) for unscoped analysis. ` + | ||
| `Max ${MAX_NODES_WITHOUT_SCOPE} nodes without a node-id scope.\n\n` + | ||
| `Add ?node-id=XXX to the Figma URL to target a specific section.\n` + | ||
| `Example: canicode analyze "https://www.figma.com/design/.../MyDesign?node-id=1-234"` | ||
| ); | ||
| } | ||
| } | ||
| if (!effectiveNodeId && totalNodes > 100) { | ||
| console.warn(`\nWarning: Analyzing ${totalNodes} nodes without scope. Results may be noisy.`); | ||
| console.warn("Tip: Add ?node-id=XXX to analyze a specific section.\n"); | ||
| } | ||
|
|
||
| console.log(`\nAnalyzing: ${file.name}`); | ||
| console.log(`Nodes: ${totalNodes}`); | ||
|
Comment on lines
+104
to
+105
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
By the time this branch runs, the command has already written progress text to stdout, and the early Also applies to: 121-131, 144-152, 197-200 🤖 Prompt for AI Agents
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pre-existing issue — the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Would you like me to open a follow-up GitHub issue to track the 🧠 Learnings used |
||
|
|
||
| // Build rule configs: start from preset or defaults | ||
| let configs: Record<string, RuleConfig> = options.preset | ||
| ? { ...getConfigsWithPreset(options.preset) } | ||
| : { ...RULE_CONFIGS }; | ||
|
|
||
| // Load and merge config file overrides | ||
| let excludeNodeNames: string[] | undefined; | ||
| let excludeNodeTypes: string[] | undefined; | ||
|
|
||
| if (options.config) { | ||
| const configFile = await loadConfigFile(options.config); | ||
| configs = mergeConfigs(configs, configFile); | ||
| excludeNodeNames = configFile.excludeNodeNames; | ||
| excludeNodeTypes = configFile.excludeNodeTypes; | ||
| console.log(`Config loaded: ${options.config}`); | ||
| } | ||
|
|
||
| // Load and register custom rules | ||
| if (options.customRules) { | ||
| const { rules, configs: customConfigs } = await loadCustomRules(options.customRules); | ||
| for (const rule of rules) { | ||
| ruleRegistry.register(rule); | ||
| } | ||
| configs = { ...configs, ...customConfigs }; | ||
| console.log(`Custom rules loaded: ${rules.length} rules from ${options.customRules}`); | ||
| } | ||
|
|
||
| // Build analysis options | ||
| const analyzeOptions = { | ||
| configs: configs as Record<RuleId, RuleConfig>, | ||
| ...(effectiveNodeId && { targetNodeId: effectiveNodeId }), | ||
| ...(excludeNodeNames && { excludeNodeNames }), | ||
| ...(excludeNodeTypes && { excludeNodeTypes }), | ||
| }; | ||
|
|
||
| // Run analysis | ||
| const result = analyzeFile(file, analyzeOptions); | ||
| console.log(`Nodes: ${result.nodeCount} (max depth: ${result.maxDepth})`); | ||
|
|
||
| // Calculate scores | ||
| const scores = calculateScores(result); | ||
|
|
||
| // JSON output mode | ||
| if (options.json) { | ||
| console.log(JSON.stringify(buildResultJson(file.name, result, scores), null, 2)); | ||
| return; | ||
| } | ||
|
|
||
| // Print summary to terminal | ||
| console.log("\n" + "=".repeat(50)); | ||
| console.log(formatScoreSummary(scores)); | ||
| console.log("=".repeat(50)); | ||
|
|
||
| // Generate HTML report | ||
| const now = new Date(); | ||
| const ts = `${now.getFullYear()}-${String(now.getMonth() + 1).padStart(2, "0")}-${String(now.getDate()).padStart(2, "0")}-${String(now.getHours()).padStart(2, "0")}-${String(now.getMinutes()).padStart(2, "0")}`; | ||
| let outputPath: string; | ||
|
|
||
| if (options.output) { | ||
| outputPath = resolve(options.output); | ||
| const outputDir = dirname(outputPath); | ||
| if (!existsSync(outputDir)) { | ||
| mkdirSync(outputDir, { recursive: true }); | ||
| } | ||
| } else { | ||
| ensureReportsDir(); | ||
| outputPath = resolve(getReportsDir(), `report-${ts}-${file.fileKey}.html`); | ||
| } | ||
|
|
||
| const figmaToken = options.token ?? getFigmaToken(); | ||
| const html = generateHtmlReport(file, result, scores, { figmaToken }); | ||
| await writeFile(outputPath, html, "utf-8"); | ||
| console.log(`\nReport saved: ${outputPath}`); | ||
|
|
||
| trackEvent(EVENTS.ANALYSIS_COMPLETED, { | ||
| nodeCount: result.nodeCount, | ||
| issueCount: result.issues.length, | ||
| grade: scores.overall.grade, | ||
| percentage: scores.overall.percentage, | ||
| duration: Date.now() - analysisStart, | ||
| }); | ||
| trackEvent(EVENTS.REPORT_GENERATED, { format: "html" }); | ||
|
|
||
| // Open in browser unless --no-open | ||
| if (!options.noOpen) { | ||
| const { exec } = await import("node:child_process"); | ||
| const cmd = process.platform === "darwin" ? "open" : process.platform === "win32" ? "start" : "xdg-open"; | ||
| exec(`${cmd} "${outputPath}"`); | ||
| } | ||
|
|
||
| // Exit with error code if grade is F | ||
| if (scores.overall.grade === "F") { | ||
| process.exit(1); | ||
| } | ||
| } catch (error) { | ||
| trackError( | ||
| error instanceof Error ? error : new Error(String(error)), | ||
| { command: "analyze", input }, | ||
| ); | ||
| trackEvent(EVENTS.ANALYSIS_FAILED, { | ||
| error: error instanceof Error ? error.message : String(error), | ||
| duration: Date.now() - analysisStart, | ||
| }); | ||
| console.error( | ||
| "\nError:", | ||
| error instanceof Error ? error.message : String(error) | ||
| ); | ||
| process.exit(1); | ||
| } | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| import type { CAC } from "cac"; | ||
|
|
||
| import { | ||
| getConfigPath, readConfig, setTelemetryEnabled, | ||
| } from "../../core/engine/config-store.js"; | ||
|
|
||
| interface ConfigOptions { | ||
| telemetry?: boolean; | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| export function registerConfig(cli: CAC): void { | ||
| cli | ||
| .command("config", "Manage canicode configuration") | ||
| .option("--telemetry", "Enable anonymous telemetry") | ||
| .option("--no-telemetry", "Disable anonymous telemetry") | ||
| .action((options: ConfigOptions) => { | ||
|
Comment on lines
+7
to
+16
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major 🧩 Analysis chain🏁 Script executed: # Check if the file exists and view its current contents
find . -name "config.ts" -path "*/cli/commands/*" | head -5Repository: let-sunny/canicode Length of output: 90 🏁 Script executed: # Search for Zod usage patterns in CLI commands
rg "import.*zod" src/cli/ --type tsRepository: let-sunny/canicode Length of output: 44 🏁 Script executed: # Check contracts directory for schema examples
fd "schema\|contract" --type f src/Repository: let-sunny/canicode Length of output: 44 🏁 Script executed: # Check package.json to confirm zod is a dependency
cat package.json | grep -A 50 "dependencies\|devDependencies" | grep -i zodRepository: let-sunny/canicode Length of output: 81 🏁 Script executed: # Read the config.ts file to verify contents match review snippet
cat -n src/cli/commands/config.tsRepository: let-sunny/canicode Length of output: 2063 🏁 Script executed: # Check how other CLI commands are structured (if they exist)
ls -la src/cli/commands/Repository: let-sunny/canicode Length of output: 799 🏁 Script executed: # Look for any existing schema usage in the codebase to understand conventions
rg "z\.object\|ZodSchema\|const.*Schema" src/ --type ts | head -20Repository: let-sunny/canicode Length of output: 44 🏁 Script executed: # Check for any Zod schema files more broadly in the codebase
rg "z\.object|ZodSchema|const.*Schema.*=.*z\." src/ --type ts -A 2 | head -30Repository: let-sunny/canicode Length of output: 1721 🏁 Script executed: # Check if other CLI commands also lack Zod validation
for file in src/cli/commands/*.ts; do
echo "=== $file ==="
head -20 "$file" | grep -E "interface|type.*Options|z\.object|z\.infer" || echo "No schema/interface found"
doneRepository: let-sunny/canicode Length of output: 651 Add Zod schema validation for CLI options.
Proposed change import type { CAC } from "cac";
+import { z } from "zod";
import {
getConfigPath, readConfig, setTelemetryEnabled,
} from "../../core/engine/config-store.js";
-interface ConfigOptions {
- telemetry?: boolean;
-}
+const ConfigOptionsSchema = z.object({
+ telemetry: z.boolean().optional(),
+});
+type ConfigOptions = z.infer<typeof ConfigOptionsSchema>;
export function registerConfig(cli: CAC): void {
cli
@@
- .action((options: ConfigOptions) => {
+ .action((rawOptions: unknown) => {
try {
+ const options: ConfigOptions = ConfigOptionsSchema.parse(rawOptions);
// CAC maps --no-telemetry to options.telemetry === false
if (options.telemetry === false) {🤖 Prompt for AI Agents |
||
| try { | ||
| // CAC maps --no-telemetry to options.telemetry === false | ||
| if (options.telemetry === false) { | ||
| setTelemetryEnabled(false); | ||
| console.log("Telemetry disabled. No analytics data will be sent."); | ||
| return; | ||
| } | ||
|
|
||
| if (options.telemetry === true) { | ||
| setTelemetryEnabled(true); | ||
| console.log("Telemetry enabled. Only anonymous usage events are tracked — no design data."); | ||
| return; | ||
| } | ||
|
|
||
| // No flags: show current config | ||
| const cfg = readConfig(); | ||
| console.log("CANICODE CONFIG\n"); | ||
| console.log(` Config path: ${getConfigPath()}`); | ||
| console.log(` Figma token: ${cfg.figmaToken ? "set" : "not set"}`); | ||
| console.log(` Telemetry: ${cfg.telemetry !== false ? "enabled" : "disabled"}`); | ||
| console.log(`\nOptions:`); | ||
| console.log(` canicode config --no-telemetry Opt out of anonymous telemetry`); | ||
| console.log(` canicode config --telemetry Opt back in`); | ||
| } catch (error) { | ||
| console.error( | ||
| "\nError:", | ||
| error instanceof Error ? error.message : String(error) | ||
| ); | ||
| process.exit(1); | ||
| } | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| import { existsSync, mkdirSync } from "node:fs"; | ||
| import { resolve, dirname } from "node:path"; | ||
| import type { CAC } from "cac"; | ||
|
|
||
| import { loadFile, isJsonFile } from "../../core/engine/loader.js"; | ||
|
|
||
| export function registerDesignTree(cli: CAC): void { | ||
| cli | ||
| .command( | ||
| "design-tree <input>", | ||
| "Generate a DOM-like design tree from a Figma file or fixture" | ||
| ) | ||
| .option("--token <token>", "Figma API token (or use FIGMA_TOKEN env var)") | ||
| .option("--output <path>", "Output file path (default: stdout)") | ||
| .option("--vector-dir <path>", "Directory with SVG files for VECTOR nodes (auto-detected from fixture path)") | ||
| .option("--image-dir <path>", "Directory with image PNGs for IMAGE fill nodes (auto-detected from fixture path)") | ||
| .example(" canicode design-tree ./fixtures/my-design") | ||
| .example(" canicode design-tree https://www.figma.com/design/ABC/File?node-id=1-234 --output tree.txt") | ||
| .action(async (input: string, options: { token?: string; output?: string; vectorDir?: string; imageDir?: string }) => { | ||
| try { | ||
| const { file } = await loadFile(input, options.token); | ||
|
|
||
| const fixtureBase = isJsonFile(input) ? dirname(resolve(input)) : resolve(input); | ||
|
|
||
| // Auto-detect vector dir from fixture path | ||
| let vectorDir = options.vectorDir; | ||
| if (!vectorDir) { | ||
| const autoDir = resolve(fixtureBase, "vectors"); | ||
| if (existsSync(autoDir)) vectorDir = autoDir; | ||
| } | ||
|
|
||
| // Auto-detect image dir from fixture path | ||
| let imageDir = options.imageDir; | ||
| if (!imageDir) { | ||
| const autoDir = resolve(fixtureBase, "images"); | ||
| if (existsSync(autoDir)) imageDir = autoDir; | ||
| } | ||
|
|
||
| const { generateDesignTreeWithStats } = await import("../../core/engine/design-tree.js"); | ||
| const treeOptions = { | ||
| ...(vectorDir ? { vectorDir } : {}), | ||
| ...(imageDir ? { imageDir } : {}), | ||
| }; | ||
| const stats = generateDesignTreeWithStats(file, treeOptions); | ||
|
|
||
| if (options.output) { | ||
| const outputDir = dirname(resolve(options.output)); | ||
| if (!existsSync(outputDir)) mkdirSync(outputDir, { recursive: true }); | ||
| const { writeFile: writeFileAsync } = await import("node:fs/promises"); | ||
| await writeFileAsync(resolve(options.output), stats.tree, "utf-8"); | ||
| console.log(`Design tree saved: ${resolve(options.output)} (${Math.round(stats.bytes / 1024)}KB, ~${stats.estimatedTokens} tokens)`); | ||
| } else { | ||
| console.log(stats.tree); | ||
| } | ||
| } catch (error) { | ||
| console.error("\nError:", error instanceof Error ? error.message : String(error)); | ||
| process.exit(1); | ||
| } | ||
| }); | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Validate the CLI contract at the boundary.
AnalyzeOptionsonly constrains the TypeScript side; it does not validate what the user actually typed.--preset,--config, and--custom-rulesare all consumed as trusted runtime values here, so this command should switch to a Zod schema and infer the option type from it.As per coding guidelines, "Validate all external inputs with Zod schemas" and "Infer TypeScript types from Zod schemas using
z.infer<typeof Schema>."Also applies to: 108-118, 125-126
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a pre-existing pattern from the original
index.ts. Adding Zod validation for CLI options is a valid improvement but out of scope for this pure refactoring PR. Filed as a follow-up concern for #50 (test improvements).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@let-sunny, understood — since this PR is a pure structural refactor and the pattern was already present in the originalindex.ts, keeping the scope focused is the right call. Tracking it under#50makes sense. I'll leave the concern open there.🧠 Learnings used