river-review-code

Code Quality Review（一般コード品質レビュー）

コードの可読性、保守性、型安全性を検証する。他の専門エージェントに該当しない場合のデフォルトフォールバック先。

When to Use / いつ使うか

• 一般的なコード変更のレビュー時
• 他の専門エージェント（architecture, security, performance, testing）に該当しない場合
• コード品質の総合的なチェックが必要な場合

Routing / ルーティング

キーワード	スキルID	説明
型, TypeScript, strict	typescript-strict	TypeScript strict モード準拠
null, undefined, optional	typescript-nullcheck	null 安全性チェック
非同期, await, Promise	async-correctness	非同期処理の正しさ検証
型駆動, 設計	type-driven-design	型駆動設計
ログ, 監視	logging-observability	ロギング・可観測性
自動化, 境界	review-automation-boundary	レビュー自動化の境界
コメント, トリアージ	review-comment-triage	レビューコメント分類
幻覚的参照, 実在確認	hallucinated-reference	新規参照の実在確認
簡素化, 整理, simplify	SIMPLIFY 観点（本 skill 内）	品質クリーンアップ4観点
破壊的操作, undo, 回復支援	UX-SAFEGUARD 観点（本 skill 内）	操作の安全装置2観点

UI/コンポーネント系のルーティング（a11y, デザインシステム, Next.js App Router 境界等）は river-review-frontend に一元化済み（#1462）。本ルーターからは移設し、二重発火を避けている。

デフォルト動作

• キーワード指定なし → 以下のヒューリスティクスで判定:
  • .ts/.tsxファイル → TypeScript strict + nullチェック
  • コンポーネントファイル → river-review-frontend も参照（a11y・デザインシステム観点は frontend 側が担当）
  • 設定ファイル → 型駆動設計チェック

Checklist / チェックリスト

一般コードレビューでは以下を確認する:

可読性

• 関数・変数の命名が意図を表現しているか
• 意図を伝えない広すぎる名前（data / info / manager / handler / util / current）、共有されていない略語、同一概念の別名（または別概念の同名）がないか
• 関数の責務が単一か
• ネストが深すぎないか（3段以内。深い場合は guard clause で平坦化を提案）
• マジックナンバー・マジックストリングがないか

保守性

• DRY原則にしたがっているか（ただし過度な抽象化を避ける）
• 変更の影響範囲が限定的か
• 依存方向が正しいか
• カプセル化リークがないか: オブジェクト内部へ深く手を伸ばすコード（a.b.c.type === 'x'）、値オブジェクトから primitive を取り出して外部で分岐、getter による内部状態の露出。Tell-Don't-Ask（例: user.subscription.plan.type === 'premium' より user.isPremium()）を推奨する（Law of Demeter）

型安全性

• anyの使用が最小限か
• 型ガードが適切か
• null/undefinedの扱いが安全か
• 型検査対象外の JSDoc 型変更: scripts/ などで unknown を any へ緩める提案は直接行わない。境界判断が必要なときだけ owner skill existing-pattern-conformance を skill ID で解決し、その False-positive guards / fixtures に従う。owner を解決できない場合は推測せず、この境界判断をスキップする。

エラーハンドリング

• エラーを握り潰していないか
• エラーメッセージが十分な情報を含むか
• リカバリー可能なエラーと不可能なエラーの区別
• 防衛的フォールバックの分界: config / registry などの内部不変条件へ fallback を足す提案はしない。外部 IO / 環境境界では defensive handling を確認する。詳細判断が必要なときだけ owner skill nullability-contract を skill ID で解決し、その False-positive guards / fixtures に従う。owner を解決できない場合は推測せず、この境界判断をスキップする。

Execution Flow / 実行フロー

1. ファイル種別の判定
   ├─ .ts/.tsxファイル → TypeScript strict + nullチェックを選択
   ├─ コンポーネントファイル → river-review-frontend も参照（a11y・デザインシステム観点）
   ├─ 設定ファイル → 型駆動設計チェックを選択
   └─ キーワード指定あり → 該当スキルを直接選択
      （SIMPLIFY / UX-SAFEGUARD 観点のキーワード該当時は本 skill 内で実行。キーワードは ROUTING.md を参照）

2. スキルの実行
   ├─ typescript-strict: strictモード準拠
   ├─ typescript-nullcheck: null安全性
   ├─ async-correctness: 非同期処理の正しさ
   ├─ type-driven-design: 型駆動設計
   ├─ logging-observability: ロギング・可観測性
   ├─ review-automation-boundary: レビュー自動化の境界
   └─ hallucinated-reference: 新規参照の実在確認

3. 統合
   ├─ 重複する指摘の除去
   └─ Checklistに基づく一般品質チェックの補完

Multi-perspective Execution / 多観点実行（旧 agent-code-review から統合）

複数観点を横断するレビューでは、以下の順で差分を走査し findings を統合する。

順序	観点	実行ルール
1	セキュリティ	Critical finding 検出時: 以降の観点も実行するが、Critical を先頭に出力
2	パフォーマンス	ホットパス外の変更のみの場合はスキップ可
3	品質・設計	常に実行
4	テスト網羅性	テストファイルが差分に含まれない場合も、対象コードのテスト有無を確認

観点間の重要度比較: 異なる観点の findings が同一箇所を指す場合、severity が異なれば高い方を採用（もう一方は補足として併記）、同じなら security > performance > quality > testing の順で先に記載する。

出力件数の制約: 1 PR あたり最大 15 件（超過分は severity 降順で切り捨て、切り捨て件数を末尾に記載）。同一ファイルへの同一観点の指摘は最大 3 件にグルーピングする。

判定の手がかり:

• catch ブロック内の空文、// TODO → security / quality
• O(n*m) パターン、ループ内の DB / API コール → performance
• any 型、型アサーション（as）、未使用 import → quality
• 新規 export 関数にテストファイル内の対応する describe / test がない → testing

Output Format / 出力形式

<file>:<line>: <message>

• Finding: 何が問題か（1文）
• Impact: 何が困るか（短く）
• Fix: 次の一手（最小の修正案）

他スキルとの関係

スキル	関係	棲み分け
river-review-architecture	補完	code は「ミクロ品質」、architecture は「マクロ設計」
river-review-testing	補完	code は「プロダクションコード」、testing は「テストコード」
river-review-performance	補完	code は「可読性」、performance は「実行効率」
river-review-frontend	補完	code は「一般コード品質」、frontend は「UI 固有の懸念」。UI 系ルートは frontend へ移設済み（#1462）

References

• ROUTING.md: 詳細なルーティングルール
• SIMPLIFY.md: 品質クリーンアップ4観点の実行手順と委譲表
• UX-SAFEGUARD.md: 操作の安全装置2観点（破壊的操作の確認・取り消し / 入力エラーの回復支援）の実行手順と委譲表