feat: structured violation messages + tab/rule-group report UI (#155)#167
Merged
feat: structured violation messages + tab/rule-group report UI (#155)#167
Conversation
Data layer:
- Add suggestion and optional guide fields to RuleViolation
- Split rule-messages.ts: each message function returns { message, suggestion, guide? }
- Update all 16 rule check functions to spread ViolationMsg
- Context-aware non-semantic-name suggestions per node type
Report UI restructure:
- Category tabs (scrollable, shadcn pill style) replace accordion
- Issues grouped by rule (not severity) — static Why/Impact/Fix shown once per rule
- Rule sections as <details> (blocking/risk open by default)
- Node accordion shows suggestion, guide, path, actions
- Opportunities section uses rule-based top N (total score)
- Gauge click → scroll to tabs, tab click → no scroll
- Opportunity click → activate category + focus rule
Other:
- Category order: Minor before Interaction
- Guide messages rewritten for designers (not config references)
- CLI topbar no longer sticky
- Tests rewritten for new structure (600 passing)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughRestructures rule violations into separate Changes
Sequence Diagram(s)sequenceDiagram
participant App as App (web / plugin)
participant Renderer as renderReportBody
participant Init as initReportInteractions
participant DOM as Report DOM
App->>Renderer: call renderReportBody(data)
Renderer-->>App: returns HTML (tabs + rule accordions)
App->>DOM: inject HTML into results container
App->>Init: call initReportInteractions(container)
Init->>DOM: attach handlers (tab clicks, panel toggle, opportunity clicks)
Note right of DOM: User -> click tab / click opportunity -> open rule <details>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
- Extract initReportInteractions() from inline <script> — innerHTML scripts don't execute - Web app and plugin call CanICode.initReportInteractions(el) after rendering - CLI inlines via toString() in generateHtmlReport - Reduce gauge padding (8px → 4px) to prevent overflow in plugin - Show thin scrollbar + fade gradient on tab list (narrow viewport only) - Tab click no longer scrolls, gauge click still scrolls to tabs 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/core/rules/component/index.ts (1)
133-149:⚠️ Potential issue | 🟠 MajorDelay Stage 1 dedupe until the first frame is actually emitted.
Line 141 records the component name in
seenStage1before Line 142 proves this node is the first repeated frame. Ifcheck()sees a later duplicate first, the real first occurrence is suppressed; with only two matching frames, Stage 1 disappears entirely. Move theseenStage1.add(...)into thefirstFrame === node.idbranch and add a regression test for the out-of-order call sequence.🛠️ Proposed fix
if (matchingComponent) { const seenStage1 = getSeenStage1(context); if ( sameNameFrames && firstFrame !== undefined && sameNameFrames.length >= 2 && !seenStage1.has(node.name.toLowerCase()) ) { - seenStage1.add(node.name.toLowerCase()); if (firstFrame === node.id) { + seenStage1.add(node.name.toLowerCase()); return { ruleId: missingComponentDef.id, subType: "unused-component" as const, nodeId: node.id, nodePath: context.path.join(" > "),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/rules/component/index.ts` around lines 133 - 149, The code records the component name in seenStage1 before confirming this node is the actual first repeated frame, which can suppress the true first occurrence; fix by moving the seenStage1.add(node.name.toLowerCase()) call inside the firstFrame === node.id branch (i.e., only add to getSeenStage1(context) after you verify this node is the first frame), and add a regression test that triggers an out-of-order call sequence (two matching frames seen where the later duplicate arrives first) to ensure Stage 1 dedupe behaves correctly; refer to matchingComponent, getSeenStage1, seenStage1.add, firstFrame, and node.id when locating the change.src/core/report-html/render.ts (1)
282-348: 🧹 Nitpick | 🔵 TrivialConsider extracting shared grouping logic.
getTopRulesandgroupIssuesByRuleshare nearly identical grouping logic (building aMap<string, RuleGroup>, populating fields, accumulating scores). The only differences are: (1)getTopRulessorts by score and slices to limit, (2)groupIssuesByRulesorts by severity then score.A shared helper could reduce duplication:
Sketch of refactor
function buildRuleGroups(issues: AnalysisIssue[]): Map<string, RuleGroup> { const byRule = new Map<string, RuleGroup>(); for (const issue of issues) { const id = issue.rule.definition.id; let group = byRule.get(id); if (!group) { const def = issue.rule.definition; group = { ruleId: id, ruleName: def.name, severity: issue.config.severity, severityClass: severityDot(issue.config.severity), why: def.why, impact: def.impact, fix: def.fix, issues: [], totalScore: 0, }; byRule.set(id, group); } group.issues.push(issue); group.totalScore += issue.calculatedScore; } return byRule; } function getTopRules(issues: AnalysisIssue[], limit: number): RuleGroup[] { return [...buildRuleGroups(issues).values()] .sort((a, b) => a.totalScore - b.totalScore) .slice(0, limit); } function groupIssuesByRule(issues: AnalysisIssue[]): RuleGroup[] { const SEVERITY_RANK: Record<string, number> = { blocking: 0, risk: 1, "missing-info": 2, suggestion: 3 }; return [...buildRuleGroups(issues).values()].sort((a, b) => { const sevDiff = (SEVERITY_RANK[a.severity] ?? 4) - (SEVERITY_RANK[b.severity] ?? 4); return sevDiff !== 0 ? sevDiff : a.totalScore - b.totalScore; }); }🤖 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 282 - 348, getTopRules and groupIssuesByRule duplicate the same grouping/accumulation logic; extract that into a helper (e.g., buildRuleGroups(issues: AnalysisIssue[]): Map<string, RuleGroup>) that creates the Map, initializes fields (using rule.definition, severityDot, etc.), pushes issues and increments totalScore, then replace the body of getTopRules and groupIssuesByRule to call buildRuleGroups(...).values() and apply their respective sorts/slice (getTopRules sorts by totalScore and slices to limit; groupIssuesByRule applies SEVERITY_RANK then totalScore).app/shared/styles.css (1)
549-550:⚠️ Potential issue | 🟡 MinorDead CSS selectors referencing removed structure.
.rpt-cat-headerand.rpt-sev-groupare styled in the responsive breakpoint, but these classes were removed from the renderer (replaced by tab/rule section structure). These selectors should be removed for consistency.Proposed fix
.rpt-opps-link { font-size: 11px; } - .rpt-cat-header { padding: 10px 14px; gap: 8px; } - .rpt-sev-group { padding: 10px 14px; } .rpt-issue-header { padding: 6px 10px; gap: 6px; font-size: 12px; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/shared/styles.css` around lines 549 - 550, Remove the dead CSS selectors .rpt-cat-header and .rpt-sev-group from the responsive breakpoint rules in the stylesheet where they no longer exist in the renderer; locate the media query block containing those selectors and delete the selector rules (and any duplicate declarations tied solely to them), and also search for any other references to .rpt-cat-header or .rpt-sev-group elsewhere in the CSS to remove or replace so styles remain consistent with the new tab/rule section structure.
🤖 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`:
- Line 381: Remove the empty CSS rule for .rpt-rule-body or replace it with a
minimal placeholder rule to satisfy stylelint; either delete the `.rpt-rule-body
{}` block entirely (since the renderer uses `.rpt-rule-issues` for content) or
add a non-empty rule such as a comment placeholder or a trivial property (e.g.,
.rpt-rule-body { /* reserved */ } or a harmless property) so block-no-empty is
no longer triggered and lint passes.
In `@src/core/report-html/render.ts`:
- Around line 231-247: The badge class is hardcoded to "score-red" causing
incorrect styling; update the span that currently uses class="rpt-badge
score-red" to derive the class from the rule's severity (use the existing
rg.severityClass or the helper severityBadge(rg.severity) if available) so the
badge reflects the rule's severity while keeping the displayed value as
rg.totalScore; locate this in the template that returns the rule details (the
code using the rg object in render.ts) and replace the static class with the
dynamic severity-based class.
---
Outside diff comments:
In `@app/shared/styles.css`:
- Around line 549-550: Remove the dead CSS selectors .rpt-cat-header and
.rpt-sev-group from the responsive breakpoint rules in the stylesheet where they
no longer exist in the renderer; locate the media query block containing those
selectors and delete the selector rules (and any duplicate declarations tied
solely to them), and also search for any other references to .rpt-cat-header or
.rpt-sev-group elsewhere in the CSS to remove or replace so styles remain
consistent with the new tab/rule section structure.
In `@src/core/report-html/render.ts`:
- Around line 282-348: getTopRules and groupIssuesByRule duplicate the same
grouping/accumulation logic; extract that into a helper (e.g.,
buildRuleGroups(issues: AnalysisIssue[]): Map<string, RuleGroup>) that creates
the Map, initializes fields (using rule.definition, severityDot, etc.), pushes
issues and increments totalScore, then replace the body of getTopRules and
groupIssuesByRule to call buildRuleGroups(...).values() and apply their
respective sorts/slice (getTopRules sorts by totalScore and slices to limit;
groupIssuesByRule applies SEVERITY_RANK then totalScore).
In `@src/core/rules/component/index.ts`:
- Around line 133-149: The code records the component name in seenStage1 before
confirming this node is the actual first repeated frame, which can suppress the
true first occurrence; fix by moving the seenStage1.add(node.name.toLowerCase())
call inside the firstFrame === node.id branch (i.e., only add to
getSeenStage1(context) after you verify this node is the first frame), and add a
regression test that triggers an out-of-order call sequence (two matching frames
seen where the later duplicate arrives first) to ensure Stage 1 dedupe behaves
correctly; refer to matchingComponent, getSeenStage1, seenStage1.add,
firstFrame, and node.id when locating the change.
🪄 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: ead7c0b0-51d1-400f-b51d-5f63a57ef49e
📒 Files selected for processing (14)
app/shared/styles.csssrc/core/contracts/category.tssrc/core/contracts/rule.tssrc/core/report-html/index.tssrc/core/report-html/render.test.tssrc/core/report-html/render.tssrc/core/rules/component/index.tssrc/core/rules/component/missing-component.test.tssrc/core/rules/interaction/index.tssrc/core/rules/naming/index.tssrc/core/rules/naming/non-standard-naming.test.tssrc/core/rules/rule-messages.tssrc/core/rules/structure/index.tssrc/core/rules/token/index.ts
💤 Files with no reviewable changes (1)
- src/core/report-html/index.ts
Comment on lines
+231
to
+247
| return ` <details class="card rpt-rule" data-rule="${esc(rg.ruleId)}"${isOpen ? " open" : ""}> | ||
| <summary class="rpt-rule-header"> | ||
| <span class="rpt-badge score-red">${rg.totalScore}</span> | ||
| <div class="rpt-rule-title"> | ||
| <span class="rpt-rule-name">${esc(rg.ruleName)}</span> | ||
| <span class="rpt-rule-meta"> | ||
| <span class="rpt-dot-sm ${rg.severityClass}"></span> | ||
| ${esc(rg.severity)} · ${rg.issues.length} issues | ||
| </span> | ||
| <div class="rpt-rule-info"> | ||
| <p><strong>Why:</strong> ${esc(rg.why)}</p> | ||
| <p><strong>Impact:</strong> ${esc(rg.impact)}</p> | ||
| <p><strong>Fix:</strong> ${esc(rg.fix)}</p> | ||
| </div> | ||
| </div> | ||
| </div>`; | ||
| <svg class="rpt-rule-chevron no-print" fill="none" viewBox="0 0 24 24" stroke="currentColor" stroke-width="2"><path d="M19 9l-7 7-7-7"/></svg> | ||
| </summary> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Rule section badge always uses score-red.
The badge on line 233 hardcodes score-red class regardless of the rule's actual severity or whether the score is positive. For consistency with the issue-level severity badges (which use severityBadge(sev)), consider deriving the badge class from severity:
- <span class="rpt-badge score-red">${rg.totalScore}</span>
+ <span class="rpt-badge ${severityBadge(rg.severity)}">${rg.totalScore}</span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return ` <details class="card rpt-rule" data-rule="${esc(rg.ruleId)}"${isOpen ? " open" : ""}> | |
| <summary class="rpt-rule-header"> | |
| <span class="rpt-badge score-red">${rg.totalScore}</span> | |
| <div class="rpt-rule-title"> | |
| <span class="rpt-rule-name">${esc(rg.ruleName)}</span> | |
| <span class="rpt-rule-meta"> | |
| <span class="rpt-dot-sm ${rg.severityClass}"></span> | |
| ${esc(rg.severity)} · ${rg.issues.length} issues | |
| </span> | |
| <div class="rpt-rule-info"> | |
| <p><strong>Why:</strong> ${esc(rg.why)}</p> | |
| <p><strong>Impact:</strong> ${esc(rg.impact)}</p> | |
| <p><strong>Fix:</strong> ${esc(rg.fix)}</p> | |
| </div> | |
| </div> | |
| </div>`; | |
| <svg class="rpt-rule-chevron no-print" fill="none" viewBox="0 0 24 24" stroke="currentColor" stroke-width="2"><path d="M19 9l-7 7-7-7"/></svg> | |
| </summary> | |
| return ` <details class="card rpt-rule" data-rule="${esc(rg.ruleId)}"${isOpen ? " open" : ""}> | |
| <summary class="rpt-rule-header"> | |
| <span class="rpt-badge ${severityBadge(rg.severity)}">${rg.totalScore}</span> | |
| <div class="rpt-rule-title"> | |
| <span class="rpt-rule-name">${esc(rg.ruleName)}</span> | |
| <span class="rpt-rule-meta"> | |
| <span class="rpt-dot-sm ${rg.severityClass}"></span> | |
| ${esc(rg.severity)} · ${rg.issues.length} issues | |
| </span> | |
| <div class="rpt-rule-info"> | |
| <p><strong>Why:</strong> ${esc(rg.why)}</p> | |
| <p><strong>Impact:</strong> ${esc(rg.impact)}</p> | |
| <p><strong>Fix:</strong> ${esc(rg.fix)}</p> | |
| </div> | |
| </div> | |
| <svg class="rpt-rule-chevron no-print" fill="none" viewBox="0 0 24 24" stroke="currentColor" stroke-width="2"><path d="M19 9l-7 7-7-7"/></svg> | |
| </summary> |
🤖 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 231 - 247, The badge class is
hardcoded to "score-red" causing incorrect styling; update the span that
currently uses class="rpt-badge score-red" to derive the class from the rule's
severity (use the existing rg.severityClass or the helper
severityBadge(rg.severity) if available) so the badge reflects the rule's
severity while keeping the displayed value as rg.totalScore; locate this in the
template that returns the rule details (the code using the rg object in
render.ts) and replace the static class with the dynamic severity-based class.
Owner
Author
There was a problem hiding this comment.
Won't fix — all rule scores are negative deductions, so the badge is always red. This is intentional.
Contributor
There was a problem hiding this comment.
@let-sunny, understood! Since all rule scores are negative deductions by design, hardcoding score-red is the correct and intentional choice. I'll note this for future reviews.
ʕ •ᴥ•ʔ
✏️ Learnings added
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 167
File: src/core/report-html/render.ts:183-199
Timestamp: 2026-03-29T07:26:36.727Z
Learning: In `let-sunny/canicode`, rule-level `totalScore` values in `src/core/report-html/render.ts` are always negative (all rules produce score deductions, never positive contributions). Therefore, the `score-red` CSS class on the rule section badge in `renderRuleSection()` is intentionally hardcoded and should NOT be flagged as inconsistent or suggested to be replaced with a dynamic severity-based class.
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: let-sunny
Repo: let-sunny/canicode PR: 165
File: src/core/report-html/render.ts:249-254
Timestamp: 2026-03-29T05:46:16.648Z
Learning: In `let-sunny/canicode`, `groupIssuesByCategory()` in `src/core/report-html/render.ts` pre-initializes the Map with all `CATEGORIES` keys before iterating issues (line 251), so `grouped.get(issue.rule.definition.category)!` on line 252 is guaranteed non-null. Do NOT flag this non-null assertion as fragile or unsafe.
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
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.
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: 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 : `no-auto-layout` is the single highest-impact rule with score -10 — empirically validated via ablation experiments
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: 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 : Token consumption is a first-class metric — designs that waste tokens on repeated structures are penalized in scoring
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/cli/**/*.ts : Full-file analysis is discouraged — use section or page level analysis with `node-id` required in URL
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 : Rule scores can be tuned in `rule-config.ts` without touching rule logic
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 154
File: src/core/rules/token/index.ts:36-135
Timestamp: 2026-03-29T00:24:15.976Z
Learning: In let-sunny/canicode, `RuleCheckFn` in `src/core/rules/token/index.ts` (and all rule files) returns `RuleViolation | null` — only a single violation per node per rule is supported by the engine contract. The `rawValueCheck` returning on the first matched subtype is intentional. Multi-violation support per node requires engine contract changes, tracked in issue `#155`.
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, when a value is typed via a mapped type over a finite string-union key (e.g., `scores.byCategory: Record<Category, CategoryScoreResult>` where `Category` is a string union), TypeScript `noUncheckedIndexedAccess` does not introduce `| undefined` the way it does for index signatures. Therefore, do not flag `scores.byCategory[cat]` as potentially `undefined` when `cat: Category`.
- Tab list: wider gap + side padding on mobile so last tab visibly clips - Remove fade gradient (natural clipping is clearer) - Remove empty .rpt-rule-body rule and dead .rpt-cat-header/.rpt-sev-group selectors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Scrollbar was blocking tab clicks in Figma plugin. Fade gradient on right edge signals more tabs are available. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previously seenStage1.add() ran before checking firstFrame === node.id, so out-of-order traversal could suppress the real first occurrence. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 275-303: The gradient pseudo-element is currently on
.rpt-tabs::after and uses height: 100%, causing it to overlay the whole
container; move or re-target the pseudo-element to the tab strip so it only
covers the tab row (e.g., implement the pseudo on .rpt-tab-list::after) or
constrain the height to the tab bar by setting the pseudo-element height to
match the tab-list (use the .rpt-tab-list element or its computed height) and
keep its positioning relative to the tab-list; update selector references from
.rpt-tabs::after to .rpt-tab-list::after (or change height from 100% to the
tab-list height) and ensure pointer-events, border-radius and gradient remain
the same.
In `@src/core/report-html/render.ts`:
- Around line 221-226: The Comment-on-Figma button currently serializes only
v.message (so postComment() loses the new suggestion/guide content); update the
button's data-message payload to include the suggestion (and guide when present)
alongside the original message (e.g., concatenate esc(v.message) with
esc(v.suggestion) and optionally esc(v.guide) separated by clear
labels/newlines) so postComment() receives the full actionable text; modify the
button element that sets data-message (and any code that reads it) to use the
combined string while keeping data-file-key and data-node-id unchanged.
- Around line 241-280: initReportInteractions is adding a delegated click
listener every time it's called which multiplies behavior on rerenders; make it
idempotent by detecting prior initialization (e.g., set a flag on the container
like container.dataset.reportInteractionsInit or attach a property) and
early-return if already initialized, or alternatively register a named handler
once (use the same function reference for container.addEventListener so it can
be checked/removed before re-adding). Ensure the existing helper functions
(activate, focusRule) remain usable and that the guard references the same
container used to attach the delegated click listener so repeated calls do not
add duplicate handlers.
- Around line 157-171: The opportunity rows rendered from ruleGroups (the
template mapping over ruleGroups producing elements with class "rpt-opps-item"
and data attributes data-opp-rule/data-opp-cat) are non-semantic divs with
click-only behavior; change the outer <div class="rpt-opps-item"...> to a
<button type="button" class="rpt-opps-item" data-opp-rule=... data-opp-cat=...>
so rows become keyboard-focusable and screen-reader accessible, keep the
esc(rg.ruleId)/esc(rg.ruleName) usage and all inner markup the same, ensure
existing click handlers/listeners bound to ".rpt-opps-item" still work (or
update listeners if they expect a div), and add/reset minimal button CSS (e.g.,
remove default button styles) so visual appearance remains unchanged.
🪄 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: 01c720c3-6537-4f0b-b52c-fa8fd7826ff9
📒 Files selected for processing (8)
app/figma-plugin/src/ui.template.htmlapp/shared/styles.cssapp/web/src/index.htmlsrc/browser.tssrc/core/report-html/index.tssrc/core/report-html/render.test.tssrc/core/report-html/render.tssrc/core/rules/component/index.ts
Comment on lines
+157
to
171
| ${ruleGroups.map(rg => { | ||
| const barW = Math.round((Math.abs(rg.totalScore) / maxAbs) * 100); | ||
| const cat = rg.issues[0]?.rule.definition.category ?? ""; | ||
| return ` <div class="rpt-opps-item" data-opp-rule="${esc(rg.ruleId)}" data-opp-cat="${cat}" style="cursor:pointer"> | ||
| <div class="rpt-opps-info"> | ||
| <div class="rpt-opps-name">${esc(def.name)}</div> | ||
| <div class="rpt-opps-msg">${esc(issue.violation.message)}</div> | ||
| <div class="rpt-opps-name">${esc(rg.ruleName)}</div> | ||
| <div class="rpt-opps-msg">${rg.issues.length} issues</div> | ||
| </div> | ||
| <div class="rpt-opps-bar-wrap"> | ||
| <div class="rpt-opps-bar"> | ||
| <div class="rpt-opps-bar-fill" style="width:${barW}%"></div> | ||
| </div> | ||
| <span class="rpt-opps-score">${issue.calculatedScore}</span> | ||
| <span class="rpt-opps-score">${rg.totalScore}</span> | ||
| </div> | ||
| <a href="${link}" target="_blank" rel="noopener" data-node-id="${esc(issue.violation.nodeId)}" class="rpt-opps-link no-print">Go to node →</a> | ||
| </div>`; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd src/core/report-html && wc -l render.tsRepository: let-sunny/canicode
Length of output: 75
🏁 Script executed:
cd src/core/report-html && sed -n '150,180p' render.tsRepository: let-sunny/canicode
Length of output: 1229
🏁 Script executed:
cd src/core/report-html && sed -n '1,50p' render.tsRepository: let-sunny/canicode
Length of output: 1447
🏁 Script executed:
cd src/core/report-html && sed -n '130,200p' render.tsRepository: let-sunny/canicode
Length of output: 3079
🏁 Script executed:
cd src/core/report-html && grep -n "initReportInteractions" render.tsRepository: let-sunny/canicode
Length of output: 128
🏁 Script executed:
cd src && rg -n "initReportInteractions" --type ts --type tsx --type js --type jsxRepository: let-sunny/canicode
Length of output: 89
🏁 Script executed:
cd src && rg -n "initReportInteractions" --type ts --type jsRepository: let-sunny/canicode
Length of output: 581
🏁 Script executed:
cd src/core/report-html && sed -n '241,310p' render.tsRepository: let-sunny/canicode
Length of output: 2487
🏁 Script executed:
cd src && rg -n "role=" core/report-html/render.ts | head -20Repository: let-sunny/canicode
Length of output: 458
🏁 Script executed:
cd src && rg -n "tabindex" core/report-html/render.tsRepository: let-sunny/canicode
Length of output: 44
🏁 Script executed:
cd src && rg -n "keydown|keyup|keypress" core/report-html/render.tsRepository: let-sunny/canicode
Length of output: 44
🏁 Script executed:
cat README.md | head -100Repository: let-sunny/canicode
Length of output: 4350
🏁 Script executed:
cd src && git log --oneline -5Repository: let-sunny/canicode
Length of output: 135
🏁 Script executed:
cd src && rg -n "aria-" core/report-html/render.ts | head -20Repository: let-sunny/canicode
Length of output: 352
🏁 Script executed:
cd src/core/report-html && sed -n '105,125p' render.tsRepository: let-sunny/canicode
Length of output: 959
🏁 Script executed:
cd src && rg -n "aria-label|aria-describedby|role=" core/report-html/render.ts | wc -lRepository: let-sunny/canicode
Length of output: 63
🏁 Script executed:
cd src/core/report-html && sed -n '157,175p' render.tsRepository: let-sunny/canicode
Length of output: 860
Render opportunity rows as semantic interactive controls for keyboard and screen reader support.
These rows are non-semantic <div> elements with click handlers only, making them inaccessible to keyboard-only users and screen readers. Replace with <button type="button"> (keeping the same data-opp-rule and data-opp-cat attributes) so they are focusable, activatable via keyboard, and properly announced by assistive technologies.
🤖 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 157 - 171, The opportunity rows
rendered from ruleGroups (the template mapping over ruleGroups producing
elements with class "rpt-opps-item" and data attributes
data-opp-rule/data-opp-cat) are non-semantic divs with click-only behavior;
change the outer <div class="rpt-opps-item"...> to a <button type="button"
class="rpt-opps-item" data-opp-rule=... data-opp-cat=...> so rows become
keyboard-focusable and screen-reader accessible, keep the
esc(rg.ruleId)/esc(rg.ruleName) usage and all inner markup the same, ensure
existing click handlers/listeners bound to ".rpt-opps-item" still work (or
update listeners if they expect a div), and add/reset minimal button CSS (e.g.,
remove default button styles) so visual appearance remains unchanged.
Owner
Author
There was a problem hiding this comment.
Won't fix — these rows are handled by event delegation on the container, so keyboard/screen reader behavior is inherited. Converting to would require resetting all default button styles for no functional gain.
Contributor
There was a problem hiding this comment.
@let-sunny, understood — I'll respect that decision. I'll note this for future reviews.
✏️ Learnings added
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 167
File: src/core/report-html/render.ts:157-171
Timestamp: 2026-03-29T07:47:23.497Z
Learning: In `let-sunny/canicode`, the opportunity rows in `src/core/report-html/render.ts` (`renderOpportunities`) are intentionally rendered as `<div class="rpt-opps-item">` elements rather than `<button>` elements. Click interactions are handled via event delegation on the container. The maintainer has decided not to convert them to buttons (to avoid CSS resets), so do NOT flag these divs as inaccessible or suggest converting them to `<button>` elements.
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:249-254
Timestamp: 2026-03-29T05:46:16.648Z
Learning: In `let-sunny/canicode`, `groupIssuesByCategory()` in `src/core/report-html/render.ts` pre-initializes the Map with all `CATEGORIES` keys before iterating issues (line 251), so `grouped.get(issue.rule.definition.category)!` on line 252 is guaranteed non-null. Do NOT flag this non-null assertion as fragile or unsafe.
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: let-sunny
Repo: let-sunny/canicode PR: 167
File: src/core/report-html/render.ts:183-199
Timestamp: 2026-03-29T07:26:36.727Z
Learning: In `let-sunny/canicode`, rule-level `totalScore` values in `src/core/report-html/render.ts` are always negative (all rules produce score deductions, never positive contributions). Therefore, the `score-red` CSS class on the rule section badge in `renderRuleSection()` is intentionally hardcoded and should NOT be flagged as inconsistent or suggested to be replaced with a dynamic severity-based class.
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 161
File: src/core/rules/node-semantics.ts:102-104
Timestamp: 2026-03-29T02:24:54.672Z
Learning: In let-sunny/canicode (`src/core/rules/node-semantics.ts`), `isInteractiveNode()` is intentionally narrow: it returns `true` only for state-variant interactive elements (button, link, tab, input, toggle) detected via `getInteractiveType()`. Overlay and carousel nodes are a separate semantic category called "behavior triggers" — they require prototype interactions but do NOT have hover/focus/disabled state variants. Do not suggest broadening `isInteractiveNode` to include overlay/carousel; the three categories (state-variant interactive, overlay, carousel) are siblings, not a hierarchy.
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.
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: 161
File: src/core/rules/node-semantics.ts:157-160
Timestamp: 2026-03-29T02:11:22.940Z
Learning: In let-sunny/canicode, `isExcludedName()` from `src/core/rules/node-semantics.ts` is intentionally NOT used in interaction rules. Interaction detection uses `getInteractiveType()` (via `INTERACTIVE_PATTERNS`) as an independent classification path. `isExcludedName` only affects: naming rules (`src/core/rules/naming/index.ts`), auto-layout exemptions (`src/core/rules/rule-exceptions.ts`), and orchestrator-level node filtering (`src/agents/orchestrator.ts`). Do not flag `cta`, `filter`, or other terms in `EXCLUDED_NAME_PATTERN` as suppressing interactive control analysis.
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 : `no-auto-layout` is the single highest-impact rule with score -10 — empirically validated via ablation experiments
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 154
File: src/core/rules/token/index.ts:36-135
Timestamp: 2026-03-29T00:24:15.976Z
Learning: In let-sunny/canicode, `RuleCheckFn` in `src/core/rules/token/index.ts` (and all rule files) returns `RuleViolation | null` — only a single violation per node per rule is supported by the engine contract. The `rawValueCheck` returning on the first matched subtype is intentional. Multi-violation support per node requires engine contract changes, tracked in issue `#155`.
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, when a value is typed via a mapped type over a finite string-union key (e.g., `scores.byCategory: Record<Category, CategoryScoreResult>` where `Category` is a string union), TypeScript `noUncheckedIndexedAccess` does not introduce `| undefined` the way it does for index signatures. Therefore, do not flag `scores.byCategory[cat]` as potentially `undefined` when `cat: Category`.
- Move fade gradient from .rpt-tabs to .rpt-tab-list (was overlaying panels) - Include suggestion in Comment on Figma data-message - Make initReportInteractions idempotent (remove previous handler on re-init) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/core/report-html/render.ts (1)
1-11:⚠️ Potential issue | 🟡 MinorUpdate the module header to reflect the new DOM behavior.
The file now exports
initReportInteractions(), so describing it as a set of "pure template functions" is no longer accurate. Please update the header/JSDoc to make the browser-side event binding explicit.As per coding guidelines,
**/*.{ts,js}: If the code change introduces behavior that contradicts existing documentation (README.md, CLAUDE.md, JSDoc comments), flag it and suggest updating the relevant documentation to stay in sync.🤖 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 1 - 11, The module header JSDoc incorrectly labels this file as "pure template functions" even though it now exports runtime DOM behavior via initReportInteractions(); update the top comment to state that the file exports both template rendering helpers (used by generateHtmlReport() and CanICode.renderReportBody()) and browser-side event binding initialization (initReportInteractions()) so callers know it mutates/attaches DOM listeners, and add a short note advising to update any external docs (README.md/CLAUDE.md) that reference the old purity assumption.app/shared/styles.css (1)
114-125:⚠️ Potential issue | 🟡 MinorAdd
font: inheritto both button selectors for cross-browser consistency.Both
.rpt-gauge-item(lines 114–125) and.rpt-tab(lines 306–318) style native<button>elements without explicit font inheritance..rpt-gauge-itemlacks any font properties entirely, while.rpt-tabsetsfont-sizeandfont-weightbut omitsfont-family. Native buttons in Safari and Firefox may still use system control fonts, causing visual drift from the report typography. Addfont: inheritto both rules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/shared/styles.css` around lines 114 - 125, Both .rpt-gauge-item and .rpt-tab style native button elements but do not inherit the document font, which can cause Safari/Firefox to use system control fonts; update the CSS rules for .rpt-gauge-item and .rpt-tab to include font: inherit so they inherit font-family, size and weight consistently across browsers and match the report typography.
♻️ Duplicate comments (1)
src/core/report-html/render.ts (1)
221-226:⚠️ Potential issue | 🟠 MajorInclude
v.guidein the serialized Figma comment.The button now carries the suggestion, but Line 226 still drops the optional guide. That loses the exemption/edge-case context this PR adds whenever a guide is present, so one-click comments remain less actionable than the expanded issue body. Build
data-messagefrom[v.message, v.suggestion, v.guide].filter(Boolean).🤖 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 221 - 226, The serialized Figma comment button omits the optional guide (v.guide) causing lost context; update the data-message assembly used in the button inside renderHTML (the element using postComment and attributes data-file-key, data-node-id, data-message) to include v.guide by building the message from [v.message, v.suggestion, v.guide].filter(Boolean).join(" — ") (or equivalent) so the Comment on Figma button carries message, suggestion and guide when present.
🤖 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/core/report-html/render.ts`:
- Around line 241-286: The initReportInteractions function currently only
applies tab/gauge active state on clicks; to sync the initial visual state call
activate() once during setup: after defining activate (and before attaching the
click handler and assigning container._rptHandler) find the initial category
(e.g., container.querySelector(".rpt-tab.active") or the first
.rpt-tab/.rpt-gauge-item) and invoke activate(initialCat, false) so the
corresponding .rpt-gauge-item and panels receive the .active class on load; keep
the existing focusRule/handler logic and do not remove the handler assignment.
- Around line 184-196: The <summary class="rpt-rule-header"> block contains
non-phrasing elements (<div> and <p>) which violates HTML5; update the markup
inside the summary (the block that renders rg.ruleName, rg.severity, rg.issues
count and the rpt-rule-info content) to use only phrasing/heading-level elements
such as <span> (or heading tags) and inline wrappers instead of <div>/<p>,
preserve the same classes (e.g., rpt-rule-title, rpt-rule-meta, rpt-rule-info)
and keep the escaped values (esc(rg.ruleName), esc(rg.severity), esc(rg.why),
esc(rg.impact), esc(rg.fix), rg.issues.length, rg.totalScore) and existing CSS
hooks so layout/visuals remain identical while ensuring the summary contains
only phrasing content.
---
Outside diff comments:
In `@app/shared/styles.css`:
- Around line 114-125: Both .rpt-gauge-item and .rpt-tab style native button
elements but do not inherit the document font, which can cause Safari/Firefox to
use system control fonts; update the CSS rules for .rpt-gauge-item and .rpt-tab
to include font: inherit so they inherit font-family, size and weight
consistently across browsers and match the report typography.
In `@src/core/report-html/render.ts`:
- Around line 1-11: The module header JSDoc incorrectly labels this file as
"pure template functions" even though it now exports runtime DOM behavior via
initReportInteractions(); update the top comment to state that the file exports
both template rendering helpers (used by generateHtmlReport() and
CanICode.renderReportBody()) and browser-side event binding initialization
(initReportInteractions()) so callers know it mutates/attaches DOM listeners,
and add a short note advising to update any external docs (README.md/CLAUDE.md)
that reference the old purity assumption.
---
Duplicate comments:
In `@src/core/report-html/render.ts`:
- Around line 221-226: The serialized Figma comment button omits the optional
guide (v.guide) causing lost context; update the data-message assembly used in
the button inside renderHTML (the element using postComment and attributes
data-file-key, data-node-id, data-message) to include v.guide by building the
message from [v.message, v.suggestion, v.guide].filter(Boolean).join(" — ") (or
equivalent) so the Comment on Figma button carries message, suggestion and guide
when present.
🪄 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: bb1b34d5-abad-4d10-9bef-a0e381051908
📒 Files selected for processing (2)
app/shared/styles.csssrc/core/report-html/render.ts
- Replace <div>/<p> inside <summary> with <span> (phrasing content only) - Add display:block CSS for block-level appearance - First gauge item rendered with active class on initial load Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/core/report-html/render.ts`:
- Around line 354-358: SEVERITY_RANK is recreated each time groupIssuesByRule
runs; hoist it to module scope as a single constant (e.g., declare const
SEVERITY_RANK: Record<string, number> = { blocking: 0, risk: 1, "missing-info":
2, suggestion: 3 }; at the top of the file) and remove the local declaration
inside the function so the sort code (the sevDiff calculation and fallback to 4)
continues to reference the module-scoped SEVERITY_RANK without reallocating it
on every call.
- Around line 106-124: The tab markup needs matching id/aria pairs: update the
button generation (where CATEGORIES.map returns the <button> with class
"rpt-tab" and uses CATEGORY_LABELS and scores.byCategory) to include a unique id
(e.g., `tab-${cat}`) and an aria-controls that points to the corresponding panel
id, and update the panel generation (where CATEGORIES.map returns the <div
class="rpt-tab-panel"...> using issuesByCategory and groupIssuesByRule and
renderRuleSection) to include a unique id (e.g., `panel-${cat}`) and
aria-labelledby that points back to the tab id; ensure the first tab/panel still
receives the "active" state and aria-selected="true" for the tab so the ids
remain stable and accessible.
🪄 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: a4d9bfda-41de-4bf2-8774-df69cc470864
📒 Files selected for processing (2)
app/shared/styles.csssrc/core/report-html/render.ts
Comment on lines
+106
to
+124
| <!-- Category Tabs --> | ||
| <div class="rpt-tabs"> | ||
| <div class="rpt-tab-list" role="tablist"> | ||
| ${CATEGORIES.map((cat, i) => { | ||
| const cs = scores.byCategory[cat]; | ||
| return ` <button type="button" role="tab" class="rpt-tab${i === 0 ? " active" : ""}" data-tab="${cat}" aria-selected="${i === 0}">${CATEGORY_LABELS[cat]} <span class="rpt-tab-count">${cs.issueCount}</span></button>`; | ||
| }).join("\n")} | ||
| </div> | ||
|
|
||
| <!-- Categories --> | ||
| <div class="rpt-cats"> | ||
| ${CATEGORIES.map(cat => renderCategory(cat, scores, issuesByCategory.get(cat) ?? [], fileKey, figmaToken)).join("\n")} | ||
| ${CATEGORIES.map((cat, i) => { | ||
| const catIssues = issuesByCategory.get(cat) ?? []; | ||
| const ruleGroups = groupIssuesByRule(catIssues); | ||
| return ` <div class="rpt-tab-panel${i === 0 ? " active" : ""}" data-panel="${cat}" role="tabpanel"> | ||
| ${ruleGroups.length === 0 | ||
| ? ' <div class="rpt-cat-empty">No issues found</div>' | ||
| : ruleGroups.map(rg => renderRuleSection(rg, fileKey, figmaToken)).join("\n") | ||
| } | ||
| </div>`; | ||
| }).join("\n")} |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding aria-controls and aria-labelledby for full WAI-ARIA tab pattern.
The tabs have proper role="tablist", role="tab" with aria-selected, and role="tabpanel". For complete accessibility, tabs should reference their panel via aria-controls and panels should reference their tab via aria-labelledby. This requires adding id attributes to both elements.
♿ Optional enhancement for tab accessibility
${CATEGORIES.map((cat, i) => {
const cs = scores.byCategory[cat];
- return ` <button type="button" role="tab" class="rpt-tab${i === 0 ? " active" : ""}" data-tab="${cat}" aria-selected="${i === 0}">${CATEGORY_LABELS[cat]} <span class="rpt-tab-count">${cs.issueCount}</span></button>`;
+ return ` <button type="button" role="tab" id="tab-${cat}" class="rpt-tab${i === 0 ? " active" : ""}" data-tab="${cat}" aria-selected="${i === 0}" aria-controls="panel-${cat}">${CATEGORY_LABELS[cat]} <span class="rpt-tab-count">${cs.issueCount}</span></button>`;
}).join("\n")}- return ` <div class="rpt-tab-panel${i === 0 ? " active" : ""}" data-panel="${cat}" role="tabpanel">
+ return ` <div class="rpt-tab-panel${i === 0 ? " active" : ""}" id="panel-${cat}" data-panel="${cat}" role="tabpanel" aria-labelledby="tab-${cat}">🤖 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 106 - 124, The tab markup needs
matching id/aria pairs: update the button generation (where CATEGORIES.map
returns the <button> with class "rpt-tab" and uses CATEGORY_LABELS and
scores.byCategory) to include a unique id (e.g., `tab-${cat}`) and an
aria-controls that points to the corresponding panel id, and update the panel
generation (where CATEGORIES.map returns the <div class="rpt-tab-panel"...>
using issuesByCategory and groupIssuesByRule and renderRuleSection) to include a
unique id (e.g., `panel-${cat}`) and aria-labelledby that points back to the tab
id; ensure the first tab/panel still receives the "active" state and
aria-selected="true" for the tab so the ids remain stable and accessible.
Owner
Author
There was a problem hiding this comment.
Won't fix — current implementation has role=tablist, role=tab, and aria-selected which covers the essential pattern. Full ARIA with aria-controls/aria-labelledby adds complexity for minimal gain in this context.
Contributor
There was a problem hiding this comment.
@let-sunny, understood — the essential WAI-ARIA tab semantics (role="tablist", role="tab", aria-selected) are already in place and the supplementary aria-controls/aria-labelledby pairing won't be flagged again.
✏️ Learnings added
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 167
File: src/core/report-html/render.ts:106-124
Timestamp: 2026-03-29T08:05:29.133Z
Learning: In `let-sunny/canicode`, the category tab component in `src/core/report-html/render.ts` intentionally omits `aria-controls` and `aria-labelledby` attributes. The maintainer considers `role="tablist"`, `role="tab"`, and `aria-selected` sufficient for the ARIA tab pattern. Do NOT flag the absence of `aria-controls`/`aria-labelledby` on these tab elements as an accessibility 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: let-sunny
Repo: let-sunny/canicode PR: 165
File: src/core/report-html/render.ts:249-254
Timestamp: 2026-03-29T05:46:16.648Z
Learning: In `let-sunny/canicode`, `groupIssuesByCategory()` in `src/core/report-html/render.ts` pre-initializes the Map with all `CATEGORIES` keys before iterating issues (line 251), so `grouped.get(issue.rule.definition.category)!` on line 252 is guaranteed non-null. Do NOT flag this non-null assertion as fragile or unsafe.
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 167
File: src/core/report-html/render.ts:157-171
Timestamp: 2026-03-29T07:47:23.497Z
Learning: In `let-sunny/canicode`, the opportunity rows in `src/core/report-html/render.ts` (`renderOpportunities`) are intentionally rendered as `<div class="rpt-opps-item">` elements rather than `<button>` elements. Click interactions are handled via event delegation on the container. The maintainer has decided not to convert them to buttons (to avoid CSS resets), so do NOT flag these divs as inaccessible or suggest converting them to `<button>` elements.
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: let-sunny
Repo: let-sunny/canicode PR: 167
File: src/core/report-html/render.ts:183-199
Timestamp: 2026-03-29T07:26:36.727Z
Learning: In `let-sunny/canicode`, rule-level `totalScore` values in `src/core/report-html/render.ts` are always negative (all rules produce score deductions, never positive contributions). Therefore, the `score-red` CSS class on the rule section badge in `renderRuleSection()` is intentionally hardcoded and should NOT be flagged as inconsistent or suggested to be replaced with a dynamic severity-based class.
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
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 161
File: src/core/rules/node-semantics.ts:102-104
Timestamp: 2026-03-29T02:24:54.672Z
Learning: In let-sunny/canicode (`src/core/rules/node-semantics.ts`), `isInteractiveNode()` is intentionally narrow: it returns `true` only for state-variant interactive elements (button, link, tab, input, toggle) detected via `getInteractiveType()`. Overlay and carousel nodes are a separate semantic category called "behavior triggers" — they require prototype interactions but do NOT have hover/focus/disabled state variants. Do not suggest broadening `isInteractiveNode` to include overlay/carousel; the three categories (state-variant interactive, overlay, carousel) are siblings, not a hierarchy.
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, when a value is typed via a mapped type over a finite string-union key (e.g., `scores.byCategory: Record<Category, CategoryScoreResult>` where `Category` is a string union), TypeScript `noUncheckedIndexedAccess` does not introduce `| undefined` the way it does for index signatures. Therefore, do not flag `scores.byCategory[cat]` as potentially `undefined` when `cat: Category`.
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 : `no-auto-layout` is the single highest-impact rule with score -10 — empirically validated via ablation experiments
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 154
File: src/core/rules/token/index.ts:36-135
Timestamp: 2026-03-29T00:24:15.976Z
Learning: In let-sunny/canicode, `RuleCheckFn` in `src/core/rules/token/index.ts` (and all rule files) returns `RuleViolation | null` — only a single violation per node per rule is supported by the engine contract. The `rawValueCheck` returning on the first matched subtype is intentional. Multi-violation support per node requires engine contract changes, tracked in issue `#155`.
Comment on lines
+354
to
+358
| const SEVERITY_RANK: Record<string, number> = { blocking: 0, risk: 1, "missing-info": 2, suggestion: 3 }; | ||
| return [...byRule.values()].sort((a, b) => { | ||
| const sevDiff = (SEVERITY_RANK[a.severity] ?? 4) - (SEVERITY_RANK[b.severity] ?? 4); | ||
| return sevDiff !== 0 ? sevDiff : a.totalScore - b.totalScore; | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider hoisting SEVERITY_RANK to module scope.
SEVERITY_RANK is recreated on every groupIssuesByRule call. Since it's a static lookup table, hoisting to module scope avoids repeated object allocation.
♻️ Optional: hoist constant
+const SEVERITY_RANK: Record<string, number> = { blocking: 0, risk: 1, "missing-info": 2, suggestion: 3 };
+
/** Group issues by rule within a category, sorted by severity then score */
function groupIssuesByRule(issues: AnalysisIssue[]): RuleGroup[] {
// ...
- const SEVERITY_RANK: Record<string, number> = { blocking: 0, risk: 1, "missing-info": 2, suggestion: 3 };
return [...byRule.values()].sort((a, b) => {🤖 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 354 - 358, SEVERITY_RANK is
recreated each time groupIssuesByRule runs; hoist it to module scope as a single
constant (e.g., declare const SEVERITY_RANK: Record<string, number> = {
blocking: 0, risk: 1, "missing-info": 2, suggestion: 3 }; at the top of the
file) and remove the local declaration inside the function so the sort code (the
sevDiff calculation and fallback to 4) continues to reference the module-scoped
SEVERITY_RANK without reallocating it on every call.
Owner
Author
There was a problem hiding this comment.
Won't fix — trivial allocation, function is called once per render.
Contributor
There was a problem hiding this comment.
@let-sunny, understood — once-per-render allocation is negligible. No action needed.
🧠 Learnings used
Learnt from: let-sunny
Repo: let-sunny/canicode PR: 165
File: src/core/report-html/render.ts:249-254
Timestamp: 2026-03-29T05:46:16.648Z
Learning: In `let-sunny/canicode`, `groupIssuesByCategory()` in `src/core/report-html/render.ts` pre-initializes the Map with all `CATEGORIES` keys before iterating issues (line 251), so `grouped.get(issue.rule.definition.category)!` on line 252 is guaranteed non-null. Do NOT flag this non-null assertion as fragile or unsafe.
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: 167
File: src/core/report-html/render.ts:183-199
Timestamp: 2026-03-29T07:26:36.727Z
Learning: In `let-sunny/canicode`, rule-level `totalScore` values in `src/core/report-html/render.ts` are always negative (all rules produce score deductions, never positive contributions). Therefore, the `score-red` CSS class on the rule section badge in `renderRuleSection()` is intentionally hardcoded and should NOT be flagged as inconsistent or suggested to be replaced with a dynamic severity-based class.
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.
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: let-sunny
Repo: let-sunny/canicode PR: 167
File: src/core/report-html/render.ts:157-171
Timestamp: 2026-03-29T07:47:23.497Z
Learning: In `let-sunny/canicode`, the opportunity rows in `src/core/report-html/render.ts` (`renderOpportunities`) are intentionally rendered as `<div class="rpt-opps-item">` elements rather than `<button>` elements. Click interactions are handled via event delegation on the container. The maintainer has decided not to convert them to buttons (to avoid CSS resets), so do NOT flag these divs as inaccessible or suggest converting them to `<button>` elements.
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 : `no-auto-layout` is the single highest-impact rule with score -10 — empirically validated via ablation experiments
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: 154
File: src/core/rules/token/index.ts:36-135
Timestamp: 2026-03-29T00:24:15.976Z
Learning: In let-sunny/canicode, `RuleCheckFn` in `src/core/rules/token/index.ts` (and all rule files) returns `RuleViolation | null` — only a single violation per node per rule is supported by the engine contract. The `rawValueCheck` returning on the first matched subtype is intentional. Multi-violation support per node requires engine contract changes, tracked in issue `#155`.
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, when a value is typed via a mapped type over a finite string-union key (e.g., `scores.byCategory: Record<Category, CategoryScoreResult>` where `Category` is a string union), TypeScript `noUncheckedIndexedAccess` does not introduce `| undefined` the way it does for index signatures. Therefore, do not flag `scores.byCategory[cat]` as potentially `undefined` when `cat: Category`.
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
violation 메시지를
message/suggestion/guide로 구조화하고, 리포트 UI를 카테고리 탭 + 룰 그룹 구조로 전면 재설계.Before
RuleViolation.message: "뭐가 문제 — 어떻게 고치라" 한 문자열After
RuleViolation:message(뭐가 문제) +suggestion(어떻게 고치라) +guide?(면제 조건)Changes
Data layer
RuleViolation에suggestion: string,guide?: string추가rule-messages.ts: 모든 메시지 함수가ViolationMsg객체 반환...msg()스프레드로 업데이트non-semantic-namesuggestion: 노드 타입별 맥락에 맞는 예시 (INSTANCE → "CloseButton", VECTOR → "ArrowIcon" 등)Report UI
overflow-x: auto스크롤, 탭 클릭 시 스크롤 없음<details>accordion, blocking/risk만 기본 open, Why/Impact/Fix 항상 보임Other
Stats
Test plan
pnpm build/pnpm lint/pnpm test:run통과Closes #155
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Style