Skip to content

Commit a647262

Browse files
authored
Merge pull request #95 from watany-dev/claude/review-design-document-4jX8s
2 parents 722e494 + cd2898b commit a647262

8 files changed

Lines changed: 304 additions & 29 deletions

File tree

README.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ review-codecommit lets you browse AWS CodeCommit repositories, view pull request
1414
- Browse pull requests with status filter (Open / Closed / Merged, `f` key cycle)
1515
- Search pull requests by title or author (`/` key)
1616
- Paginate through PR lists (`n`/`p` keys)
17-
- View PR details with color-coded unified diffs (green for additions, red for deletions)
17+
- View PR details with color-coded unified diffs (green for additions, red for deletions) with line number gutter
1818
- Read PR comments inline
1919
- Post comments on pull requests (`c` key in PR detail view)
2020
- Post inline comments on specific diff lines (`C` key at cursor position)
@@ -32,6 +32,7 @@ review-codecommit lets you browse AWS CodeCommit repositories, view pull request
3232
- Close PRs without merging (`x` key)
3333
- Commit-level review with Tab/Shift+Tab navigation between "All changes" and individual commits
3434
- Cursor-based diff navigation with `>` marker
35+
- Jump to next/previous change line with `]c`/`[c` (Vim diff-style)
3536
- View PR activity timeline (created, approved, merged, status changes, etc.) (`A` key)
3637
- Shell completion for bash, zsh, and fish (`--completions` option)
3738
- Vim-style keybindings (`j`/`k` navigation)
@@ -107,6 +108,9 @@ Completions support:
107108
| `Ctrl+d` | Half page down | PR Detail |
108109
| `Ctrl+u` | Half page up | PR Detail |
109110
| `G` | Jump to end | PR Detail |
111+
| `n` / `N` | Next / previous file | PR Detail |
112+
| `]c` | Jump to next change (add/delete line) | PR Detail |
113+
| `[c` | Jump to previous change (add/delete line) | PR Detail |
110114
| `Enter` | Select / confirm / submit comment | List screens / Comment input |
111115
| `q` / `Esc` | Go back / cancel | All / Comment input / Confirm prompt |
112116
| `c` | Post comment | PR Detail |

docs/design-review-readability.md

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# レビュー画面の可読性改善設計書
22

3-
> **ステータス**: 設計完了(2026-04-09
3+
> **✅ 実装完了** (2026-04-09)
44
>
55
> PR詳細画面のdiff表示に行番号ガターと変更行ジャンプ(`]c`/`[c`)を追加し、コードレビュー体験を改善する。
66
@@ -60,10 +60,12 @@ PR詳細画面(`PullRequestDetail`)のdiff表示に2つの機能を追加し
6060
`DiffLine.tsx``formatGutter(line: DisplayLine): string` ヘルパーを追加:
6161
```typescript
6262
function formatGutter(line: DisplayLine): string {
63-
const before = line.beforeLineNumber ? String(line.beforeLineNumber).padStart(4) : " ";
64-
const after = line.afterLineNumber ? String(line.afterLineNumber).padStart(4) : " ";
63+
const before = line.beforeLineNumber !== undefined ? String(line.beforeLineNumber).padStart(4) : " ";
64+
const after = line.afterLineNumber !== undefined ? String(line.afterLineNumber).padStart(4) : " ";
6565
return `${before} ${after} │ `;
6666
}
67+
// Note: computeSimpleDiff() は bi+1/ai+1 で行番号を生成するため 0 は来ないが、
68+
// DisplayLine の型定義(number | undefined)に対して防御的に !== undefined を使用する。
6769
```
6870

6971
`add`, `delete`, `context` の各caseでガター部分を `<Text dimColor>` で先頭に追加:
@@ -121,6 +123,8 @@ Vim diff風の `]c` (次の変更行) / `[c` (前の変更行) キーバイン
121123

122124
`design-review-keybindings.md:349` で「2ストローク操作はタイムアウトベースのキーシーケンス管理が必要」と記載されている。しかしこれは `gg` のように**1キー目が既存キーバインドと衝突する場合**`g` はリアクション操作に使用中)の話。`[`/`]` は現在どのキーバインドにも使用されていないため、押下時点で即座に `pendingBracket` state を設定する state ベースアプローチが使える。タイムアウト管理は不要。
123125

126+
**タイムアウトなしのUX影響**: `]` を押した後に長時間放置してから `c` を押すと、コメント投稿ではなく変更行ジャンプが発火する。ただし、間に別のキー(`j`/`k` 等)を1つでも押せば `pendingBracket` はリセットされるため、誤発火は「`]`→(何もしない)→`c`」のパターンに限定される。実用上問題にならないと判断し、タイムアウトは導入しない。
127+
124128
**state遷移図:**
125129
```
126130
[null] ──("]"キー)──→ ["]"] ──("c"キー)──→ [null] + 次の変更行へジャンプ
@@ -190,6 +194,8 @@ if (input === "[") {
190194
| `[` 押下直後に `q` を押す | pendingBracket リセット、`q` で画面遷移 | 同上 |
191195
| All changes / コミットビュー | 両方で動作する | diff行のtype判定は共通。`viewIndex` によるガードなし |
192196
| 連続して `]c`/`[c` を押す | 変更行を順にたどれる | 毎回 `cursorIndex` から検索するため |
197+
| `n`/`N` とのラップアラウンド差異 | `]c`/`[c` はラップアラウンドしない | `n`/`N`(ファイルジャンプ)はラップアラウンドするが、`]c`/`[c` はVimの `]c` に準拠しラップしない。意図的な設計差異 |
198+
| コミットビューでの行番号ガター | 表示される | `DiffLine.tsx` はビューモード非依存の共通コンポーネントであり、コミットビューでも行番号ガターが表示される |
193199

194200
**キーバインドの整合性:**
195201

@@ -201,6 +207,12 @@ if (input === "[") {
201207
**フッター更新** (`PullRequestDetail.tsx` line 905付近):
202208
`n/N file` の直後に `]c/[c change` を追加。
203209

210+
**`pendingBracket` の視覚的フィードバック**: `]` または `[` 押下時、フッターに pending 状態を表示する。`pendingBracket` が非 null の場合、通常のフッターヒントの代わりに以下を表示:
211+
```
212+
]_ Press c for next change, other key to cancel
213+
```
214+
これにより、ユーザーが2キー目の入力を待っている状態であることを明示する。Vimのpending operator表示に倣った設計。
215+
204216
**Help.tsx更新:**
205217
Navigation セクション内(line 50-51 の `n`/`N` の直後)に追加:
206218
```tsx
@@ -251,24 +263,25 @@ TDDサイクル:
251263
- フッター情報量の削減(`design-review-keybindings.md` P2 として別途対応)
252264
- 表示行数のターミナル高さ連動
253265
- 要件定義書(`docs/requirements.md`)のキーバインド表への `]c`/`[c` 追記は実装完了後に `/update-docs` で対応
266+
- ロードマップ(`docs/roadmap.md`)の v0.3.0 セクションへの行番号ガター・`]c`/`[c` 追記は実装完了後に `/update-docs` で対応
254267

255268
---
256269

257270
## update-plan 検証結果
258271

259-
### 設計書品質評価
272+
### 設計書品質評価(レビュー反映後)
260273

261274
| # | カテゴリ | 点数 | コメント |
262275
|---|---------|------|---------|
263276
| 1 | 基本構造 | 9/10 | 概要・コンポーネント設計・実装詳細・テスト・検証方法・ファイル構成が揃っている |
264277
| 2 | 目的・スコープの明確性 | 9/10 | 何を実装し何を実装しないかが明確。対象外も列挙済み |
265-
| 3 | コンポーネント設計の完全性 | 9/10 | DiffLine.tsx の全case、pendingBracket の型定義state遷移図データフローを記載 |
278+
| 3 | コンポーネント設計の完全性 | 9/10 | DiffLine.tsx の全case、pendingBracket の型定義state遷移図・視覚的フィードバック・データフローを記載 |
266279
| 4 | AWS SDK連携設計 | N/A | 本変更にAWS SDK連携なし |
267280
| 5 | 内部アーキテクチャ | 9/10 | ファイル構成表、データフロー図、state遷移図、`useInput` 内の配置順序を記載 |
268-
| 6 | エッジケース・異常系 | 9/10 | 8ケースの網羅的な表、行番号オーバーフロー、モーダル中の挙動を記載 |
281+
| 6 | エッジケース・異常系 | 9/10 | 10ケースの網羅的な表(`n`/`N` とのラップアラウンド差異、コミットビューのガター表示を追記)、行番号オーバーフロー、モーダル中の挙動を記載 |
269282
| 7 | テスト戦略 | 9/10 | TDDサイクル(Red→Green→Refactor)で構成。具体的なテストケース、既存テスト影響分析を記載 |
270283
| 8 | セキュリティ考慮 | N/A | 本変更にセキュリティ影響なし |
271-
| 9 | 技術選定の根拠 | 9/10 | stateベース vs タイムアウトの比較、`design-review-keybindings.md` との整合性説明、ユーザー選択の経緯 |
284+
| 9 | 技術選定の根拠 | 9/10 | stateベース vs タイムアウトの比較、タイムアウトなしのUX影響分析、`design-review-keybindings.md` との整合性説明を記載 |
272285
| 10 | 実装計画 | 9/10 | 2イテレーション、Tidy First判定、TDDサイクル、対象ファイル/行番号が明確 |
273286
| | **合計** | **72/80 (90%)** | N/A 2項目を除外した8項目での評価 |
274287

@@ -278,14 +291,25 @@ TDDサイクル:
278291

279292
| チェック項目 | 結果 | 詳細 |
280293
|-------------|------|------|
281-
| 設計書 ↔ ソースコード || `DisplayLine``beforeLineNumber`/`afterLineNumber` が既存(`formatDiff.ts:1-24`)。`DiffLine.tsx` の switch-case 構造と一致 |
282-
| ロードマップ ↔ 設計書 ↔ 要件定義 || v0.3.0 UX強化(diff可読性・ナビゲーション向上)に合致 |
294+
| 設計書 ↔ ソースコード || `DisplayLine``beforeLineNumber`/`afterLineNumber` が既存(`formatDiff.ts:1-24`)。`DiffLine.tsx` の switch-case 構造と一致`formatGutter``!== undefined` で型安全にチェック |
295+
| ロードマップ ↔ 設計書 ↔ 要件定義 || v0.3.0 UX強化(diff可読性・ナビゲーション向上)に合致`roadmap.md``requirements.md` の更新は対象外セクションで言及済み |
283296
| CLAUDE.md との整合性 || TDDサイクル、Tidy First判定、カバレッジ95%を担保 |
284297
| ソースコード規約 || 既存のキーハンドラパターン、`Help.tsx``SectionHeader`+`KeyRow` パターンに従う |
285-
| design-review-keybindings.md || 2ストローク操作にタイムアウト不要の根拠を記載 |
298+
| design-review-keybindings.md || 2ストローク操作にタイムアウト不要の根拠・UX影響を記載 |
299+
300+
### レビュー反映事項
301+
302+
以下の指摘をレビューに基づき設計書に反映済み:
303+
304+
- **P1-1**: `pendingBracket` の視覚的フィードバック → フッターに pending 状態表示を追加する設計を追記
305+
- **P1-2**: `formatGutter` の truthiness チェック → `!== undefined` に変更し、`computeSimpleDiff` が1始まりである根拠をコメントで明記
306+
- **P2-3**: `n`/`N` とのラップアラウンド差異 → エッジケース表に追記
307+
- **P2-4**: タイムアウトなしのUX影響 → 2キーシーケンスの設計判断セクションに追記
308+
- **P2-5**: コミットビューの行番号ガター → エッジケース表に追記
309+
- **P2-6**: `roadmap.md` 更新 → 対象外セクションに追記
286310

287311
### 修正事項
288312

289313
- **P0**: なし
290-
- **P1**: なし
291-
- **P2**: フッターがさらに長くなる(`]c/[c change` 追加)が、`design-review-keybindings.md` P2(フッター整理)で別途対応
314+
- **P1**: なし(反映済み)
315+
- **P2**: フッターがさらに長くなる(`]c/[c change` + pending状態表示)が、`design-review-keybindings.md` P2(フッター整理)で別途対応

docs/requirements.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,10 @@ npx review-codecommit --completions <shell> # シェル補完スクリプト生
170170
| `Ctrl+d` | 半ページ下スクロール | PR詳細画面(コメント入力中・確認プロンプト中は無効) |
171171
| `Ctrl+u` | 半ページ上スクロール | PR詳細画面(コメント入力中・確認プロンプト中は無効) |
172172
| `G` | 最終行へジャンプ | PR詳細画面(コメント入力中・確認プロンプト中は無効) |
173+
| `n` | 次のファイルへジャンプ | PR詳細画面(コメント入力中・確認プロンプト中は無効) |
174+
| `N` | 前のファイルへジャンプ | PR詳細画面(コメント入力中・確認プロンプト中は無効) |
175+
| `]c` | 次の変更行(add/delete)へジャンプ | PR詳細画面(コメント入力中・確認プロンプト中は無効) |
176+
| `[c` | 前の変更行(add/delete)へジャンプ | PR詳細画面(コメント入力中・確認プロンプト中は無効) |
173177
| `Enter` | 選択・決定 / コメント送信 | リスト画面 / コメント入力 |
174178
| `q` / `Esc` | 前の画面に戻る / コメント入力キャンセル | 全画面 / コメント入力 |
175179
| `Ctrl+C` | 即座に終了 | 全画面 |
@@ -258,12 +262,12 @@ npx review-codecommit --completions <shell> # シェル補完スクリプト生
258262
│ src/auth.ts │
259263
│ │
260264
│ @@ -15,7 +15,7 @@ │
261-
const config = {
262-
│ > - timeout: 3000,
265+
15 15 │ const config = {
266+
│ > 16 │ - timeout: 3000,
263267
│ 💬 taro: この値はconfigから取る方が良さそう │
264268
│ └ watany: 次のPRで修正します │
265-
+ timeout: 10000,
266-
};
269+
16 │ + timeout: 10000,
270+
17 17 │ };
267271
│ │
268272
│──────────────────────────────────────────────│
269273
│ Comments (3): │
@@ -282,6 +286,7 @@ npx review-codecommit --completions <shell> # シェル補完スクリプト生
282286

283287
- **形式**: unified diff (`git diff` 同等)
284288
- **色付け**: 追加行=緑、削除行=赤、コンテキスト行=デフォルト
289+
- **行番号ガター**: 変更前/変更後の行番号を各4桁右寄せで表示(例: ` 3 4 │ `)。add/delete/context行のみ。dimColorで本文色と分離
285290
- **構造**: ファイル単位でセクション分け
286291

287292
## ページネーション

docs/roadmap.md

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,17 +104,21 @@ PR レビューの完全なワークフロー(閲覧→コメント→承認
104104

105105
#### 機能
106106

107-
| 機能 | 内容 |
108-
|------|------|
109-
| シンタックスハイライト | diff 内のコードをファイル拡張子に応じて色付け表示 |
110-
| ファイルツリー | PR 内の変更ファイルをツリー表示し、ファイル間をジャンプ(`t` キー) |
111-
| diff 統計 | ファイル別・合計の追加行/削除行数を表示 |
112-
| ファイルジャンプ | ファイルツリーで選択したファイルの diff 位置へ直接移動 |
107+
| 機能 | 内容 | 状態 |
108+
|------|------|------|
109+
| 行番号ガター | diff の add/delete/context 行に変更前/変更後の行番号を表示(` 3 4 │ ` 形式) ||
110+
| 変更行ジャンプ | `]c` で次の変更行、`[c` で前の変更行へジャンプ(Vim diff 風 2キーシーケンス) ||
111+
| シンタックスハイライト | diff 内のコードをファイル拡張子に応じて色付け表示 | |
112+
| ファイルツリー | PR 内の変更ファイルをツリー表示し、ファイル間をジャンプ(`t` キー) | |
113+
| diff 統計 | ファイル別・合計の追加行/削除行数を表示 | |
114+
| ファイルジャンプ | ファイルツリーで選択したファイルの diff 位置へ直接移動 | |
113115

114116
#### キーバインド追加
115117

116118
| キー | 動作 | 画面 |
117119
|------|------|------|
120+
| `]c` | 次の変更行(add/delete)へジャンプ | PR 詳細 |
121+
| `[c` | 前の変更行(add/delete)へジャンプ | PR 詳細 |
118122
| `t` | ファイルツリー表示 | PR 詳細 |
119123

120124
#### 設計上の考慮点

src/components/DiffLine.tsx

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,14 @@ import { Text } from "ink";
22
import React from "react";
33
import type { DisplayLine } from "../utils/formatDiff.js";
44

5+
function formatGutter(line: DisplayLine): string {
6+
const before =
7+
line.beforeLineNumber !== undefined ? String(line.beforeLineNumber).padStart(4) : " ";
8+
const after =
9+
line.afterLineNumber !== undefined ? String(line.afterLineNumber).padStart(4) : " ";
10+
return `${before} ${after} │ `;
11+
}
12+
513
export function renderDiffLine(line: DisplayLine, isCursor = false): React.ReactNode {
614
const bold = isCursor;
715
switch (line.type) {
@@ -16,17 +24,24 @@ export function renderDiffLine(line: DisplayLine, isCursor = false): React.React
1624
case "add":
1725
return (
1826
<Text color="green" bold={bold}>
27+
<Text dimColor>{formatGutter(line)}</Text>
1928
{line.text}
2029
</Text>
2130
);
2231
case "delete":
2332
return (
2433
<Text color="red" bold={bold}>
34+
<Text dimColor>{formatGutter(line)}</Text>
2535
{line.text}
2636
</Text>
2737
);
2838
case "context":
29-
return <Text bold={bold}>{line.text}</Text>;
39+
return (
40+
<Text bold={bold}>
41+
<Text dimColor>{formatGutter(line)}</Text>
42+
{line.text}
43+
</Text>
44+
);
3045
case "truncate-context":
3146
return <Text dimColor>{line.text}</Text>;
3247
case "truncation":

src/components/Help.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ export function Help({ onClose }: Props) {
4949
<KeyRow keyName="G" description="Jump to end" />
5050
<KeyRow keyName="n" description="Next file" />
5151
<KeyRow keyName="N" description="Previous file" />
52+
<KeyRow keyName="]c" description="Next change (add/delete line)" />
53+
<KeyRow keyName="[c" description="Previous change (add/delete line)" />
5254
<KeyRow keyName="f" description="File list" />
5355
<KeyRow keyName="Enter" description="Select / confirm" />
5456
<KeyRow keyName="q / Esc" description="Back / quit" />

0 commit comments

Comments
 (0)