refactor: split CLI index.ts into per-command modules (#47)#59
refactor: split CLI index.ts into per-command modules (#47)#59
Conversation
Extract 18 commands from the 1,630-line monolithic CLI entry point into individual modules under src/cli/commands/, with internal calibration commands separated into src/cli/commands/internal/. Structure: - src/cli/index.ts — slim entry point (registration + monitoring + help) - src/cli/helpers.ts — shared utilities (countNodes, pickRandomScope, etc.) - src/cli/commands/*.ts — user-facing commands (analyze, save-fixture, etc.) - src/cli/commands/internal/*.ts — calibration commands (subagent-only) https://claude.ai/code/session_01YBznYqcXw5uSGdbcFeNnXR
📝 WalkthroughWalkthroughThis PR splits the monolithic CLI into modular command files under Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as "CLI (analyze cmd)"
participant Mon as "Monitoring"
participant Loader as "File Loader (Figma/Fixture)"
participant Analyzer as "analyzeFile"
participant Rules as "Rule Registry/Custom Rules"
participant Reporter as "Reporter (HTML/JSON + Disk)"
participant Browser as "Browser (open)"
CLI->>Mon: emit start
CLI->>Loader: loadFile(input, token?)
Loader-->>CLI: file / fixture data
CLI->>Rules: build active rule config (preset + overrides + custom)
CLI->>Analyzer: run analyzeFile(file, scope, exclusions, config)
Analyzer-->>CLI: analysisOutput (grade, node summaries)
CLI->>Reporter: format output (JSON or HTML)
Reporter-->>CLI: reportPath / stdout
CLI->>Mon: emit complete/failure
alt open report
CLI->>Browser: open(reportPath)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/index.ts (1)
57-59:⚠️ Potential issue | 🟠 MajorUse
process.exitCodeinstead ofprocess.exit()to allow telemetry shutdown.Direct calls to
process.exit(1)across command handlers (analyze, implement, design-tree, config, init, etc.) bypass thebeforeExitevent entirely, preventingshutdownMonitoring()from running. This means in-flight telemetry requests are lost on common error paths. Setprocess.exitCodeinstead and let the process exit naturally sobeforeExitfires and monitoring completes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/index.ts` around lines 57 - 59, Several command handlers currently call process.exit(...) directly which prevents the beforeExit handler (shutdownMonitoring) from running; update all direct exits (e.g., any uses of process.exit(1) in handlers for analyze, implement, design-tree, config, init or any helper like exitWithError) to instead set process.exitCode = 1 and then return/resolve from the handler so the process can terminate naturally and fire the beforeExit event. Search for process.exit(...) usages in the CLI handlers and replace them with setting process.exitCode (or 0/other codes as appropriate) and removing the explicit exit call; ensure no code paths immediately call process.exit() after setting exitCode. This preserves existing control flow while allowing shutdownMonitoring() bound in process.on("beforeExit", ...) to run.
🤖 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/analyze.ts`:
- Around line 104-105: The progress and status messages in analyze.ts (e.g., the
console.log calls around "Analyzing: ${file.name}" and "Nodes: ${totalNodes}")
must be routed to stderr when --json is requested so stdout remains pure
machine-readable JSON; change these progress/log prints to use console.error (or
a logger that writes to stderr) when options.json is true, remove early returns
that skip the final grade handling, and centralize the completion path so that
JSON output is written once to stdout and the process exit code is set based on
grade === "F" (ensure the code paths referenced around the blocks at 104-105,
121-131, 144-152, and 197-200 all follow this pattern).
- Around line 21-31: Replace the unvalidated TypeScript-only interface
AnalyzeOptions with a Zod schema that explicitly validates runtime CLI inputs
(create e.g. AnalyzeOptionsSchema using z.object({...}) covering keys preset,
config, customRules, output, token, api, screenshot, noOpen, json), validate the
parsed CLI options against that schema at the command boundary and throw/report
errors for invalid input, and infer the TypeScript type via z.infer<typeof
AnalyzeOptionsSchema> to replace the AnalyzeOptions interface wherever used;
ensure the schema specifically validates the trusted flags --preset, --config,
and --custom-rules (and any other options listed) before their values are
consumed.
In `@src/cli/commands/config.ts`:
- Around line 7-10: Remove the unused noTelemetry property from the
ConfigOptions interface and update the command handler that currently checks
options.noTelemetry to instead check options.telemetry === false (CAC maps
--no-telemetry to options.telemetry === false); locate the ConfigOptions
interface and the handler referencing options.noTelemetry and replace the
condition so the disable-telemetry branch is reachable.
In `@src/cli/commands/implement.ts`:
- Around line 60-80: The copied vectors/images are logged but
generateDesignTreeWithStats() is still using the original fixture paths
(vectorDir/imageDir), producing external references; after copying files into
vecOutputDir/imgOutputDir (the dirs created in this block) update the variables
passed into generateDesignTreeWithStats() (or set vectorDir/imageDir) to the new
local paths (vecOutputDir and imgOutputDir) so the function resolves assets
inside the package; apply the same fix in the similar block around the other
occurrence referenced (197-201).
- Around line 83-99: Add a warning when the implement command is given an
unscoped Figma URL (no node-id) by checking the parsed nodeId from
parseFigmaUrl(input) before proceeding; if nodeId is falsy, emit a clear
console.warn (or use the existing logger) explaining that an unscoped/full-file
URL was provided and that this may download the entire file or produce large
output, then continue using rootNodeId as currently computed—update the logic
around the variables nodeId and rootNodeId in implement (where parseFigmaUrl is
called) to perform this check and warn.
In `@src/cli/commands/init.ts`:
- Around line 15-20: The CLI currently lets --token and --mcp be passed together
and silently prefers the token path; update the action handler for InitOptions
to validate inputs with a Zod schema that enforces mutual exclusivity of token
and mcp (e.g., require exactly one of token or mcp), and if validation fails
return/throw a user-facing error before calling initAiready or the MCP display
function; locate the action callback around InitOptions, the initAiready(...)
call, and the branch that handles the mcp flag and perform the validation there
so automation fails fast when both flags are supplied.
In `@src/cli/commands/internal/calibrate-evaluate.ts`:
- Around line 39-55: Add explicit Zod validation for the parsed JSON before
dereferencing or passing to runCalibrationEvaluate: create Zod schemas for the
expected analysis and conversion payloads (mirror existing patterns like
GapAnalyzerOutputSchema) in src/agents/contracts (e.g., AnalysisOutputSchema,
ConversionOutputSchema), then replace the direct JSON.parse usage by parsing and
immediately running schema.safeParse on analysisData and conversionData,
throwing a clear CLI validation error if safeParse fails; only after both
safeParse checks succeed, use the inferred/parsed values (the validated
analysisData and conversionData) when calling runCalibrationEvaluate and when
accessing analysisData.ruleScores.
- Around line 15-21: Change the .command signature in calibrate-evaluate.ts from
required positionals ("<analysisJson> <conversionJson>") to optional
("[analysisJson] [conversionJson]") so the CAC parser allows --run-dir-only
invocations, update the action handler parameter types to accept undefined for
analysisJsonPath and conversionJsonPath and let the existing run-dir branch read
files when those args are absent; additionally, after reading/parsing JSON in
the action handler (where JSON.parse currently runs), validate the data using
the Zod schemas from src/agents/contracts (e.g., the analysis and conversion
record schemas) via schema.parse() or schema.safeParse() and handle parse
failures (log error/exit) before proceeding with calibration logic—refer to the
.command call, the action(...) handler, and CalibrateEvaluateOptions to locate
where to make these changes.
In `@src/cli/commands/internal/calibrate-gap-report.ts`:
- Around line 45-77: The handler currently prints human-readable backup/status
messages to stdout even when options.json is set; change it so JSON is the only
thing sent to stdout and all human messages go to stderr (or are suppressed in
JSON mode). Specifically, update console.log calls around the backup logic and
the final status lines (references: options.json, outPath, result, backupPath,
writeFile/readFileAsync) to either: (a) if options.json then avoid printing
those human messages (or print them via console.error), and then emit only JSON
to stdout via console.log(JSON.stringify(...)); or (b) unconditionally send
human-readable logs to stderr using console.error while leaving the JSON payload
written to stdout. Ensure the backup message (`Previous report backed up:
${backupPath}`) and the three status lines are moved accordingly so parsers
receive a clean JSON on stdout.
- Around line 33-37: The current expression treats an explicit "0" as falsy and
falls back to 2; to fix, parse options.minRepeat into a temporary numeric
variable (e.g. parsed = parseInt(options.minRepeat ?? "2", 10)), check for NaN
(using Number.isNaN or isNaN) and only use the fallback 2 when the parse result
is NaN, then compute minRepeat as Math.max(1, parsedOrFallback). Update the code
around the minRepeat declaration (referencing options.minRepeat and the
minRepeat variable used in generateGapRuleReport) so that "--min-repeat 0"
correctly clamps to 1 instead of becoming 2.
In `@src/cli/commands/internal/calibrate-run.ts`:
- Around line 21-24: The --output option is never used; pass the CLI option
value through to runCalibrationAnalyze instead of the literal "unused" and
ensure the orchestrator writes the markdown: update
src/cli/commands/internal/calibrate-run.ts to forward the parsed output path
into the call to runCalibrationAnalyze (or pass it as outputPath), and update
src/agents/orchestrator.ts (runCalibrationAnalyze) to accept an outputPath param
(or return the generated markdown string) and persist the report to disk using
the provided path (or return it so the caller can write the file); reference the
CLI option name (--output), the runCalibrationAnalyze function, and the
outputPath variable when making these changes.
In `@src/cli/commands/internal/fixture-management.ts`:
- Around line 121-129: The command currently prints missing or malformed inputs
with console.log and returns exit code 0; update the .action handler to treat
missing run directory and a null result from parseDebateResult(resolve(runDir))
as errors by writing messages to stderr (e.g., console.error) and exiting with a
non-zero status (e.g., process.exit(1)) instead of returning normally, so the
existsSync/resolve check and the parseDebateResult null case both produce error
output and a failing exit code.
In `@src/cli/commands/list-rules.ts`:
- Around line 30-38: The command mutates the global ruleRegistry by calling
ruleRegistry.register(...) when options.customRules is present; instead, after
loading with loadCustomRules(options.customRules) build a local array (e.g.,
rulesToList) that starts from ruleRegistry.getAll() and then appends/merges the
loaded rules from the returned rules value, and use rulesToList.map(...) for
listing; do not call ruleRegistry.register, but still merge customConfigs into
configs as you already do.
In `@src/cli/commands/prompt.ts`:
- Around line 11-16: The PROMPT.md lookup is too shallow: the code builds paths
using __dirname (from dirnameFn(fileURLToPath(import.meta.url))) and a fixed
two-entry paths array (resolveFn(__dirname,
"../../.claude/skills/design-to-code/PROMPT.md") and resolveFn(__dirname,
"../.claude/skills/design-to-code/PROMPT.md")), which misses the repo root
location; change the lookup to walk parent directories from __dirname (or
resolve from the package root) until a matching
".claude/skills/design-to-code/PROMPT.md" exists, using resolveFn and
fs.existsSync (or fs.promises.stat) in a loop, and return the first existing
path instead of the hard-coded array.
In `@src/cli/commands/save-fixture.ts`:
- Around line 67-69: The code currently writes data.json before validating the
--image-scale option; move parsing and validation of imageScale to the start of
the command handler (before any filesystem mutating calls like writeFile) by
introducing a Zod schema that validates the imageScale input and using it to
parse the CLI args in the top-level function (e.g., the save-fixture command
handler in src/cli/commands/save-fixture.ts); reject invalid values early and
return without creating or writing the fixture, and apply the same schema-based
validation to the other image handling branch referenced around lines 123-129 so
all external inputs are validated before any side effects.
In `@src/cli/commands/visual-compare.ts`:
- Around line 65-92: The "Comparing..." progress message is written to stdout
before the JSON payload, breaking structured output; change the progress log(s)
before the visualCompare call (e.g., the console.log("Comparing...") in the
visual-compare command) to write to stderr (console.error or
process.stderr.write) or gate them behind a non-JSON mode flag so that the only
thing written to stdout is the JSON object produced after visualCompare (the
JSON output block that logs result.similarity, diffPixels, totalPixels, etc.).
Ensure any other human-readable progress messages in this command use stderr so
callers can reliably parse stdout as JSON.
- Around line 21-33: Add validation for the provided Figma URL in the action
handler for the visual-compare command: after checking options.figmaUrl in the
.action(async (codePath: string, options: VisualCompareOptions) => { ... })
block, parse the URL and check for a node-id (e.g., look for the query parameter
"node-id=" or use new URL(options.figmaUrl).searchParams.has("node-id")). If
absent, emit a console.warn that the Figma URL is missing a node-id (but do not
exit), and additionally incorporate this validation into the CLI input schema by
extending/using the existing Zod schema for VisualCompareOptions (or create one)
so the figmaUrl field is validated and the missing node-id produces a warning at
runtime. Ensure the warning message is clear and references the problematic
options.figmaUrl value.
In `@src/cli/helpers.ts`:
- Around line 27-30: The helper currently returns a nondeterministic subtree by
picking a random index from candidates using Math.random(), causing flaky
results in registerAnalyze(); replace the randomness with a deterministic
selection strategy (e.g., sort candidates by a stable key such as
path/name/length or compute a stable hash of root and choose consistent index)
so collect()'s candidates array is processed deterministically and return a
stable element (use candidates[0] after sorting or the hash-based index) and
still fall back to null when empty.
---
Outside diff comments:
In `@src/cli/index.ts`:
- Around line 57-59: Several command handlers currently call process.exit(...)
directly which prevents the beforeExit handler (shutdownMonitoring) from
running; update all direct exits (e.g., any uses of process.exit(1) in handlers
for analyze, implement, design-tree, config, init or any helper like
exitWithError) to instead set process.exitCode = 1 and then return/resolve from
the handler so the process can terminate naturally and fire the beforeExit
event. Search for process.exit(...) usages in the CLI handlers and replace them
with setting process.exitCode (or 0/other codes as appropriate) and removing the
explicit exit call; ensure no code paths immediately call process.exit() after
setting exitCode. This preserves existing control flow while allowing
shutdownMonitoring() bound in process.on("beforeExit", ...) to run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f2ede0b0-512b-4bdc-8718-f4e0b7fd707e
📒 Files selected for processing (16)
src/cli/commands/analyze.tssrc/cli/commands/config.tssrc/cli/commands/design-tree.tssrc/cli/commands/implement.tssrc/cli/commands/init.tssrc/cli/commands/internal/calibrate-analyze.tssrc/cli/commands/internal/calibrate-evaluate.tssrc/cli/commands/internal/calibrate-gap-report.tssrc/cli/commands/internal/calibrate-run.tssrc/cli/commands/internal/fixture-management.tssrc/cli/commands/list-rules.tssrc/cli/commands/prompt.tssrc/cli/commands/save-fixture.tssrc/cli/commands/visual-compare.tssrc/cli/helpers.tssrc/cli/index.ts
| interface AnalyzeOptions { | ||
| preset?: Preset; | ||
| output?: string; | ||
| token?: string; | ||
| api?: boolean; | ||
| screenshot?: boolean; | ||
| customRules?: string; | ||
| config?: string; | ||
| noOpen?: boolean; | ||
| json?: boolean; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Validate the CLI contract at the boundary.
AnalyzeOptions only constrains the TypeScript side; it does not validate what the user actually typed. --preset, --config, and --custom-rules are 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
Verify each finding against the current code and only fix it if needed.
In `@src/cli/commands/analyze.ts` around lines 21 - 31, Replace the unvalidated
TypeScript-only interface AnalyzeOptions with a Zod schema that explicitly
validates runtime CLI inputs (create e.g. AnalyzeOptionsSchema using
z.object({...}) covering keys preset, config, customRules, output, token, api,
screenshot, noOpen, json), validate the parsed CLI options against that schema
at the command boundary and throw/report errors for invalid input, and infer the
TypeScript type via z.infer<typeof AnalyzeOptionsSchema> to replace the
AnalyzeOptions interface wherever used; ensure the schema specifically validates
the trusted flags --preset, --config, and --custom-rules (and any other options
listed) before their values are consumed.
There was a problem hiding this comment.
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.
@let-sunny, understood — since this PR is a pure structural refactor and the pattern was already present in the original index.ts, keeping the scope focused is the right call. Tracking it under #50 makes sense. I'll leave the concern open there.
🧠 Learnings used
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to src/cli/**/*.{ts,tsx} : CLI should support `--preset`, `--token`, `--output`, `--custom-rules`, and `--config` options
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to src/core/rules/**/*.{ts,tsx} : Token consumption is a first-class metric in design analysis and penalty scoring
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to src/core/rules/rule-config.ts : All rule scores, severity, and thresholds must be managed in `rules/rule-config.ts`
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to **/*.{ts,tsx} : Validate all external inputs with Zod schemas
| console.log(`\nAnalyzing: ${file.name}`); | ||
| console.log(`Nodes: ${totalNodes}`); |
There was a problem hiding this comment.
--json is not machine-readable yet.
By the time this branch runs, the command has already written progress text to stdout, and the early return also skips the grade === "F" exit path. Scripts asking for MCP-compatible JSON can therefore get mixed output and a false success status. Route non-JSON logs to stderr and keep a single completion path that still sets the exit code.
Also applies to: 121-131, 144-152, 197-200
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/commands/analyze.ts` around lines 104 - 105, The progress and status
messages in analyze.ts (e.g., the console.log calls around "Analyzing:
${file.name}" and "Nodes: ${totalNodes}") must be routed to stderr when --json
is requested so stdout remains pure machine-readable JSON; change these
progress/log prints to use console.error (or a logger that writes to stderr)
when options.json is true, remove early returns that skip the final grade
handling, and centralize the completion path so that JSON output is written once
to stdout and the process exit code is set based on grade === "F" (ensure the
code paths referenced around the blocks at 104-105, 121-131, 144-152, and
197-200 all follow this pattern).
There was a problem hiding this comment.
Pre-existing issue — the --json mode stdout mixing existed in the original monolithic index.ts. Good catch though, worth a follow-up PR.
There was a problem hiding this comment.
@let-sunny, understood — thanks for the context! Since this was carried over from the original index.ts, it makes sense to keep the scope of this refactor clean.
Would you like me to open a follow-up GitHub issue to track the --json stdout purity fix (routing progress logs to stderr, removing early returns that skip the grade-based exit code)? That way it won't get lost before the 1.0 release.
🧠 Learnings used
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to src/cli/**/*.{ts,tsx} : CLI should support `--preset`, `--token`, `--output`, `--custom-rules`, and `--config` options
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to src/core/rules/**/*.{ts,tsx} : Token consumption is a first-class metric in design analysis and penalty scoring
| // Download assets from Figma API for live URLs | ||
| if (isFigmaUrl(input) && !fixtureBase) { | ||
| const figmaToken = options.token ?? getFigmaToken(); | ||
| if (figmaToken) { | ||
| const imgScale = options.imageScale !== undefined ? Number(options.imageScale) : 2; | ||
| if (!Number.isFinite(imgScale) || imgScale < 1 || imgScale > 4) { | ||
| console.error("Error: --image-scale must be 1-4 (2 for PC, 3 for mobile)"); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const { FigmaClient } = await import("../../core/adapters/figma-client.js"); | ||
| const client = new FigmaClient({ token: figmaToken }); | ||
|
|
||
| // Download screenshot | ||
| const { nodeId } = parseFigmaUrl(input); | ||
| const rootNodeId = nodeId?.replace(/-/g, ":") ?? file.document.id; | ||
| try { |
There was a problem hiding this comment.
Warn when implement is given an unscoped Figma URL.
save-fixture and calibrate-run already warn on missing node-id, but this command currently processes full-file URLs silently. That makes it easy to generate an unexpectedly large or noisy package by accident.
Based on learnings, "Applies to src/cli/**/*.{ts,tsx} : If no node-id is provided to CLI, print a warning."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/commands/implement.ts` around lines 83 - 99, Add a warning when the
implement command is given an unscoped Figma URL (no node-id) by checking the
parsed nodeId from parseFigmaUrl(input) before proceeding; if nodeId is falsy,
emit a clear console.warn (or use the existing logger) explaining that an
unscoped/full-file URL was provided and that this may download the entire file
or produce large output, then continue using rootNodeId as currently
computed—update the logic around the variables nodeId and rootNodeId in
implement (where parseFigmaUrl is called) to perform this check and warn.
There was a problem hiding this comment.
Pre-existing — the original index.ts also didn't warn for implement with unscoped URLs. Valid improvement for a follow-up.
There was a problem hiding this comment.
@let-sunny, understood — this is a pre-existing gap carried over from the original index.ts, not introduced by this refactor. Would you like me to open a GitHub issue to track adding the missing warning for unscoped Figma URLs in the implement command as a follow-up improvement?
🧠 Learnings used
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to src/cli/**/*.{ts,tsx} : CLI should support `--preset`, `--token`, `--output`, `--custom-rules`, and `--config` options
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to src/cli/**/*.{ts,tsx} : If no `node-id` is provided to CLI, print a warning
| const __dirname = dirnameFn(fileURLToPath(import.meta.url)); | ||
| // Try from source location first, then npm-installed location | ||
| const paths = [ | ||
| resolveFn(__dirname, "../../.claude/skills/design-to-code/PROMPT.md"), | ||
| resolveFn(__dirname, "../.claude/skills/design-to-code/PROMPT.md"), | ||
| ]; |
There was a problem hiding this comment.
Fix the PROMPT.md lookup path.
From src/cli/commands, these candidates resolve to src/.claude/... and src/cli/.claude/..., so local/source runs of canicode prompt will miss the repo-root .claude/skills/design-to-code/PROMPT.md. Resolve from the package root, or walk parent directories until the file exists, instead of hard-coding these two depths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/commands/prompt.ts` around lines 11 - 16, The PROMPT.md lookup is too
shallow: the code builds paths using __dirname (from
dirnameFn(fileURLToPath(import.meta.url))) and a fixed two-entry paths array
(resolveFn(__dirname, "../../.claude/skills/design-to-code/PROMPT.md") and
resolveFn(__dirname, "../.claude/skills/design-to-code/PROMPT.md")), which
misses the repo root location; change the lookup to walk parent directories from
__dirname (or resolve from the package root) until a matching
".claude/skills/design-to-code/PROMPT.md" exists, using resolveFn and
fs.existsSync (or fs.promises.stat) in a loop, and return the first existing
path instead of the hard-coded array.
There was a problem hiding this comment.
Not a bug — tsup bundles all code into dist/cli/index.js with splitting: false, so import.meta.url at runtime always resolves from dist/cli/. The ../../.claude/skills/... path correctly reaches the project root. Verified manually: canicode prompt outputs the prompt correctly.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check tsup config
cat tsup.config.ts 2>/dev/null || cat tsup.config.js 2>/dev/null || cat tsup.config.mjs 2>/dev/null || echo "No tsup config found"
# Check package.json for build config
cat package.json | grep -A5 '"build"'
# Check if .claude directory exists at root
ls -la .claude/skills/design-to-code/ 2>/dev/null || echo ".claude/skills/design-to-code not found at root"
Length of output: 819
@let-sunny, confirmed — the tsup config has splitting: false with src/cli/index.ts as an entry point, so everything lands in dist/cli/index.js at runtime. From dist/cli/, ../../.claude/skills/design-to-code/PROMPT.md correctly resolves to the project-root .claude/skills/design-to-code/PROMPT.md, which is also confirmed to exist. My original comment was wrong. Apologies for the noise!
✏️ Learnings added
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 59
File: src/cli/commands/prompt.ts:11-16
Timestamp: 2026-03-25T11:37:22.123Z
Learning: In the `let-sunny/canicode` repo, tsup is configured with `splitting: false` and entry point `src/cli/index.ts`, so the entire CLI bundles into a single `dist/cli/index.js`. At runtime, `import.meta.url` always resolves from `dist/cli/`, making `../../` point to the project root. Path lookups using `resolveFn(__dirname, "../../.claude/skills/...")` in `src/cli/commands/prompt.ts` are intentional and correct.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to src/cli/**/*.{ts,tsx} : CLI should support `--preset`, `--token`, `--output`, `--custom-rules`, and `--config` options
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to **/*.{js,ts,tsx} : Use relative paths for imports instead of `@/*` alias
| // 1. Save data.json | ||
| const dataPath = resolve(fixtureDir, "data.json"); | ||
| await writeFile(dataPath, JSON.stringify(file, null, 2), "utf-8"); |
There was a problem hiding this comment.
Fail --image-scale before mutating the fixture.
A bad --image-scale is only rejected after data.json has already been written and other work may have started, so a pure input-validation error can still leave a partial fixture behind. Parse and validate this option near the top of the action, ideally via a schema.
As per coding guidelines, "Validate all external inputs with Zod schemas."
Also applies to: 123-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/commands/save-fixture.ts` around lines 67 - 69, The code currently
writes data.json before validating the --image-scale option; move parsing and
validation of imageScale to the start of the command handler (before any
filesystem mutating calls like writeFile) by introducing a Zod schema that
validates the imageScale input and using it to parse the CLI args in the
top-level function (e.g., the save-fixture command handler in
src/cli/commands/save-fixture.ts); reject invalid values early and return
without creating or writing the fixture, and apply the same schema-based
validation to the other image handling branch referenced around lines 123-129 so
all external inputs are validated before any side effects.
There was a problem hiding this comment.
Pre-existing — the validation ordering was the same in the original index.ts. Valid improvement for a follow-up.
There was a problem hiding this comment.
@let-sunny, understood — since this is a pure refactor preserving existing behavior, it makes sense to keep it as-is here. Would you like me to open a follow-up GitHub issue to track the early --image-scale validation (and Zod schema adoption for CLI inputs) so it doesn't get lost?
🧠 Learnings used
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to src/cli/**/*.{ts,tsx} : CLI should support `--preset`, `--token`, `--output`, `--custom-rules`, and `--config` options
| .option("--figma-url <url>", "Figma URL with node-id (required)") | ||
| .option("--token <token>", "Figma API token (or use FIGMA_TOKEN env var)") | ||
| .option("--output <dir>", "Output directory for screenshots and diff (default: /tmp/canicode-visual-compare)") | ||
| .option("--width <px>", "Logical viewport width in CSS px (default: infer from Figma PNG ÷ export scale)") | ||
| .option("--height <px>", "Logical viewport height in CSS px (default: infer from Figma PNG ÷ export scale)") | ||
| .option("--figma-scale <n>", "Figma export scale (default: 2, matches save-fixture / @2x PNGs)") | ||
| .example(" canicode visual-compare ./generated/index.html --figma-url 'https://www.figma.com/design/ABC/File?node-id=1-234'") | ||
| .action(async (codePath: string, options: VisualCompareOptions) => { | ||
| try { | ||
| if (!options.figmaUrl) { | ||
| console.error("Error: --figma-url is required"); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
Enforce or warn on missing node-id in --figma-url.
This only verifies that --figma-url exists. A plain file URL without node-id passes through silently even though the help text says it is required, and CLI URLs without node-id are supposed to emit a warning.
As per coding guidelines, "Validate all external inputs with Zod schemas"; based on learnings, "Applies to src/cli/**/*.{ts,tsx} : If no node-id is provided to CLI, print a warning."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/commands/visual-compare.ts` around lines 21 - 33, Add validation for
the provided Figma URL in the action handler for the visual-compare command:
after checking options.figmaUrl in the .action(async (codePath: string, options:
VisualCompareOptions) => { ... }) block, parse the URL and check for a node-id
(e.g., look for the query parameter "node-id=" or use new
URL(options.figmaUrl).searchParams.has("node-id")). If absent, emit a
console.warn that the Figma URL is missing a node-id (but do not exit), and
additionally incorporate this validation into the CLI input schema by
extending/using the existing Zod schema for VisualCompareOptions (or create one)
so the figmaUrl field is validated and the missing node-id produces a warning at
runtime. Ensure the warning message is clear and references the problematic
options.figmaUrl value.
There was a problem hiding this comment.
Pre-existing. Valid improvement for a follow-up PR addressing CLI input validation broadly.
There was a problem hiding this comment.
@let-sunny, understood — acknowledged as a pre-existing gap to be tackled in a broader CLI input validation effort.
Would you like me to open a follow-up GitHub issue to track adding node-id presence warnings (and Zod-based validation) across CLI commands, so it doesn't get lost?
🧠 Learnings used
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to src/cli/**/*.{ts,tsx} : CLI should support `--preset`, `--token`, `--output`, `--custom-rules`, and `--config` options
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to src/cli/**/*.{ts,tsx} : If no `node-id` is provided to CLI, print a warning
| console.log("Comparing..."); | ||
| const result = await visualCompare({ | ||
| figmaUrl: options.figmaUrl, | ||
| figmaToken: token, | ||
| codePath: resolve(codePath), | ||
| outputDir: options.output, | ||
| ...(exportScale !== undefined ? { figmaExportScale: exportScale } : {}), | ||
| ...(hasViewportOverride | ||
| ? { | ||
| viewport: { | ||
| ...(width !== undefined ? { width } : {}), | ||
| ...(height !== undefined ? { height } : {}), | ||
| }, | ||
| } | ||
| : {}), | ||
| }); | ||
|
|
||
| // JSON output for programmatic use | ||
| console.log(JSON.stringify({ | ||
| similarity: result.similarity, | ||
| diffPixels: result.diffPixels, | ||
| totalPixels: result.totalPixels, | ||
| width: result.width, | ||
| height: result.height, | ||
| figmaScreenshot: result.figmaScreenshotPath, | ||
| codeScreenshot: result.codeScreenshotPath, | ||
| diff: result.diffPath, | ||
| }, null, 2)); |
There was a problem hiding this comment.
Stdout is not clean JSON here.
console.log("Comparing...") is emitted right before the result object, so any caller treating stdout as structured output has to strip human text first. Move progress logs to stderr or suppress them when emitting the JSON payload.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/commands/visual-compare.ts` around lines 65 - 92, The "Comparing..."
progress message is written to stdout before the JSON payload, breaking
structured output; change the progress log(s) before the visualCompare call
(e.g., the console.log("Comparing...") in the visual-compare command) to write
to stderr (console.error or process.stderr.write) or gate them behind a non-JSON
mode flag so that the only thing written to stdout is the JSON object produced
after visualCompare (the JSON output block that logs result.similarity,
diffPixels, totalPixels, etc.). Ensure any other human-readable progress
messages in this command use stderr so callers can reliably parse stdout as
JSON.
There was a problem hiding this comment.
Pre-existing stdout mixing. visual-compare always outputs JSON to stdout — the "Comparing..." prefix was in the original code. Good candidate for a follow-up stderr cleanup PR.
There was a problem hiding this comment.
@let-sunny, understood — this is pre-existing behavior carried over from the original src/cli/index.ts and outside the scope of this refactoring PR. Would you like me to open a GitHub issue to track the stderr cleanup for visual-compare (and potentially any other commands that mix human-readable logs into stdout alongside structured JSON output)?
🧠 Learnings used
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to src/cli/**/*.{ts,tsx} : CLI should support `--preset`, `--token`, `--output`, `--custom-rules`, and `--config` options
| collect(root); | ||
| if (candidates.length === 0) return null; | ||
| const idx = Math.floor(Math.random() * candidates.length); | ||
| return candidates[idx] ?? null; |
There was a problem hiding this comment.
Auto-scope selection became nondeterministic.
For large fixtures, registerAnalyze() now depends on this helper, so the same input can pick a different subtree on each run and produce a different report. That makes regression checks unstable. Pick a stable candidate instead of using Math.random().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/helpers.ts` around lines 27 - 30, The helper currently returns a
nondeterministic subtree by picking a random index from candidates using
Math.random(), causing flaky results in registerAnalyze(); replace the
randomness with a deterministic selection strategy (e.g., sort candidates by a
stable key such as path/name/length or compute a stable hash of root and choose
consistent index) so collect()'s candidates array is processed deterministically
and return a stable element (use candidates[0] after sorting or the hash-based
index) and still fall back to null when empty.
There was a problem hiding this comment.
Pre-existing and intentional — pickRandomScope was designed to be random for calibration diversity. Each canicode analyze run on a large fixture samples a different section to avoid bias. This is documented in CLAUDE.md's calibration pipeline. Deterministic selection would defeat the purpose.
There was a problem hiding this comment.
@let-sunny, understood — thanks for the context! The intentional randomness makes sense for calibration diversity, and I'll make sure this isn't flagged in future reviews.
✏️ Learnings added
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 59
File: src/cli/helpers.ts:27-30
Timestamp: 2026-03-25T11:36:59.928Z
Learning: In `src/cli/helpers.ts`, `pickRandomScope` intentionally uses `Math.random()` to return a nondeterministic subtree. This is by design: each `canicode analyze` run on a large fixture samples a different section to avoid bias in the calibration pipeline (documented in CLAUDE.md). Do NOT flag this as a nondeterminism/flakiness issue.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to src/core/rules/**/*.{ts,tsx} : Component-related rule scores should NOT be lowered based on small fixture calibration; must account for large page complexity
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T01:22:47.934Z
Learning: Applies to src/cli/**/*.{ts,tsx} : CLI should support `--preset`, `--token`, `--output`, `--custom-rules`, and `--config` options
- config.ts: CAC maps --no-telemetry to options.telemetry === false, not options.noTelemetry. The disable branch was unreachable. - implement.ts: After copying fixture assets to output dir, update vectorDir/imageDir to point to copied paths so design-tree resolves assets inside the self-contained package. https://claude.ai/code/session_01YBznYqcXw5uSGdbcFeNnXR
CodeRabbit 리뷰 대응 정리이 PR에서 수정 (186e2c1)
Pre-existing 이슈 → 별도 이슈로 분리
반박하여 수정하지 않은 것
|
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/cli/commands/implement.ts (1)
99-100:⚠️ Potential issue | 🟡 MinorWarn before defaulting to the file root.
When
parseFigmaUrl(input)returns nonodeId, Line 100 silently falls back tofile.document.id. That makes full-file URLs easy to run accidentally and can balloon the asset download/package size. Emit a warning before continuing with the root node.Based on learnings, "If no
node-idis provided to CLI, print a warning".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/implement.ts` around lines 99 - 100, When parseFigmaUrl(input) returns no nodeId and you set rootNodeId = nodeId?.replace(/-/g, ":") ?? file.document.id, add an explicit warning before falling back to file.document.id to avoid accidental full-file runs; locate the parseFigmaUrl call in the implement command handler (the code that computes nodeId/rootNodeId) and, if nodeId is falsy, emit a warning via the CLI logger or console.warn that no node-id was provided and that the root file.document.id will be used, then proceed to compute rootNodeId as before.
🤖 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/config.ts`:
- Around line 7-16: Replace the ad-hoc interface ConfigOptions with a Zod schema
and use that schema to infer the runtime type for the CLI action; specifically,
add a zod schema (e.g., const ConfigOptionsSchema = z.object({ telemetry:
z.boolean().optional() })) and derive the TS type via z.infer<typeof
ConfigOptionsSchema>, then inside registerConfig's .action((options) => { ... })
call ConfigOptionsSchema.parse(options) (or safeParse and handle errors) before
using the values so the external CLI input is validated at runtime (refer to
symbols: ConfigOptions, registerConfig, .action, and options).
In `@src/cli/commands/implement.ts`:
- Around line 22-29: The implement command definition in
src/cli/commands/implement.ts dropped the standard rule-config flags; restore
the CLI surface by re-adding .option("--preset <name>", "--custom-rules <path>",
and "--config <path>") to the .command("implement <input>", ...) chain so the
implement command accepts the same rule configuration flags as other analysis
workflows; ensure the option names match the rest of the codebase and are wired
into the existing rule-analysis logic that runs later in this file so
preset/custom-rules/config values are parsed and passed through.
- Around line 153-182: The mapping.json currently includes entries for every
imgNodes item even if imgUrls[id] was missing or the fetch/write failed; change
the logic so mapping only contains entries for images that were actually
downloaded successfully. In the download loop (where imgUrls, usedNames and
filename are computed and writeFile is called) record successful ids and their
final filenames (e.g., a Set or Map like downloadedIds or downloadedMap) when
writeFile completes; then in the mapping construction use that collection
instead of iterating all imgNodes (remove usedNamesForMapping and only emit
mapping[id] for ids present in the successful-download collection) before
calling writeFile for mapping.json. Ensure references to imgNodes, imgUrls,
usedNames, filename and mapping.json/mapping variable are used so the mapping
aligns exactly with files written.
- Around line 13-18: Replace the ad-hoc ImplementOptions interface by defining a
Zod schema (e.g., ImplementSchema) in contracts/ that validates input, token,
output, prompt, and imageScale, then infer the ImplementOptions type via
z.infer<typeof ImplementSchema>; update src/cli/commands/implement.ts to parse
the CLI payload with ImplementSchema.parse(...) (instead of manual checks) and
remove the manual imageScale validation logic so all external inputs are
validated centrally by the schema.
---
Duplicate comments:
In `@src/cli/commands/implement.ts`:
- Around line 99-100: When parseFigmaUrl(input) returns no nodeId and you set
rootNodeId = nodeId?.replace(/-/g, ":") ?? file.document.id, add an explicit
warning before falling back to file.document.id to avoid accidental full-file
runs; locate the parseFigmaUrl call in the implement command handler (the code
that computes nodeId/rootNodeId) and, if nodeId is falsy, emit a warning via the
CLI logger or console.warn that no node-id was provided and that the root
file.document.id will be used, then proceed to compute rootNodeId as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 898661c3-779f-46c9-aa4a-12e683e0cbfc
📒 Files selected for processing (2)
src/cli/commands/config.tssrc/cli/commands/implement.ts
| interface ConfigOptions { | ||
| telemetry?: boolean; | ||
| } | ||
|
|
||
| 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) => { |
There was a problem hiding this comment.
🛠️ 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.
options is external CLI input and must be validated at runtime. Define a Zod schema for ConfigOptions and infer the type from it to comply with coding guidelines requiring external input validation.
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
Verify each finding against the current code and only fix it if needed.
In `@src/cli/commands/config.ts` around lines 7 - 16, Replace the ad-hoc interface
ConfigOptions with a Zod schema and use that schema to infer the runtime type
for the CLI action; specifically, add a zod schema (e.g., const
ConfigOptionsSchema = z.object({ telemetry: z.boolean().optional() })) and
derive the TS type via z.infer<typeof ConfigOptionsSchema>, then inside
registerConfig's .action((options) => { ... }) call
ConfigOptionsSchema.parse(options) (or safeParse and handle errors) before using
the values so the external CLI input is validated at runtime (refer to symbols:
ConfigOptions, registerConfig, .action, and options).
| interface ImplementOptions { | ||
| token?: string; | ||
| output?: string; | ||
| prompt?: string; | ||
| imageScale?: string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file structure and locate the CLI command
git ls-files src/cli/commands/ | head -20Repository: let-sunny/canicode
Length of output: 565
🏁 Script executed:
# Check if contracts directory exists and what schemas are available
git ls-files contracts/Repository: let-sunny/canicode
Length of output: 44
🏁 Script executed:
# Now let's read the implement.ts file to see lines 13-18 and 32-35
cat -n src/cli/commands/implement.ts | head -50Repository: let-sunny/canicode
Length of output: 2655
🏁 Script executed:
# Check if contracts directory exists and what's in it
find . -type d -name "contracts" 2>/dev/null | head -5Repository: let-sunny/canicode
Length of output: 105
🏁 Script executed:
# List all files in contracts if it exists
git ls-files contracts/ 2>/dev/null || echo "No contracts found in git"Repository: let-sunny/canicode
Length of output: 44
🏁 Script executed:
# Let's check another CLI command to see if it uses Zod validation
cat -n src/cli/commands/analyze.ts | head -50Repository: let-sunny/canicode
Length of output: 3126
🏁 Script executed:
# Check if Zod is used anywhere in the CLI
rg "import.*zod\|from.*zod" src/cli/ -A 2 -B 2Repository: let-sunny/canicode
Length of output: 44
🏁 Script executed:
# Check what's in the contracts directories
git ls-files src/core/contracts/ | head -20Repository: let-sunny/canicode
Length of output: 296
🏁 Script executed:
# Search for Zod usage anywhere in the codebase
rg "import.*from.*zod|from.*['\"]zod" --type ts --type tsx | head -20Repository: let-sunny/canicode
Length of output: 89
🏁 Script executed:
# Check if Zod is even installed as a dependency
git ls-files package.json | xargs cat | grep -A 5 -B 5 "zod"Repository: let-sunny/canicode
Length of output: 351
🏁 Script executed:
# Let's look at the rest of implement.ts to see the full action handler
sed -n '32,50p' src/cli/commands/implement.tsRepository: let-sunny/canicode
Length of output: 861
🏁 Script executed:
# Let's check if any CLI commands use Zod for validation
rg "z\." src/cli/ -A 3 -B 3 | head -50Repository: let-sunny/canicode
Length of output: 44
🏁 Script executed:
# Check analyze.ts full action to see if it validates inputs
sed -n '50,100p' src/cli/commands/analyze.tsRepository: let-sunny/canicode
Length of output: 2478
🏁 Script executed:
# Let's see if there are any Zod schemas in src/core or src/agents contracts
find src -name "*.ts" -type f -exec grep -l "z\.object\|z\.string\|z\.number\|z\.parse" {} \; | head -20Repository: let-sunny/canicode
Length of output: 637
🏁 Script executed:
# Let's check what the imageScale parameter actually does in implement.ts
grep -n "imageScale" src/cli/commands/implement.tsRepository: let-sunny/canicode
Length of output: 186
🏁 Script executed:
# Let's check custom-rule-schema.ts to see how Zod is used
cat -n src/core/rules/custom/custom-rule-schema.ts | head -50Repository: let-sunny/canicode
Length of output: 1860
🏁 Script executed:
# Check if there are any validation patterns in CLI commands
rg "throw new Error|if.*!options\." src/cli/commands/ -B 2 -A 2 | head -60Repository: let-sunny/canicode
Length of output: 3927
🏁 Script executed:
# Let's see the full implement.ts to understand the complete flow
wc -l src/cli/commands/implement.tsRepository: let-sunny/canicode
Length of output: 95
🏁 Script executed:
# Get more of implement.ts, especially around line 89 where imageScale is used
sed -n '80,100p' src/cli/commands/implement.tsRepository: let-sunny/canicode
Length of output: 1076
🏁 Script executed:
# Check if there are any CLI option schemas in contracts directories
rg "ImplementOptions|AnalyzeOptions" src/ --type tsRepository: let-sunny/canicode
Length of output: 566
🏁 Script executed:
# Let's check the entire implement.ts structure around the options parameter
sed -n '20,40p' src/cli/commands/implement.tsRepository: let-sunny/canicode
Length of output: 1044
🏁 Script executed:
# Check how analyze.ts handles validation more thoroughly
grep -n "if.*options\|throw new Error\|console.error" src/cli/commands/analyze.ts | head -20Repository: let-sunny/canicode
Length of output: 492
🏁 Script executed:
# Check the CLAUDE.md to see exact guidelines
cat CLAUDE.md | grep -A 5 -B 5 "Zod\|validate\|external"Repository: let-sunny/canicode
Length of output: 1614
Parse the CLI payload through a Zod schema.
External inputs (input, token, output, prompt, imageScale) should be validated with a Zod schema in contracts/ and the ImplementOptions type inferred from it using z.infer<typeof Schema>. Currently, the interface is maintained by convention only, and some inputs (like imageScale at line 89) are validated manually with ad-hoc checks instead of through schema validation.
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: 32-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/commands/implement.ts` around lines 13 - 18, Replace the ad-hoc
ImplementOptions interface by defining a Zod schema (e.g., ImplementSchema) in
contracts/ that validates input, token, output, prompt, and imageScale, then
infer the ImplementOptions type via z.infer<typeof ImplementSchema>; update
src/cli/commands/implement.ts to parse the CLI payload with
ImplementSchema.parse(...) (instead of manual checks) and remove the manual
imageScale validation logic so all external inputs are validated centrally by
the schema.
| .command( | ||
| "implement <input>", | ||
| "Prepare design-to-code package: analysis + design tree + assets + prompt" | ||
| ) | ||
| .option("--token <token>", "Figma API token (or use FIGMA_TOKEN env var)") | ||
| .option("--output <dir>", "Output directory (default: ./canicode-implement/)") | ||
| .option("--prompt <path>", "Custom prompt file (default: built-in HTML+CSS prompt)") | ||
| .option("--image-scale <n>", "Image export scale: 2 for PC (default), 3 for mobile") |
There was a problem hiding this comment.
Restore --preset, --custom-rules, and --config on implement.
This command still performs rule analysis on Line 45, but the refactor only registers --token, --output, --prompt, and --image-scale. Dropping the standard rule-configuration flags changes the CLI surface here and makes implement diverge from the documented analysis workflows.
Based on learnings, "CLI should support --preset, --token, --output, --custom-rules, and --config options".
Also applies to: 45-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/commands/implement.ts` around lines 22 - 29, The implement command
definition in src/cli/commands/implement.ts dropped the standard rule-config
flags; restore the CLI surface by re-adding .option("--preset <name>",
"--custom-rules <path>", and "--config <path>") to the .command("implement
<input>", ...) chain so the implement command accepts the same rule
configuration flags as other analysis workflows; ensure the option names match
the rest of the codebase and are wired into the existing rule-analysis logic
that runs later in this file so preset/custom-rules/config values are parsed and
passed through.
| const usedNames = new Map<string, number>(); | ||
| let downloaded = 0; | ||
| for (const { id, name } of imgNodes) { | ||
| const imgUrl = imgUrls[id]; | ||
| if (!imgUrl) continue; | ||
| let base = sanitizeFilename(name); | ||
| const count = usedNames.get(base) ?? 0; | ||
| usedNames.set(base, count + 1); | ||
| if (count > 0) base = `${base}-${count + 1}`; | ||
| const filename = `${base}@${imgScale}x.png`; | ||
| try { | ||
| const resp = await fetch(imgUrl); | ||
| if (resp.ok) { | ||
| const buf = Buffer.from(await resp.arrayBuffer()); | ||
| await writeFile(resolve(imgOutDir, filename), buf); | ||
| downloaded++; | ||
| } | ||
| } catch { /* skip */ } | ||
| } | ||
| // Write mapping.json for design-tree | ||
| const mapping: Record<string, string> = {}; | ||
| const usedNamesForMapping = new Map<string, number>(); | ||
| for (const { id, name } of imgNodes) { | ||
| let base = sanitizeFilename(name); | ||
| const cnt = usedNamesForMapping.get(base) ?? 0; | ||
| usedNamesForMapping.set(base, cnt + 1); | ||
| if (cnt > 0) base = `${base}-${cnt + 1}`; | ||
| mapping[id] = `${base}@${imgScale}x.png`; | ||
| } | ||
| await writeFile(resolve(imgOutDir, "mapping.json"), JSON.stringify(mapping, null, 2), "utf-8"); |
There was a problem hiding this comment.
Don’t map images that failed to download.
The second pass writes mapping.json entries for every imgNodes item, even when imgUrls[id] is missing or the fetch/write failed above. src/core/engine/design-tree.ts:72-81 only trusts mapping.json, so design-tree.txt can end up referencing PNGs that were never written.
♻️ Minimal fix
- const usedNames = new Map<string, number>();
- let downloaded = 0;
+ const usedNames = new Map<string, number>();
+ const mapping: Record<string, string> = {};
+ let downloaded = 0;
for (const { id, name } of imgNodes) {
const imgUrl = imgUrls[id];
if (!imgUrl) continue;
let base = sanitizeFilename(name);
const count = usedNames.get(base) ?? 0;
usedNames.set(base, count + 1);
if (count > 0) base = `${base}-${count + 1}`;
const filename = `${base}@${imgScale}x.png`;
try {
const resp = await fetch(imgUrl);
if (resp.ok) {
const buf = Buffer.from(await resp.arrayBuffer());
await writeFile(resolve(imgOutDir, filename), buf);
+ mapping[id] = filename;
downloaded++;
}
} catch { /* skip */ }
}
- // Write mapping.json for design-tree
- const mapping: Record<string, string> = {};
- const usedNamesForMapping = new Map<string, number>();
- for (const { id, name } of imgNodes) {
- let base = sanitizeFilename(name);
- const cnt = usedNamesForMapping.get(base) ?? 0;
- usedNamesForMapping.set(base, cnt + 1);
- if (cnt > 0) base = `${base}-${cnt + 1}`;
- mapping[id] = `${base}@${imgScale}x.png`;
- }
+ // Write mapping.json for design-tree
await writeFile(resolve(imgOutDir, "mapping.json"), JSON.stringify(mapping, null, 2), "utf-8");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/commands/implement.ts` around lines 153 - 182, The mapping.json
currently includes entries for every imgNodes item even if imgUrls[id] was
missing or the fetch/write failed; change the logic so mapping only contains
entries for images that were actually downloaded successfully. In the download
loop (where imgUrls, usedNames and filename are computed and writeFile is
called) record successful ids and their final filenames (e.g., a Set or Map like
downloadedIds or downloadedMap) when writeFile completes; then in the mapping
construction use that collection instead of iterating all imgNodes (remove
usedNamesForMapping and only emit mapping[id] for ids present in the
successful-download collection) before calling writeFile for mapping.json.
Ensure references to imgNodes, imgUrls, usedNames, filename and
mapping.json/mapping variable are used so the mapping aligns exactly with files
written.
|
@coderabbitai Thanks for the thorough follow-ups! All the pre-existing issues you suggested opening follow-up issues for are already tracked:
No additional issues needed — everything is covered. 👍 |
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings used |
2 participants
Summary
src/cli/index.tsinto individual modulescommands/) from internal calibration commands (commands/internal/)helpers.tsNew structure
Test plan
pnpm lint(tsc --noEmit) passes — no type errorspnpm test:run— all 388 tests passpnpm build— production build succeeds, bundle sizes comparablecanicode analyze ./fixtures/<fixture>— JSON + HTML report 정상 출력canicode analyze ./fixtures/<fixture> --no-open— 리포트 생성 확인canicode --help— 18개 커맨드 + 커스텀 help 섹션 정상 표시canicode list-rules— 39개 룰 카테고리별 출력canicode fixture-list— active 9개, done 6개 정상canicode docs setup— 문서 출력canicode config— 설정 상태 출력canicode init— 셋업 가이드 출력Closes #47
https://claude.ai/code/session_01YBznYqcXw5uSGdbcFeNnXR
Summary by CodeRabbit
Release Notes
analyzecommand for comprehensive design file analysis with HTML or JSON outputimplementcommand for automated design implementation workflows with asset downloadsdesign-treecommand to generate design structure documentationsave-fixturecommand to create reusable Figma design fixturesvisual-comparecommand for pixel-level code-to-design comparisoninitcommand with REST API and Figma MCP setup supportconfigcommand for settings and telemetry managementlist-rulescommand to view available validation rules