Merge pull request #94 from watany-dev/claude/improve-review-readability-2vmY6

watany-dev · web-flow · commit 722e49445496 · 2026-04-09T21:28:05.000+09:00
diff --git a/docs/design-review-readability.md b/docs/design-review-readability.md
@@ -0,0 +1,291 @@
+# レビュー画面の可読性改善設計書
+
+> **ステータス**: 設計完了（2026-04-09）
+>
+> PR詳細画面のdiff表示に行番号ガターと変更行ジャンプ（`]c`/`[c`）を追加し、コードレビュー体験を改善する。
+
+## 概要
+
+PR詳細画面（`PullRequestDetail`）のdiff表示に2つの機能を追加し、コードレビュー体験を改善する：
+1. 行番号ガターの表示
+2. 変更行への高速ジャンプナビゲーション（`]c`/`[c`）
+
+## Context
+
+ユーザーがレビュー画面を使用中に「見づらい」と感じている。具体的には：
+1. **diffに行番号がない** — コードの位置が分からず、レビューコメントで行を参照できない
+2. **変更行にすぐ飛べない** — `j`/`k`で1行ずつ移動するか、`n`/`N`でファイル間ジャンプしかできず、変更箇所（add/delete行）に素早く移動する手段がない
+
+本設計は v0.3.0（UX強化: diff可読性・ナビゲーション向上）ロードマップに合致する。
+
+## ファイル構成
+
+| ファイル | 変更種別 | Iteration |
+|---------|---------|-----------|
+| `src/components/DiffLine.tsx` | 変更: 行番号ガター追加 | 1 |
+| `src/components/PullRequestDetail.tsx` | 変更: `]c`/`[c` キーバインド、`pendingBracket` state追加 | 2 |
+| `src/components/Help.tsx` | 変更: Navigation セクションに `]c`/`[c` 追加 | 2 |
+| `src/components/PullRequestDetail.test.tsx` | 変更: テストケース追加 | 1, 2 |
+
+## コンポーネント設計
+
+### Iteration 1: diff行番号の表示
+
+**Tidy First?**: 不要。`DiffLine.tsx` はシンプルな switch-case で読みやすく、構造変更なしに機能を追加できる。
+
+**対象ファイル:**
+- `src/components/DiffLine.tsx` — 行番号ガターのレンダリング追加
+- `src/components/PullRequestDetail.test.tsx` — テスト追加
+
+**設計:**
+
+`renderDiffLine` に行番号ガターを追加する。`DisplayLine` には既に `beforeLineNumber` と `afterLineNumber` が計算済み（`formatDiff.ts:55-56,77,94`）なので、表示するだけ。
+
+表示形式（ガター幅: 12文字 = 4 + 1 + 4 + 3）:
+```
+   3    4 │  context line text
+   5      │ -deleted line text
+        6 │ +added line text
+```
+
+- before/after それぞれ4文字幅（`padStart(4)`）で右寄せ、スペース1文字区切り、`│ ` セパレータ付き
+- `<Text dimColor>` で表示し、diff本文の色（green/red/white）と競合しないようにする
+- `add`, `delete`, `context` 行のみガター表示
+- `header`, `separator`, `truncation`, `truncate-context`, `comment-header`, `comment`, `inline-comment`, `inline-reply`, `comment-reply`, `fold-indicator` はガター不要（行番号プロパティが存在しないため）
+- ターミナル幅80カラムの場合: カーソル2文字 + ガター12文字 = 14文字消費、残り66文字がdiff本文（十分）
+- 行番号が9999を超える場合: `padStart(4)` では左にはみ出すが、レイアウトが壊れるわけではなく数字が左に1文字伸びるだけ。10,000行超のファイルは稀であり、対応不要
+
+**実装詳細:**
+
+`DiffLine.tsx` に `formatGutter(line: DisplayLine): string` ヘルパーを追加:
+```typescript
+function formatGutter(line: DisplayLine): string {
+  const before = line.beforeLineNumber ? String(line.beforeLineNumber).padStart(4) : "    ";
+  const after = line.afterLineNumber ? String(line.afterLineNumber).padStart(4) : "    ";
+  return `${before} ${after} │ `;
+}
+```
+
+`add`, `delete`, `context` の各caseでガター部分を `<Text dimColor>` で先頭に追加:
+```tsx
+case "context":
+  return (
+    <Text bold={bold}>
+      <Text dimColor>{formatGutter(line)}</Text>
+      {line.text}
+    </Text>
+  );
+case "add":
+  return (
+    <Text color="green" bold={bold}>
+      <Text dimColor>{formatGutter(line)}</Text>
+      {line.text}
+    </Text>
+  );
+case "delete":
+  return (
+    <Text color="red" bold={bold}>
+      <Text dimColor>{formatGutter(line)}</Text>
+      {line.text}
+    </Text>
+  );
+```
+
+**データフロー:**
+```
+formatDiff.ts: computeSimpleDiff()
+  → DisplayLine { beforeLineNumber: 3, afterLineNumber: 4, type: "context", text: " line1" }
+    → displayLines.ts: buildDisplayLines() でそのまま伝搬
+      → PullRequestDetail.tsx: renderDiffLine(line, isCursor) を呼び出し
+        → DiffLine.tsx: formatGutter(line) で "   3    4 │ " を生成、<Text dimColor> で表示
+```
+
+---
+
+### Iteration 2: 変更行ジャンプ機能の追加 (`]c`/`[c`)
+
+**Tidy First?**: 不要。`useInput` ハンドラは既存のパターン（キー検出 → 状態更新 → return）で一貫しており、新キーバインドを同パターンで追加可能。
+
+**対象ファイル:**
+- `src/components/PullRequestDetail.tsx` — `]c`/`[c` キーバインド追加
+- `src/components/PullRequestDetail.test.tsx` — テスト追加
+- `src/components/Help.tsx` — ヘルプ画面にキー説明追加
+
+**設計:**
+
+Vim diff風の `]c` (次の変更行) / `[c` (前の変更行) キーバインドを追加する。
+
+変更行 = `line.type === "add" || line.type === "delete"` の行
+
+**2キーシーケンスの設計判断:**
+
+`design-review-keybindings.md:349` で「2ストローク操作はタイムアウトベースのキーシーケンス管理が必要」と記載されている。しかしこれは `gg` のように**1キー目が既存キーバインドと衝突する場合**（`g` はリアクション操作に使用中）の話。`[`/`]` は現在どのキーバインドにも使用されていないため、押下時点で即座に `pendingBracket` state を設定する state ベースアプローチが使える。タイムアウト管理は不要。
+
+**state遷移図:**
+```
+[null] ──("]"キー)──→ ["]"] ──("c"キー)──→ [null] + 次の変更行へジャンプ
+  ↑                     │
+  └──(他のキー)─────────┘ (リセット + そのキーを通常処理)
+
+[null] ──("["キー)──→ ["["] ──("c"キー)──→ [null] + 前の変更行へジャンプ
+  ↑                     │
+  └──(他のキー)─────────┘ (リセット + そのキーを通常処理)
+```
+
+**実装詳細:**
+
+1. `pendingBracket` state を追加（`PullRequestDetail.tsx`、既存state群の近くに配置）:
+```typescript
+const [pendingBracket, setPendingBracket] = useState<"[" | "]" | null>(null);
+```
+
+2. `useInput` コールバック内で、**モーダルガード（line 405-416）の直後、既存キーハンドラの前**に配置:
+
+```typescript
+// === 2-key sequence: ]c / [c for change navigation ===
+if (pendingBracket && input === "c") {
+  const isNext = pendingBracket === "]";
+  setPendingBracket(null);
+  if (isNext) {
+    const idx = lines.findIndex(
+      (l, i) => i > cursorIndex && (l.type === "add" || l.type === "delete"),
+    );
+    if (idx !== -1) setCursorIndex(idx);
+  } else {
+    for (let i = cursorIndex - 1; i >= 0; i--) {
+      if (lines[i]!.type === "add" || lines[i]!.type === "delete") {
+        setCursorIndex(i);
+        break;
+      }
+    }
+  }
+  return;
+}
+if (pendingBracket) {
+  setPendingBracket(null); // 無効な2キー目: リセットしてフォールスルー
+}
+```
+
+3. 既存キーハンドラの末尾付近（`G` ハンドラ等の後）に `[`/`]` キャプチャを追加:
+```typescript
+if (input === "]") {
+  setPendingBracket("]");
+  return;
+}
+if (input === "[") {
+  setPendingBracket("[");
+  return;
+}
+```
+
+## エッジケース・異常系
+
+| ケース | 挙動 | 根拠 |
+|--------|------|------|
+| カーソルが既に変更行上 | 次の/前の**別の**変更行にジャンプ | `i > cursorIndex` / `i < cursorIndex` で現在行を除外 |
+| 前方/後方に変更行がない | カーソル位置変更なし | `findIndex` が `-1` を返す / for ループが `break` せず終了。ラップアラウンドしない（Vimの `]c` と同じ） |
+| diffが空（ファイルロード中） | カーソル位置変更なし | `lines` にadd/delete行が存在しない |
+| モーダル表示中に `[` を押下 | `pendingBracket` は設定されない | モーダルガード（line 405-416）で早期リターンするため |
+| `[` 押下直後に `j` を押す | pendingBracket リセット、`j` は通常のカーソル移動 | `pendingBracket` チェックで null 化後、return せずフォールスルー |
+| `[` 押下直後に `q` を押す | pendingBracket リセット、`q` で画面遷移 | 同上 |
+| All changes / コミットビュー | 両方で動作する | diff行のtype判定は共通。`viewIndex` によるガードなし |
+| 連続して `]c`/`[c` を押す | 変更行を順にたどれる | 毎回 `cursorIndex` から検索するため |
+
+**キーバインドの整合性:**
+
+`c` は既にコメント投稿に使用中（line 519-522）。衝突しない理由：
+- `[` 押下 → `pendingBracket` が `"["` にセット、即 return
+- 次の入力 `c` → `pendingBracket` チェック（モーダルガード直後に配置）が先に発火し、変更行ジャンプ実行
+- `c` 単体押下（`pendingBracket` が null）→ pendingBracket チェック条件 `if (pendingBracket && input === "c")` が false → 従来通り line 519 のコメント投稿ハンドラに到達
+
+**フッター更新** (`PullRequestDetail.tsx` line 905付近):
+`n/N file` の直後に `]c/[c change` を追加。
+
+**Help.tsx更新:**
+Navigation セクション内（line 50-51 の `n`/`N` の直後）に追加:
+```tsx
+<KeyRow keyName="]c" description="Next change (add/delete line)" />
+<KeyRow keyName="[c" description="Previous change (add/delete line)" />
+```
+
+## テスト戦略
+
+### Iteration 1: 行番号テスト
+
+TDDサイクル:
+1. **Red**: `PullRequestDetail.test.tsx` にテスト追加 — diff行に `│` セパレータが含まれることを検証
+2. **Green**: `DiffLine.tsx` に `formatGutter` + 各caseの変更を実装
+3. **Refactor**: 必要に応じて整理
+
+**既存テストへの影響:**
+- `toContain("line1")` （line 251）: ガターが前置されても `line1` は含まれるためパス
+- `toMatch(/> .*new line 1/)` （line 1938）: `.*` がガター文字にマッチするためパス
+- `toContain("src/auth.ts")` （line 225）: header行にはガターが付かないためパス
+
+### Iteration 2: 変更行ジャンプテスト
+
+TDDサイクル:
+1. **Red**: テストを追加
+   - `]c` で次のadd/delete行にジャンプすることを確認
+   - `[c` で前のadd/delete行にジャンプすることを確認
+   - 変更行がない場合はカーソル位置が変わらないことを確認
+   - `[` + `j` でpendingBracketがリセットされ、`j` が通常処理されることを確認
+2. **Green**: `PullRequestDetail.tsx` に pendingBracket state + useInput ハンドラ実装、`Help.tsx` にキー説明追加
+3. **Refactor**: 必要に応じて整理
+
+## 実装計画
+
+各イテレーション完了時に `bun run ci` を実行して全チェックをパス:
+1. `lint` — oxlint
+2. `format:check` — Biome formatting
+3. `check` — TypeScript type check
+4. `dead-code` — knip unused export detection
+5. `audit` — dependency vulnerability check
+6. `test:coverage` — Vitest with 95% coverage threshold
+7. `build` — production build
+
+## 対象外（今回のスコープ外）
+
+- ファイル間の余白改善（セパレータ幅の動的化など）
+- コメントの視覚的階層改善
+- フッター情報量の削減（`design-review-keybindings.md` P2 として別途対応）
+- 表示行数のターミナル高さ連動
+- 要件定義書（`docs/requirements.md`）のキーバインド表への `]c`/`[c` 追記は実装完了後に `/update-docs` で対応
+
+---
+
+## update-plan 検証結果
+
+### 設計書品質評価
+
+| # | カテゴリ | 点数 | コメント |
+|---|---------|------|---------|
+| 1 | 基本構造 | 9/10 | 概要・コンポーネント設計・実装詳細・テスト・検証方法・ファイル構成が揃っている |
+| 2 | 目的・スコープの明確性 | 9/10 | 何を実装し何を実装しないかが明確。対象外も列挙済み |
+| 3 | コンポーネント設計の完全性 | 9/10 | DiffLine.tsx の全case、pendingBracket の型定義、state遷移図、データフローを記載 |
+| 4 | AWS SDK連携設計 | N/A | 本変更にAWS SDK連携なし |
+| 5 | 内部アーキテクチャ | 9/10 | ファイル構成表、データフロー図、state遷移図、`useInput` 内の配置順序を記載 |
+| 6 | エッジケース・異常系 | 9/10 | 8ケースの網羅的な表、行番号オーバーフロー、モーダル中の挙動を記載 |
+| 7 | テスト戦略 | 9/10 | TDDサイクル（Red→Green→Refactor）で構成。具体的なテストケース、既存テスト影響分析を記載 |
+| 8 | セキュリティ考慮 | N/A | 本変更にセキュリティ影響なし |
+| 9 | 技術選定の根拠 | 9/10 | stateベース vs タイムアウトの比較、`design-review-keybindings.md` との整合性説明、ユーザー選択の経緯 |
+| 10 | 実装計画 | 9/10 | 2イテレーション、Tidy First判定、TDDサイクル、対象ファイル/行番号が明確 |
+| | **合計** | **72/80 (90%)** | N/A 2項目を除外した8項目での評価 |
+
+**総合判定**: 🟢 実装着手可能（90%）
+
+### 整合性チェック
+
+| チェック項目 | 結果 | 詳細 |
+|-------------|------|------|
+| 設計書 ↔ ソースコード | ✅ | `DisplayLine` に `beforeLineNumber`/`afterLineNumber` が既存（`formatDiff.ts:1-24`）。`DiffLine.tsx` の switch-case 構造と一致 |
+| ロードマップ ↔ 設計書 ↔ 要件定義 | ✅ | v0.3.0 UX強化（diff可読性・ナビゲーション向上）に合致 |
+| CLAUDE.md との整合性 | ✅ | TDDサイクル、Tidy First判定、カバレッジ95%を担保 |
+| ソースコード規約 | ✅ | 既存のキーハンドラパターン、`Help.tsx` の `SectionHeader`+`KeyRow` パターンに従う |
+| design-review-keybindings.md | ✅ | 2ストローク操作にタイムアウト不要の根拠を記載 |
+
+### 修正事項
+
+- **P0**: なし
+- **P1**: なし
+- **P2**: フッターがさらに長くなる（`]c/[c change` 追加）が、`design-review-keybindings.md` P2（フッター整理）で別途対応
