refactor: unify report UI — self-contained CSS + plugin rendering (#164)#165
Merged
refactor: unify report UI — self-contained CSS + plugin rendering (#164)#165
Conversation
- New: src/core/report-html/render.ts — pure template functions - CLI report-html/index.ts: wraps with HTML shell + calls renderReportBody - Web app: replaced ~150 lines of inline rendering with CanICode.renderReportBody(data) — one function call - Removed shared code injection (constants/utils/gauge) — no longer needed - Simplified build-web.sh: no more placeholder substitution - browser.ts: exports renderReportBody + ReportData type Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ering (#164) - Convert render.ts from Tailwind utility classes to semantic rpt-* CSS classes - Write self-contained CSS in app/shared/styles.css with responsive rules for plugin viewport - CLI report inlines CSS via __REPORT_CSS__ define (no Tailwind CDN dependency) - Web app drops Tailwind CDN, links shared CSS + inline web shell styles - Figma plugin consumes renderReportBody() instead of custom rendering (~200 lines removed) - Plugin uses event delegation on [data-node-id] for Focus in Figma - Platform-neutral "Go to node" wording replaces "Open in Figma" / "Figma →" - Add unit tests for ui-helpers (22 tests) and render functions (35 tests) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Delete app/shared/{constants,utils,gauge}.js — no longer injected into plugin
- Remove severityDotClass, severityScoreClass — replaced by severityDot, severityBadge
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughUnifies report rendering into a single pure HTML renderer ( Changes
Sequence Diagram(s)(omitted — changes are primarily renderer consolidation, tests, CSS and targeted runtime updates; no new multi-component control flow requiring a sequence diagram) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
mainComponent sync getter is not available with documentAccess: dynamic-page. Switch transformPluginNode and buildAnalysisFile to async, await all child transforms. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CSS absolute-positioned tooltips overflow the Figma plugin viewport. Native title attributes auto-position within viewport bounds. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
title attributes don't work in Figma plugin iframe, and CSS tooltips overflow the narrow viewport. The same information is already visible in category accordion descriptions and severity labels. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![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: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/shared/styles.css`:
- Around line 529-531: The .rpt-opps-link is being hidden at the plugin (~420px)
breakpoint which removes the only actionable element produced by
renderOpportunities(); update the CSS rule that sets .rpt-opps-link { display:
none; } inside that breakpoint so the link remains visible (e.g., remove the
display:none or change it to an inline/inline-block/visible display) at the
plugin breakpoint, ensuring .rpt-opps-link stays actionable in the small-plugin
layout while keeping any other layout adjustments intact.
- Around line 27-28: The CSS custom properties --font-sans and --font-mono
currently list mixed-case font-family identifiers without quotes; update their
values to quote any multi-word or mixed-case font family names (e.g., "Inter",
"SF Mono", "Segoe UI") so the declarations for --font-sans and --font-mono
conform to the linter rule and avoid warnings while preserving rendering.
In `@app/web/src/index.html`:
- Around line 155-160: Update the privacy copy inside the elements with class
"web-hint" to remove the absolute claim "Never sent to any server" and instead
state that the Figma Personal Access Token is stored locally in the browser and
is sent directly from the browser to Figma (via the X-FIGMA-TOKEN header) when
performing analysis or posting comments, and clarify that it is not proxied
through the application's backend; apply the same wording change to both hint
occurrences currently around the two web-hint blocks so both messages are
consistent.
- Around line 207-209: The error container (element with id="error-box" and its
children error-title/error-detail) must expose async errors to assistive tech;
update the markup for the element with id="error-box" to include an appropriate
live region (e.g., add role="alert" or aria-live="assertive" and optionally
aria-atomic="true") so screen readers announce error messages populated into
`#error-title` and `#error-detail` immediately.
- Around line 391-396: The button's previous status class isn't removed, so a
prior rpt-btn-fail can keep the button styled as failed even after success;
update both the success path (where btn.classList.add('rpt-btn-ok') is used) and
the failure/catch path (where btn.classList.add('rpt-btn-fail') is used) to
first remove any existing status classes (rpt-btn-ok and rpt-btn-fail) from btn
before adding the new one so the visual state always reflects the current
result.
In `@scripts/build-web.sh`:
- Around line 19-28: The step numbering in the build script is inconsistent:
update the echoed progress label for the browser build from echo "[1/2] Building
browser.global.js..." to reflect three steps (e.g., "[1/3]") so the sequence
matches the subsequent echo messages "[2/3] Copying index.html..." and "[3/3]
Copying shared styles + favicon..."; edit the literal string in the build-web.sh
line containing echo "[1/2] Building browser.global.js..." to "[1/3] Building
browser.global.js..." to fix the inconsistency.
In `@src/core/report-html/render.ts`:
- Around line 35-52: Create and export a Zod schema named ReportDataSchema that
mirrors the ReportData shape (including nested ScoreReport and AnalysisIssue
shapes needed by renderReportBody) and export type ReportData = z.infer<typeof
ReportDataSchema>; at the top of the module import z from "zod", define schemas
for the nested structures (guaranteeing scores.byCategory objects, percentage
numbers, scores.summary keys, fileName/fileKey strings, nodeCount/maxDepth
numbers, optional figmaToken, and an array schema for AnalysisIssue), then in
renderReportBody call ReportDataSchema.parse(data) (or safeParse and
throw/return an error) before accessing fields so accesses like
scores.byCategory[cat].percentage and scores.summary.* are validated at runtime.
In `@src/core/ui-helpers.test.ts`:
- Around line 13-28: Add explicit edge-case tests for gaugeColor to document
behavior outside the 0–100 range: in the existing describe("gaugeColor") block
add a new it block that asserts gaugeColor(-10) returns the red value "#ef4444"
and gaugeColor(150) returns the green value "#22c55e". Reference the gaugeColor
function in the test file (src/core/ui-helpers.test.ts) so the behavior for
negative and >100 inputs is covered and documented alongside the existing
threshold tests.
In `@src/core/ui-helpers.ts`:
- Around line 32-52: Both severityDot and severityBadge contain identical
mappings; consolidate by creating a single helper (e.g., severityClass or
severityToCss) that holds the Record<Severity,string> and have both severityDot
and severityBadge delegate to it (or replace them with the single exported
function) so the mapping is defined once; update references to use the new
function name and keep JSDoc comments indicating intended usage contexts
(.rpt-dot vs .rpt-issue-score) if you want to preserve semantic intent.
In `@tsup.config.ts`:
- Around line 2-4: The build currently calls
readFileSync("app/shared/styles.css") and will throw a generic ENOENT if the
file is missing; add an existence check (e.g., using fs.existsSync or a
try/catch around readFileSync) before assigning reportCss so you can log or
throw a descriptive error message referencing "app/shared/styles.css"; update
the tsup.config.ts code around the reportCss declaration (and any initialization
code that depends on it) to verify file existence and provide a clear error
explaining the missing/renamed styles.css.
🪄 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: a0fb6977-4fc0-4131-bb84-e823c5e145bf
📒 Files selected for processing (15)
app/figma-plugin/src/ui.template.htmlapp/shared/constants.jsapp/shared/gauge.jsapp/shared/styles.cssapp/shared/utils.jsapp/web/src/index.htmlscripts/build-plugin.shscripts/build-web.shsrc/browser.tssrc/core/report-html/index.tssrc/core/report-html/render.test.tssrc/core/report-html/render.tssrc/core/ui-helpers.test.tssrc/core/ui-helpers.tstsup.config.ts
💤 Files with no reviewable changes (3)
- app/shared/gauge.js
- app/shared/utils.js
- app/shared/constants.js
Comment on lines
+35
to
+52
| export interface ReportData { | ||
| fileName: string; | ||
| fileKey: string; | ||
| scores: ScoreReport; | ||
| issues: AnalysisIssue[]; | ||
| nodeCount: number; | ||
| maxDepth: number; | ||
| figmaToken?: string; | ||
| } | ||
|
|
||
| // ---- Main render ---- | ||
|
|
||
| /** | ||
| * Render the report body content (everything inside <main>). | ||
| * Does NOT include <html>, <head>, <body> — caller wraps as needed. | ||
| */ | ||
| export function renderReportBody(data: ReportData): string { | ||
| const { scores, issues, fileKey, figmaToken } = data; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the renderer can reuse an existing schema before adding a new one.
rg -n --type=ts 'ReportDataSchema|ScoreReportSchema|AnalysisIssueSchema|z\.object\(' src/core/contracts src/core/engine src/core/report-htmlRepository: let-sunny/canicode
Length of output: 961
🏁 Script executed:
# Check the structure of existing schemas
cat -n src/core/contracts/score.tsRepository: let-sunny/canicode
Length of output: 601
🏁 Script executed:
# Check the issue schema
cat -n src/core/contracts/issue.tsRepository: let-sunny/canicode
Length of output: 472
🏁 Script executed:
# Check how renderReportBody uses the data parameter
head -n 100 src/core/report-html/render.ts | tail -n +35Repository: let-sunny/canicode
Length of output: 2376
🏁 Script executed:
# Check if there's already a ReportDataSchema or similar
rg -n 'ReportDataSchema|ReportSchema' src/core/contracts/Repository: let-sunny/canicode
Length of output: 215
🏁 Script executed:
# Check full ReportSchema definition
cat -n src/core/contracts/report.tsRepository: let-sunny/canicode
Length of output: 1090
🏁 Script executed:
# Find where ScoreReport and AnalysisIssue are defined
rg -n 'type ScoreReport|interface ScoreReport|type AnalysisIssue|interface AnalysisIssue|export.*ScoreReport|export.*AnalysisIssue' src/core/report-html/Repository: let-sunny/canicode
Length of output: 184
🏁 Script executed:
# Check the imports in render.ts to see where these types come from
head -n 35 src/core/report-html/render.tsRepository: let-sunny/canicode
Length of output: 1115
🏁 Script executed:
# Find the ScoreReport type definition
rg -n 'type ScoreReport|interface ScoreReport' src/core/engine/scoring.ts -A 15Repository: let-sunny/canicode
Length of output: 449
🏁 Script executed:
# Find the AnalysisIssue type definition
rg -n 'type AnalysisIssue|interface AnalysisIssue' src/core/engine/rule-engine.ts -A 15Repository: let-sunny/canicode
Length of output: 423
Add Zod validation for ReportData at the renderer boundary.
renderReportBody() is now the shared entry point for plain-JS consumers (web shell, Figma plugin), so TypeScript types are not enforced at runtime. The function accesses unchecked fields like scores.byCategory[cat].percentage and scores.summary.* without guards. Per your TypeScript strict mode with noUncheckedIndexedAccess, this will fail at runtime if the payload is malformed.
Please create a Zod schema to validate ReportData and export type ReportData = z.infer<typeof ReportDataSchema>. This aligns with the coding guideline: "Validate all external inputs with Zod schemas" and "Infer TypeScript types from Zod schemas using z.infer<typeof Schema>".
Note: ScoreReport and AnalysisIssue are engine types with different structures than the existing IssueSchema and ReportSchema, so a new composite schema may be required rather than reusing contract schemas directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/report-html/render.ts` around lines 35 - 52, Create and export a Zod
schema named ReportDataSchema that mirrors the ReportData shape (including
nested ScoreReport and AnalysisIssue shapes needed by renderReportBody) and
export type ReportData = z.infer<typeof ReportDataSchema>; at the top of the
module import z from "zod", define schemas for the nested structures
(guaranteeing scores.byCategory objects, percentage numbers, scores.summary
keys, fileName/fileKey strings, nodeCount/maxDepth numbers, optional figmaToken,
and an array schema for AnalysisIssue), then in renderReportBody call
ReportDataSchema.parse(data) (or safeParse and throw/return an error) before
accessing fields so accesses like scores.byCategory[cat].percentage and
scores.summary.* are validated at runtime.
Owner
Author
There was a problem hiding this comment.
Won't fix — renderReportBody() is an internal rendering function. Its callers (generateHtmlReport, web app, plugin) all pass typed engine output (ScoreReport, AnalysisIssue[]). This is not a system boundary receiving external input, so Zod validation adds cost without value here.
Contributor
There was a problem hiding this comment.
@let-sunny, understood — renderReportBody() is an internal renderer whose inputs are already typed at the call sites, not an external boundary. I'll note this for future reviews.
( ˘ω˘ )づ🐇
✏️ Learnings added
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 165
File: src/core/report-html/render.ts:35-52
Timestamp: 2026-03-29T05:45:34.219Z
Learning: In let-sunny/canicode, `renderReportBody()` in `src/core/report-html/render.ts` is an internal rendering function. Its callers (generateHtmlReport, web app shell, Figma plugin) all pass typed engine output (`ScoreReport`, `AnalysisIssue[]`). It is NOT a system boundary receiving external input, so Zod validation of `ReportData` is intentionally omitted. Do NOT flag the absence of Zod validation on `renderReportBody` as a deficiency.
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-28T00:25:18.075Z
Learning: Applies to **/*.{ts,tsx} : Validate all external inputs with Zod schemas
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 147
File: src/agents/ablation/run-phase1.ts:35-50
Timestamp: 2026-03-28T00:13:07.729Z
Learning: In `src/agents/ablation/run-phase1.ts`, `isCacheValid` intentionally checks only artifact existence and a finite `similarity` value. Strict schema/version field validation was deliberately rejected because `computeRankings` already guards against missing baselines with `if (!b) continue`, so stale-schema cached results produce zero deltas rather than NaN. Do NOT flag the absence of schema validation in `isCacheValid` as a deficiency.
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 154
File: src/core/rules/structure/index.ts:296-304
Timestamp: 2026-03-29T00:24:13.455Z
Learning: In let-sunny/canicode, the `non-layout-container` rule in `src/core/rules/structure/index.ts` intentionally flags non-empty `SECTION` nodes (`children.length > 0`) because Figma `SECTION` is not a layout container by design — using it structurally with children is treated as semantic misuse. This predicate is considered sufficient and intentional; do not flag it as too broad.
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 162
File: src/core/rules/naming/index.ts:157-192
Timestamp: 2026-03-29T03:02:35.383Z
Learning: In let-sunny/canicode, `AnalysisNode.name` is declared as `name: z.string()` (required, non-optional) at line 75 of `src/core/contracts/figma-node.ts`. It is guaranteed to be a defined string at runtime for all nodes reaching rule check functions. Do not flag `node.name` usage in rule files as potentially undefined.
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 147
File: src/agents/ablation/run-phase1.ts:168-202
Timestamp: 2026-03-28T00:12:55.972Z
Learning: In `src/agents/ablation/run-phase1.ts`, skipping no-op fixture/type pairs from the `computeRankings` delta aggregation is intentional by design. Zero-padding skipped pairs would dilute the signal (e.g., `variable-references` strip on a fixture with no variables is semantically meaningless). Instead, `fixtureCount` (N column) is surfaced in the rankings table to indicate per-type coverage, keeping the denominator honest without polluting the signal. Do NOT flag the omission of zero-delta entries for skipped fixtures.
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T00:25:18.075Z
Learning: Applies to src/**/rules/**/*.ts : All rule scores, severity, and thresholds are managed in `rules/rule-config.ts` — rule logic and score config must be intentionally separated
Comment on lines
+13
to
+28
| describe("gaugeColor", () => { | ||
| it("returns green for pct >= 75", () => { | ||
| expect(gaugeColor(75)).toBe("#22c55e"); | ||
| expect(gaugeColor(100)).toBe("#22c55e"); | ||
| }); | ||
|
|
||
| it("returns amber for 50 <= pct < 75", () => { | ||
| expect(gaugeColor(50)).toBe("#f59e0b"); | ||
| expect(gaugeColor(74)).toBe("#f59e0b"); | ||
| }); | ||
|
|
||
| it("returns red for pct < 50", () => { | ||
| expect(gaugeColor(0)).toBe("#ef4444"); | ||
| expect(gaugeColor(49)).toBe("#ef4444"); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding edge case tests for boundary values outside 0-100.
The gaugeColor tests cover the threshold boundaries well, but don't verify behavior for edge cases like negative values or percentages above 100. While the current implementation handles these implicitly, explicit tests would document expected behavior.
🧪 Optional: add edge case tests
it("handles edge cases outside 0-100 range", () => {
expect(gaugeColor(-10)).toBe("#ef4444"); // negative → red
expect(gaugeColor(150)).toBe("#22c55e"); // >100 → green
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/ui-helpers.test.ts` around lines 13 - 28, Add explicit edge-case
tests for gaugeColor to document behavior outside the 0–100 range: in the
existing describe("gaugeColor") block add a new it block that asserts
gaugeColor(-10) returns the red value "#ef4444" and gaugeColor(150) returns the
green value "#22c55e". Reference the gaugeColor function in the test file
(src/core/ui-helpers.test.ts) so the behavior for negative and >100 inputs is
covered and documented alongside the existing threshold tests.
Owner
Author
There was a problem hiding this comment.
Won't fix — behavior for out-of-range values is implicit and correct (negative → red, >100 → green). Adding tests for inputs that never occur in practice doesn't add value.
Contributor
There was a problem hiding this comment.
@let-sunny, understood — if out-of-range values are never produced by the calling code, testing them would just add noise without meaningful coverage value. Happy to move on.
🧠 Learnings used
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 91
File: .claude/commands/calibrate-loop.md:36-40
Timestamp: 2026-03-26T01:35:55.319Z
Learning: In `let-sunny/canicode`, `.claude/commands/calibrate-loop.md` is an internal Claude Code subagent command prompt (not user-facing documentation). Wording like "A or higher (percentage >= 90)" is intentional — it provides both the letter grade and percentage threshold simultaneously so the subagent has unambiguous dual criteria. Suggesting to collapse these into a single label (e.g., "A+") is incorrect, as it removes the explicit percentage criterion the subagent needs.
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.
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T00:25:18.075Z
Learning: Applies to src/**/rules/**/*.ts : All rule scores, severity, and thresholds are managed in `rules/rule-config.ts` — rule logic and score config must be intentionally separated
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 93
File: src/core/rules/rule-config.ts:76-80
Timestamp: 2026-03-26T01:28:57.785Z
Learning: In the let-sunny/canicode repo, `src/core/rules/rule-config.ts` is automatically adjusted by a nightly calibration pipeline. Do NOT suggest adding inline comments to this file for calibration rationale — the change evidence is tracked in PR descriptions, commit messages, and `data/calibration-evidence.json` instead. Inline comments would create clutter as the file is frequently auto-modified.
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 165
File: src/core/report-html/render.ts:35-52
Timestamp: 2026-03-29T05:45:34.219Z
Learning: In let-sunny/canicode, `renderReportBody()` in `src/core/report-html/render.ts` is an internal rendering function. Its callers (generateHtmlReport, web app shell, Figma plugin) all pass typed engine output (`ScoreReport`, `AnalysisIssue[]`). It is NOT a system boundary receiving external input, so Zod validation of `ReportData` is intentionally omitted. Do NOT flag the absence of Zod validation on `renderReportBody` as a deficiency.
Comment on lines
+32
to
52
| /** Severity to CSS modifier class (used with .rpt-dot, .rpt-issue-score) */ | ||
| export function severityDot(sev: Severity): string { | ||
| const map: Record<Severity, string> = { | ||
| blocking: "bg-red-500", | ||
| risk: "bg-amber-500", | ||
| "missing-info": "bg-zinc-400", | ||
| suggestion: "bg-green-500", | ||
| blocking: "sev-blocking", | ||
| risk: "sev-risk", | ||
| "missing-info": "sev-missing", | ||
| suggestion: "sev-suggestion", | ||
| }; | ||
| return map[sev]; | ||
| } | ||
|
|
||
| /** Severity to Tailwind badge class (for report-html / web app) */ | ||
| /** Severity to CSS modifier class (used with .rpt-issue-score) */ | ||
| export function severityBadge(sev: Severity): string { | ||
| const map: Record<Severity, string> = { | ||
| blocking: "bg-red-500/10 text-red-600 border-red-500/20", | ||
| risk: "bg-amber-500/10 text-amber-600 border-amber-500/20", | ||
| "missing-info": "bg-zinc-500/10 text-zinc-600 border-zinc-500/20", | ||
| suggestion: "bg-green-500/10 text-green-600 border-green-500/20", | ||
| blocking: "sev-blocking", | ||
| risk: "sev-risk", | ||
| "missing-info": "sev-missing", | ||
| suggestion: "sev-suggestion", | ||
| }; | ||
| return map[sev]; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider consolidating identical severityDot and severityBadge functions.
Both functions have identical implementations—same input type, same mapping, same return values. The JSDoc comments indicate different CSS class contexts (.rpt-dot vs .rpt-issue-score), but at runtime they're interchangeable.
If the semantic distinction is intentional (future divergence expected), this is fine. Otherwise, consider consolidating into a single severityClass(sev) helper to reduce duplication.
♻️ Optional: consolidate into single function
-/** Severity to CSS modifier class (used with .rpt-dot, .rpt-issue-score) */
-export function severityDot(sev: Severity): string {
- const map: Record<Severity, string> = {
- blocking: "sev-blocking",
- risk: "sev-risk",
- "missing-info": "sev-missing",
- suggestion: "sev-suggestion",
- };
- return map[sev];
-}
-
-/** Severity to CSS modifier class (used with .rpt-issue-score) */
-export function severityBadge(sev: Severity): string {
- const map: Record<Severity, string> = {
- blocking: "sev-blocking",
- risk: "sev-risk",
- "missing-info": "sev-missing",
- suggestion: "sev-suggestion",
- };
- return map[sev];
-}
+/** Severity to CSS modifier class (used with .rpt-dot, .rpt-issue-score, etc.) */
+export function severityClass(sev: Severity): string {
+ const map: Record<Severity, string> = {
+ blocking: "sev-blocking",
+ risk: "sev-risk",
+ "missing-info": "sev-missing",
+ suggestion: "sev-suggestion",
+ };
+ return map[sev];
+}
+
+// Re-export aliases for semantic clarity if needed
+export const severityDot = severityClass;
+export const severityBadge = severityClass;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/ui-helpers.ts` around lines 32 - 52, Both severityDot and
severityBadge contain identical mappings; consolidate by creating a single
helper (e.g., severityClass or severityToCss) that holds the
Record<Severity,string> and have both severityDot and severityBadge delegate to
it (or replace them with the single exported function) so the mapping is defined
once; update references to use the new function name and keep JSDoc comments
indicating intended usage contexts (.rpt-dot vs .rpt-issue-score) if you want to
preserve semantic intent.
Owner
Author
There was a problem hiding this comment.
Won't fix — keeping separate functions for semantic clarity. They may diverge if badge styling needs different class composition in the future.
Contributor
There was a problem hiding this comment.
@let-sunny, understood — keeping severityDot and severityBadge as separate functions makes sense for future flexibility. I'll note this for future reviews.
✏️ Learnings added
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 165
File: src/core/ui-helpers.ts:32-52
Timestamp: 2026-03-29T05:46:00.767Z
Learning: In `src/core/ui-helpers.ts` (let-sunny/canicode), `severityDot` and `severityBadge` are intentionally kept as separate exported functions despite having identical implementations today. The separation is for semantic clarity and to allow independent divergence if badge styling needs different class composition in the future. Do NOT flag them as duplicates or suggest consolidation.
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-28T00:25:18.075Z
Learning: Applies to src/**/rules/**/*.ts : All rule scores, severity, and thresholds are managed in `rules/rule-config.ts` — rule logic and score config must be intentionally separated
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 165
File: src/core/report-html/render.ts:35-52
Timestamp: 2026-03-29T05:45:34.219Z
Learning: In let-sunny/canicode, `renderReportBody()` in `src/core/report-html/render.ts` is an internal rendering function. Its callers (generateHtmlReport, web app shell, Figma plugin) all pass typed engine output (`ScoreReport`, `AnalysisIssue[]`). It is NOT a system boundary receiving external input, so Zod validation of `ReportData` is intentionally omitted. Do NOT flag the absence of Zod validation on `renderReportBody` as a deficiency.
- Remove unused `desc` variable in render.ts (CI lint failure) - Keep .rpt-opps-link visible at narrow breakpoint (was hiding only action) - Quote font-family names in CSS variables - Reword privacy copy: token is sent to Figma, not "never sent" - Add role="alert" to error container for screen readers - Clear previous btn status class before setting new one (comment button) - Fix build-web.sh step numbering [1/2] → [1/3] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/figma-plugin/src/main.ts (1)
428-509:⚠️ Potential issue | 🟠 MajorAdd Zod validation and error handling to the UI message handler.
The handler awaits
buildAnalysisFile()andtransformPluginNode()without a try/catch block, meaning any thrown error will escape unhandled. Additionally, message properties (nodeId,width,height) are accessed via type casts rather than runtime validation.Per the coding guidelines ("Validate all external inputs with Zod schemas"), implement a discriminated union schema with safeParse and wrap the handler in try/catch:
Suggested implementation
+import { z } from "zod"; + +const UI_MESSAGE_SCHEMA = z.discriminatedUnion("type", [ + z.object({ type: z.literal("analyze-selection") }), + z.object({ type: z.literal("analyze-page") }), + z.object({ type: z.literal("focus-node"), nodeId: z.string().min(1) }), + z.object({ + type: z.literal("resize"), + width: z.number().int().positive(), + height: z.number().int().positive(), + }), +]); +type UiMessage = z.infer<typeof UI_MESSAGE_SCHEMA>; + -figma.ui.onmessage = async (msg: { type: string }) => { +figma.ui.onmessage = async (rawMsg: unknown) => { + const parsed = UI_MESSAGE_SCHEMA.safeParse(rawMsg); + if (!parsed.success) { + figma.ui.postMessage({ type: "error", message: "Invalid UI message." }); + return; + } + + const msg: UiMessage = parsed.data; + try { // existing branches... + } catch (error) { + figma.ui.postMessage({ + type: "error", + message: error instanceof Error ? error.message : "Analysis failed.", + }); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/figma-plugin/src/main.ts` around lines 428 - 509, Wrap the figma.ui.onmessage handler logic in a try/catch and validate incoming msg with a Zod discriminated-union schema (matching types "analyze-selection", "analyze-page", "focus-node", "resize"); use schema.safeParse(msg) and return/post an error if parsing fails. Inside the try, await buildAnalysisFile and transformPluginNode calls and keep using countNodes, but only after successful validation; validate runtime fields like nodeId (string) and width/height (numbers) from the parsed object rather than via type casts. On any caught error, call figma.ui.postMessage with type "error" and include a concise error message (and optional error.message) for debugging. Ensure all async operations are awaited and that control paths return after sending error responses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/figma-plugin/src/main.ts`:
- Around line 325-326: The code currently sets result.componentId = mainComp?.id
?? "" which forces an empty string when getMainComponentAsync() returns
undefined; instead only set result.componentId when a main component exists —
call (node as InstanceNode).getMainComponentAsync(), check if mainComp is
truthy, and assign result.componentId = mainComp.id only in that branch so the
property stays absent on unresolved instances (matches the AnalysisNode optional
contract).
In `@app/web/src/index.html`:
- Line 140: Several <button> elements lack an explicit type and default to
type="submit"; update every <button> with an onclick handler (e.g., the button
that calls openAuthModal()) to include type="button" so they do not trigger form
submission, i.e., add type="button" to each button element that invokes JS
handlers (the ones referencing openAuthModal() and the other onclick callbacks).
In `@src/core/report-html/render.ts`:
- Around line 249-254: The use of a non-null assertion in groupIssuesByCategory
is fragile; instead retrieve the bucket with
grouped.get(issue.rule.definition.category) into a local variable, check if it's
undefined, and either push the issue into the existing array or create a new
array and set it on the map (i.e., use a guard like const bucket =
grouped.get(...); if (bucket) bucket.push(issue); else grouped.set(...,
[issue])). This removes the "!" assertion while keeping logic around CATEGORIES
and AnalysisIssue intact.
- Around line 70-77: The loop that maps CATEGORIES accesses
scores.byCategory[cat] and assigns it to cs but then uses cs.percentage and
cs.issueCount without guarding against undefined (symbols: CATEGORIES,
scores.byCategory, cs, renderGaugeSvg, CATEGORY_LABELS), which breaks with
noUncheckedIndexedAccess; update the mapping to first fetch const cs =
scores.byCategory[cat] and skip or handle undefined (e.g., return an empty
string/null or render a placeholder) before referencing cs.percentage or
cs.issueCount so the template only uses cs when it is defined.
---
Outside diff comments:
In `@app/figma-plugin/src/main.ts`:
- Around line 428-509: Wrap the figma.ui.onmessage handler logic in a try/catch
and validate incoming msg with a Zod discriminated-union schema (matching types
"analyze-selection", "analyze-page", "focus-node", "resize"); use
schema.safeParse(msg) and return/post an error if parsing fails. Inside the try,
await buildAnalysisFile and transformPluginNode calls and keep using countNodes,
but only after successful validation; validate runtime fields like nodeId
(string) and width/height (numbers) from the parsed object rather than via type
casts. On any caught error, call figma.ui.postMessage with type "error" and
include a concise error message (and optional error.message) for debugging.
Ensure all async operations are awaited and that control paths return after
sending error responses.
🪄 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: 637f458b-43e5-4694-8032-035cd7f74edd
📒 Files selected for processing (7)
app/figma-plugin/src/main.tsapp/shared/styles.cssapp/web/src/index.htmlscripts/build-web.shsrc/core/report-html/index.tssrc/core/report-html/render.test.tssrc/core/report-html/render.ts
| <button onclick="openAuthModal()" class="px-2.5 py-1 text-xs font-medium border border-zinc-700 rounded-md hover:bg-zinc-800 transition-colors"> | ||
| Authorize | ||
| </button> | ||
| <button onclick="openAuthModal()" class="web-auth-btn">Authorize</button> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add type="button" to prevent default form submission behavior.
The HTMLHint linter flags these buttons as missing the type attribute. Without it, buttons default to type="submit" which can cause unexpected form submissions. While there's no enclosing <form> currently, adding type="button" is a defensive practice.
Suggested fixes
- <button onclick="openAuthModal()" class="web-auth-btn">Authorize</button>
+ <button type="button" onclick="openAuthModal()" class="web-auth-btn">Authorize</button>- <button onclick="closeAuthModal()" class="web-modal-close">×</button>
+ <button type="button" onclick="closeAuthModal()" class="web-modal-close">×</button>- <button onclick="saveAuth()" class="btn-primary">Authorize</button>
- <button onclick="clearAuth()" class="btn-secondary">Logout</button>
+ <button type="button" onclick="saveAuth()" class="btn-primary">Authorize</button>
+ <button type="button" onclick="clearAuth()" class="btn-secondary">Logout</button>- <button onclick="document.getElementById('figma-url').value=''; document.getElementById('figma-url').focus();"
- class="web-url-clear" title="Clear">×</button>
+ <button type="button" onclick="document.getElementById('figma-url').value=''; document.getElementById('figma-url').focus();"
+ class="web-url-clear" title="Clear">×</button>- <button id="analyze-btn" onclick="runAnalysis()" class="web-analyze-btn">Analyze</button>
+ <button type="button" id="analyze-btn" onclick="runAnalysis()" class="web-analyze-btn">Analyze</button>Also applies to: 150-150, 164-165, 191-191, 199-199
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 140-140: The type attribute must be present on elements.
(button-type-require)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/web/src/index.html` at line 140, Several <button> elements lack an
explicit type and default to type="submit"; update every <button> with an
onclick handler (e.g., the button that calls openAuthModal()) to include
type="button" so they do not trigger form submission, i.e., add type="button" to
each button element that invokes JS handlers (the ones referencing
openAuthModal() and the other onclick callbacks).
Owner
Author
There was a problem hiding this comment.
Won't fix — the button is not inside a form, so default submit behavior does not apply.
Contributor
There was a problem hiding this comment.
@let-sunny, that's a fair point — since none of these buttons are inside a <form> element, the type="submit" default has no practical effect here. Acknowledged, and I won't flag this again.
✏️ Learnings added
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 165
File: app/web/src/index.html:140-140
Timestamp: 2026-03-29T05:46:04.111Z
Learning: In let-sunny/canicode (app/web/src/index.html), buttons with onclick handlers are intentionally outside any <form> element, so the absence of type="button" is acceptable — the default type="submit" has no effect. Do NOT flag missing type="button" on buttons in this file.
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: let-sunny
Repo: let-sunny/canicode PR: 165
File: src/core/report-html/render.ts:35-52
Timestamp: 2026-03-29T05:45:34.219Z
Learning: In let-sunny/canicode, `renderReportBody()` in `src/core/report-html/render.ts` is an internal rendering function. Its callers (generateHtmlReport, web app shell, Figma plugin) all pass typed engine output (`ScoreReport`, `AnalysisIssue[]`). It is NOT a system boundary receiving external input, so Zod validation of `ReportData` is intentionally omitted. Do NOT flag the absence of Zod validation on `renderReportBody` as a deficiency.
Comment on lines
+70
to
+77
| ${CATEGORIES.map(cat => { | ||
| const cs = scores.byCategory[cat]; | ||
| return ` <a href="#cat-${cat}" class="rpt-gauge-item"> | ||
| ${renderGaugeSvg(cs.percentage, 100, 7)} | ||
| <span class="rpt-gauge-label">${CATEGORY_LABELS[cat]}</span> | ||
| <span class="rpt-gauge-count">${cs.issueCount} issues</span> | ||
| </a>`; | ||
| }).join("\n")} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if byCategory access causes type errors
# Look for how byCategory is typed and if there are existing guards
rg -n 'byCategory\[' src/core/report-html/render.ts -B2 -A2
# Check if there are any TypeScript errors in the build
echo "---"
rg -n 'Record<Category' src/core/engine/scoring.ts -A5Repository: let-sunny/canicode
Length of output: 1648
🏁 Script executed:
#!/bin/bash
# Check tsconfig.json for noUncheckedIndexedAccess setting
grep -r "noUncheckedIndexedAccess" . --include="*.json" -A2 -B2
echo "---"
# Get full context around line 161 in render.ts
sed -n '158,170p' src/core/report-html/render.ts
echo "---"
# Check full context of the map function around line 71
sed -n '69,80p' src/core/report-html/render.tsRepository: let-sunny/canicode
Length of output: 1282
Add undefined checks for indexed access to scores.byCategory with noUncheckedIndexedAccess enabled.
With noUncheckedIndexedAccess and noPropertyAccessFromIndexSignature both enabled in tsconfig.json, accessing scores.byCategory[cat] returns CategoryScoreResult | undefined. The code at lines 71 and 161 accesses properties on cs without guarding against undefined, causing TypeScript compilation errors.
Add an undefined check or non-null assertion:
const cs = scores.byCategory[cat];
if (!cs) return null; // or handle appropriately
// now cs is safely typed as CategoryScoreResultOr use optional chaining if the call site can handle undefined returns.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/report-html/render.ts` around lines 70 - 77, The loop that maps
CATEGORIES accesses scores.byCategory[cat] and assigns it to cs but then uses
cs.percentage and cs.issueCount without guarding against undefined (symbols:
CATEGORIES, scores.byCategory, cs, renderGaugeSvg, CATEGORY_LABELS), which
breaks with noUncheckedIndexedAccess; update the mapping to first fetch const cs
= scores.byCategory[cat] and skip or handle undefined (e.g., return an empty
string/null or render a placeholder) before referencing cs.percentage or
cs.issueCount so the template only uses cs when it is defined.
Owner
Author
There was a problem hiding this comment.
Won't fix — false positive. byCategory is Record<Category, CategoryScoreResult> (mapped type, not index signature). With noUncheckedIndexedAccess, accessing a mapped type via its key union does NOT add | undefined. pnpm lint passes.
Contributor
There was a problem hiding this comment.
@let-sunny, you're absolutely right — my apologies for the false positive. noUncheckedIndexedAccess only adds | undefined for index signatures (e.g., { [key: string]: V } or Record<string, V>). A mapped type over a finite key union like Record<Category, CategoryScoreResult> is exhaustively known to TypeScript, so all keys are guaranteed to be present and no | undefined is injected. The lint passing confirms this.
✏️ Learnings added
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 165
File: src/core/report-html/render.ts:70-77
Timestamp: 2026-03-29T05:46:15.548Z
Learning: In let-sunny/canicode, `scores.byCategory` in `src/core/report-html/render.ts` is typed as `Record<Category, CategoryScoreResult>` where `Category` is a finite string union (mapped type). TypeScript's `noUncheckedIndexedAccess` does NOT add `| undefined` for mapped types over a finite key union — only for index signatures. Do NOT flag access to `scores.byCategory[cat]` (where `cat: Category`) as potentially undefined.
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: let-sunny
Repo: let-sunny/canicode PR: 165
File: src/core/report-html/render.ts:35-52
Timestamp: 2026-03-29T05:45:34.219Z
Learning: In let-sunny/canicode, `renderReportBody()` in `src/core/report-html/render.ts` is an internal rendering function. Its callers (generateHtmlReport, web app shell, Figma plugin) all pass typed engine output (`ScoreReport`, `AnalysisIssue[]`). It is NOT a system boundary receiving external input, so Zod validation of `ReportData` is intentionally omitted. Do NOT flag the absence of Zod validation on `renderReportBody` as a deficiency.
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T00:25:18.075Z
Learning: Applies to **/*.{ts,tsx} : TypeScript `noUncheckedIndexedAccess` must be enabled - must check for undefined when accessing arrays/objects
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 158
File: src/core/rules/interaction/index.ts:64-75
Timestamp: 2026-03-29T01:43:45.901Z
Learning: In let-sunny/canicode, `hasStateInVariantProps()` in `src/core/rules/interaction/index.ts` intentionally scans ALL `VARIANT`-typed `componentPropertyDefinitions` (not just ones keyed as `"State"`) because Figma variant axis names are freeform — designers use `State`, `Status`, `Mode`, `Type`, or localized names. If any variant option value matches the state regex pattern, the state is considered present. Do not flag this as a false-positive risk.
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 147
File: src/agents/ablation/run-phase1.ts:168-202
Timestamp: 2026-03-28T00:12:55.972Z
Learning: In `src/agents/ablation/run-phase1.ts`, skipping no-op fixture/type pairs from the `computeRankings` delta aggregation is intentional by design. Zero-padding skipped pairs would dilute the signal (e.g., `variable-references` strip on a fixture with no variables is semantically meaningless). Instead, `fixtureCount` (N column) is surfaced in the rankings table to indicate per-type coverage, keeping the denominator honest without polluting the signal. Do NOT flag the omission of zero-delta entries for skipped fixtures.
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T00:25:18.075Z
Learning: Applies to **/*.{ts,tsx} : TypeScript `exactOptionalPropertyTypes` must be enabled - no explicit undefined assignment to optional properties
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 147
File: src/agents/ablation/run-phase1.ts:35-50
Timestamp: 2026-03-28T00:13:07.729Z
Learning: In `src/agents/ablation/run-phase1.ts`, `isCacheValid` intentionally checks only artifact existence and a finite `similarity` value. Strict schema/version field validation was deliberately rejected because `computeRankings` already guards against missing baselines with `if (!b) continue`, so stale-schema cached results produce zero deltas rather than NaN. Do NOT flag the absence of schema validation in `isCacheValid` as a deficiency.
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T00:25:18.075Z
Learning: Applies to src/**/rules/rule-config.ts : Component-related rule scores (missing-component, etc.) should NOT be lowered based on small fixture calibration
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 162
File: src/core/rules/naming/index.ts:157-192
Timestamp: 2026-03-29T03:02:35.383Z
Learning: In let-sunny/canicode, `AnalysisNode.name` is declared as `name: z.string()` (required, non-optional) at line 75 of `src/core/contracts/figma-node.ts`. It is guaranteed to be a defined string at runtime for all nodes reaching rule check functions. Do not flag `node.name` usage in rule files as potentially undefined.
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 154
File: src/core/rules/structure/index.ts:296-304
Timestamp: 2026-03-29T00:24:13.455Z
Learning: In let-sunny/canicode, the `non-layout-container` rule in `src/core/rules/structure/index.ts` intentionally flags non-empty `SECTION` nodes (`children.length > 0`) because Figma `SECTION` is not a layout container by design — using it structurally with children is treated as semantic misuse. This predicate is considered sufficient and intentional; do not flag it as too broad.
Learnt from: CR
Repo: let-sunny/canicode PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-28T00:25:18.075Z
Learning: Applies to src/**/rules/**/*.ts : All rule scores, severity, and thresholds are managed in `rules/rule-config.ts` — rule logic and score config must be intentionally separated
…sage - Only set componentId when mainComponent exists (keep property absent otherwise) - Wrap async onmessage handler in try/catch to surface errors to UI Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 (1)
app/figma-plugin/src/main.ts (1)
499-506:⚠️ Potential issue | 🟠 MajorAwait the
focus-nodelookup.Lines 501-506 create a detached promise chain that bypasses the outer try/catch block. Any rejection from
getNodeByIdAsync()or exception inside the callback will not be caught by the error handler at line 513. Keep this async operation within the handler's control flow by awaiting it:Proposed fix
if (msg.type === "focus-node") { const { nodeId } = msg as { type: string; nodeId: string }; - const node = figma.getNodeByIdAsync(nodeId).then((n) => { - if (n && "absoluteBoundingBox" in n) { - figma.viewport.scrollAndZoomIntoView([n as SceneNode]); - figma.currentPage.selection = [n as SceneNode]; - } - }); + const focusedNode = await figma.getNodeByIdAsync(nodeId); + if (focusedNode && "absoluteBoundingBox" in focusedNode) { + figma.viewport.scrollAndZoomIntoView([focusedNode as SceneNode]); + figma.currentPage.selection = [focusedNode as SceneNode]; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/figma-plugin/src/main.ts` around lines 499 - 506, The current handler creates a detached promise by calling figma.getNodeByIdAsync(nodeId).then(...), which bypasses the surrounding try/catch; change the "focus-node" branch to await the async lookup instead: use const n = await figma.getNodeByIdAsync(nodeId) and then check if n && "absoluteBoundingBox" in n before calling figma.viewport.scrollAndZoomIntoView([n as SceneNode]) and figma.currentPage.selection = [n as SceneNode], so any rejection or exception from getNodeByIdAsync or the callback is propagated into the outer try/catch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/figma-plugin/src/main.ts`:
- Line 430: Create a Zod discriminated union schema (e.g., MessageSchema) that
describes each plugin message variant (including the discriminant "type" and
branch-specific fields like nodeId, width, height), infer its TS type with
z.infer<typeof MessageSchema>, and in the figma.ui.onmessage handler parse the
incoming msg using MessageSchema.parse (or safeParse) before any branching; then
switch on parsed.type and use the strongly-typed parsed properties (instead of
inline `as` casts at the branches that read nodeId/width/height) so runtime
validation and compile-time typing are enforced.
---
Outside diff comments:
In `@app/figma-plugin/src/main.ts`:
- Around line 499-506: The current handler creates a detached promise by calling
figma.getNodeByIdAsync(nodeId).then(...), which bypasses the surrounding
try/catch; change the "focus-node" branch to await the async lookup instead: use
const n = await figma.getNodeByIdAsync(nodeId) and then check if n &&
"absoluteBoundingBox" in n before calling
figma.viewport.scrollAndZoomIntoView([n as SceneNode]) and
figma.currentPage.selection = [n as SceneNode], so any rejection or exception
from getNodeByIdAsync or the callback is propagated into the outer try/catch.
🪄 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: e2fca397-815f-4368-9a81-54a3893d46df
📒 Files selected for processing (1)
app/figma-plugin/src/main.ts
This was referenced Mar 29, 2026
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
리포트 UI를 단일 소스(
renderReportBody())로 통합하고, Tailwind CDN 의존성을 완전 제거합니다.Before
index.ts(369줄, TS 문자열 concat)index.html(inline JS,CanICode.renderReportBody())ui.template.html(383줄, 자체 렌더링)app/shared/styles.cssAfter
renderReportBody()→<style>인라인CanICode.renderReportBody()<link>→styles.cssCanICode.renderReportBody()__SHARED_STYLES_INJECT__render.ts+styles.css한 곳만Changes
1.
renderReportBody()추출 및 Tailwind → 자체 CSSsrc/core/report-html/render.ts— 공유 렌더링 함수 (Tailwind 유틸리티 →rpt-*시맨틱 클래스)app/shared/styles.css— 리포트 컴포넌트 CSS + 셸 CSS + 반응형 (@media max-width: 600px)src/core/ui-helpers.ts—severityDot(),severityBadge(),scoreBadgeStyle()→ 시맨틱 클래스 반환2. CLI 리포트
src/core/report-html/index.ts— Tailwind CDN<script>제거,__REPORT_CSS__define으로 CSS 빌드타임 인라인tsup.config.ts—__REPORT_CSS__에app/shared/styles.css내용 주입.cli-topbar인라인 CSS3. Web App
app/web/src/index.html— Tailwind CDN +tailwind.config제거<link rel="stylesheet" href="styles.css">+ 웹 셸 인라인 CSS (.web-topbar,.web-modal,.web-input등)scripts/build-web.sh—styles.cssdist 복사 추가4. Figma Plugin
app/figma-plugin/src/ui.template.html— 커스텀 렌더링 ~200줄 삭제 →CanICode.renderReportBody()한 줄로 대체[data-node-id]event delegation으로 "Focus in Figma" 처리scripts/build-plugin.sh—constants.js,utils.js,gauge.js인젝션 제거5. 플랫폼 중립 문구
<a href>딥링크 그대로 동작[data-node-id]클릭을focusNode()postMessage로 대체figmaToken있을 때만 렌더 (Plugin은 미전달 → 미렌더)6. 불필요 코드 삭제
app/shared/{constants,utils,gauge}.js—CanICode.*글로벌 alias 파일 3개 삭제severityDotClass(),severityScoreClass()— 레거시 헬퍼 삭제browser.tsexport 정리7. 테스트 추가
src/core/ui-helpers.test.ts— 16 tests (gaugeColor, scoreClass, escapeHtml, severityDot, severityBadge, scoreBadgeStyle, renderGaugeSvg)src/core/report-html/render.test.ts— 35 tests (renderReportBody, renderSummaryDot, renderOpportunities, renderCategory, renderSeverityGroup, renderIssueRow, XSS escape, platform-neutral wording)Stats
Test plan
pnpm build— 빌드 성공, CSS 별도 추출 없이 인라인pnpm lint— 타입 체크 통과pnpm test:run— 603 tests 전부 통과pnpm build:web—dist/에index.html,styles.css,browser.global.js생성pnpm build:plugin—dist/ui.html생성 (shared CSS + browser bundle 인라인)Closes #164
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Style
Tests