docs: sync sparkle anti-pattern guidance - #192
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro (Legacy) Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughSparkle Design のドキュメントとコンポーネント注釈を更新し、CLI 呼び出しを Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.claude/skills/add-sparkle-component/references/sparkle-design-features.md (1)
499-500: 軽微な重複があります。
CardControlの既定レイアウト（flex items-center gap-2）についてのガイダンスが Lines 499-500 と Lines 568-569 の2箇所で重複しています。読みやすさの観点から、一方を削除するか、片方を参照リンクにすることを検討してください。Also applies to: 568-569
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/add-sparkle-component/references/sparkle-design-features.md around lines 499 - 500, Duplicate guidance about CardControl's default layout ("flex items-center gap-2") appears twice (around the existing CardControl documentation). Remove one of the repeated sentences or replace the second occurrence with a short reference/link to the original paragraph so the doc only describes CardControl's default layout once; update the content that mentions CardControl to point to the canonical section instead of repeating the same text..claude/skills/add-sparkle-component/references/css-structure.md (1)
324-334: npm/yarn/bun のコマンド例が pnpm と不整合です。pnpm は
generateサブコマンドを使用するよう更新されていますが、他のパッケージマネージャーの例は更新されていません。一貫性のため、すべてのパッケージマネージャーで同じコマンド形式に揃えることを推奨します。♻️ 提案する修正
# pnpm pnpm dlx sparkle-design-cli generate # npm -npm exec sparkle-design-cli +npm exec sparkle-design-cli generate # yarn -yarn dlx sparkle-design-cli +yarn dlx sparkle-design-cli generate # bun -bunx sparkle-design-cli +bunx sparkle-design-cli generate🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/add-sparkle-component/references/css-structure.md around lines 324 - 334, The examples are inconsistent: only the pnpm example uses the generate subcommand ("pnpm dlx sparkle-design-cli generate") while npm/yarn/bun lines lack it; update the npm, yarn and bun examples to include the same "generate" subcommand and consistent invocation form so all four examples match (refer to the existing example strings "pnpm dlx sparkle-design-cli generate", "npm exec sparkle-design-cli", "yarn dlx sparkle-design-cli", "bunx sparkle-design-cli" and change the latter three to include "generate" so they all use e.g. "npm exec sparkle-design-cli generate", "yarn dlx sparkle-design-cli generate", "bunx sparkle-design-cli generate")..claude/skills/add-sparkle-component/SKILL.md (1)
333-333:setup実行コマンドの前提を1行補足してください。現在の記述だと「
sparkle-design-cliがPATHにある前提」に見えるため、pnpm dlx sparkle-design-cli setup ...も併記しておくと実行失敗を減らせます。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/add-sparkle-component/SKILL.md at line 333, 補足として「sparkle-design-cli がグローバルPATHにない場合の代替実行方法」を1行追記してください：現在の説明で使われているコマンド参照（`sparkle-design-cli setup --assistant <claude|codex|cursor|generic>`）の直前か直後に、`pnpm dlx sparkle-design-cli setup --assistant ...` のように pnpm dlx を使った実行例を併記し、「PATH にない場合はこちらを使う」と短く明記してください（参照箇所の識別子は SKILL.md の setup コマンド説明行）。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/add-sparkle-component/SKILL.md:
- Line 335: Update the documentation sentence that fixes the hard-coded "src"
target: replace the example command `sparkle-design-cli check src --format json`
with a variable/placeholder or auto-detection form such as `sparkle-design-cli
check <detected-target> --format json` (or `sparkle-design-cli check --format
json` with mention that `--target` is auto-detected) so it aligns with the
earlier explanation about omitting `--target`; ensure references to the npm
scripts `lint:sparkle` and `lint:sparkle:json` remain consistent.
---
Nitpick comments:
In @.claude/skills/add-sparkle-component/references/css-structure.md:
- Around line 324-334: The examples are inconsistent: only the pnpm example uses
the generate subcommand ("pnpm dlx sparkle-design-cli generate") while
npm/yarn/bun lines lack it; update the npm, yarn and bun examples to include the
same "generate" subcommand and consistent invocation form so all four examples
match (refer to the existing example strings "pnpm dlx sparkle-design-cli
generate", "npm exec sparkle-design-cli", "yarn dlx sparkle-design-cli", "bunx
sparkle-design-cli" and change the latter three to include "generate" so they
all use e.g. "npm exec sparkle-design-cli generate", "yarn dlx
sparkle-design-cli generate", "bunx sparkle-design-cli generate").
In @.claude/skills/add-sparkle-component/references/sparkle-design-features.md:
- Around line 499-500: Duplicate guidance about CardControl's default layout
("flex items-center gap-2") appears twice (around the existing CardControl
documentation). Remove one of the repeated sentences or replace the second
occurrence with a short reference/link to the original paragraph so the doc only
describes CardControl's default layout once; update the content that mentions
CardControl to point to the canonical section instead of repeating the same
text.
In @.claude/skills/add-sparkle-component/SKILL.md:
- Line 333: 補足として「sparkle-design-cli
がグローバルPATHにない場合の代替実行方法」を1行追記してください：現在の説明で使われているコマンド参照（`sparkle-design-cli setup
--assistant <claude|codex|cursor|generic>`）の直前か直後に、`pnpm dlx sparkle-design-cli
setup --assistant ...` のように pnpm dlx を使った実行例を併記し、「PATH
にない場合はこちらを使う」と短く明記してください（参照箇所の識別子は SKILL.md の setup コマンド説明行）。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro (Legacy)
Run ID: a515d8d4-4a17-4a45-83ac-f9bad58f65e9
📒 Files selected for processing (9)
.claude/skills/add-sparkle-component/SKILL.md.claude/skills/add-sparkle-component/references/css-structure.md.claude/skills/add-sparkle-component/references/sparkle-design-features.mdsrc/components/ui/button/index.tsxsrc/components/ui/card/index.test.tsxsrc/components/ui/card/index.tsxsrc/components/ui/icon-button/index.tsxsrc/components/ui/icon/index.tsxsrc/components/ui/input/index.tsx
touyou
left a comment
touyou
left a comment
There was a problem hiding this comment.
レビューコメント
PR 全体として、CLI コマンドの最新化・アンチパターンドキュメントの同期・CardControl への既定レイアウト追加、いずれも方針が明確で良い変更だと思います。以下、気になった点をいくつか共有します。
1. css-structure.md — npm / yarn / bun のコマンドが generate サブコマンド未反映（重要度: 高）
pnpm dlx sparkle-design-cli generate に更新されていますが、同じコードブロック内の npm / yarn / bun の例は旧コマンドのままです。
ファイル: .claude/skills/add-sparkle-component/references/css-structure.md L327 付近
# pnpm
pnpm dlx sparkle-design-cli generate # ← 更新済み
# npm
npm exec sparkle-design-cli # ← generate なし
# yarn
yarn dlx sparkle-design-cli # ← generate なし
# bun
bunx sparkle-design-cli # ← generate なし他のパッケージマネージャーでも generate サブコマンドが必要であれば、統一したほうが良さそうです。意図的に省略している場合は無視してください。
2. sparkle-design-features.md — CardControl の説明が重複
CardControl は既定で flex items-center gap-2 を持つ。 という同一の文が L499 と L568 の 2 箇所に存在します。「Badge と Tag を用途で使い分ける」セクション直後（L499）と「CardHeader: CardControl を使う」セクション直後（L568）です。
どちらか一方にまとめるか、片方からもう片方を参照する形にすると読みやすくなると思います。
3. card/index.tsx — CardDescription の JSDoc コメント記法の微小な不整合
L212 の閉じ ``` の直後に空行を挟んで * @param が続いていますが、閉じ */ との間に空の * 行がありません（L211-213 付近）。CardControl の JSDoc（L249-251 付近）も同様のパターンです。動作には影響しませんが、他コンポーネント（CardHeader 等）のブロックと書式を揃えるなら統一すると良いかもしれません。
4. テスト — className の実装詳細への依存について（軽微な指摘）
card/index.test.tsx で control.className に flex, items-center, gap-2 が含まれることを直接アサートしています。Tailwind のクラス名は実装詳細であり、将来リファクタリングした際にテストが壊れやすい面もあります。ただし、今回は「CardControl が既定レイアウトを持つ」ことを保証する意図のテストなので、現時点では妥当だと思います。将来的にはビジュアルリグレッションテストなどで補完できると良さそうです。
まとめ
主要な機能変更（CardControl への flex items-center gap-2 追加）はテスト付きで安心感があります。JSDoc・ドキュメントの同期も丁寧に行われています。上記 1 のコマンド不整合だけ確認いただければ、マージして問題ないと思います。
1 participant
概要
sparkle-design-internal#91 / #92 に合わせて、公開側の shared skill / reference docs / Card 実装を更新します。変更内容
setupはnpx --yes/pnpm dlx併記generateサブコマンド付きに統一sparkle-design-cliの source of truth に同期CardControlに既定レイアウトを追加CardControl説明の重複を整理check --format json/setupベースに更新動作確認
pnpm exec vitest run src/components/ui/card/index.test.tsxセルフレビューリスト
関連 Issue