melse.tngl.sh
a code review tool
docs: add code review view design
+110
+110
docs/plans/2026-02-15-code-review-view-design.md
+110
docs/plans/2026-02-15-code-review-view-design.md
···
1
+
# Code Review View Design
2
+
3
+
## Overview
4
+
5
+
Add a dedicated code review view that allows users to review changesets file-by-file, mark each file as reviewed, and approve the changeset when complete. Review is tied to a specific commit_id, so changes to the diff invalidate prior review progress.
6
+
7
+
## Architecture: Top-level Routing
8
+
9
+
The app gains a top-level route concept:
10
+
11
+
```ocaml
12
+
module Route = struct
13
+
type t =
14
+
| Log
15
+
| Code_review of { change_id : string; commit_id : string }
16
+
end
17
+
```
18
+
19
+
`lens_tui.ml` becomes a router:
20
+
- `Route.Log` renders the existing log app (with its own Focus state machine)
21
+
- `Route.Code_review` renders the review component full-screen
22
+
23
+
Both screens share `store`, `exit`, and `dimensions`. Route state/setter are threaded through.
24
+
25
+
### Entry Trigger
26
+
27
+
When the user selects a change in the log view, check if they have a pending review request for that change AND no valid approval (where valid means an approval with `operation_id` matching the current `commit_id`). If so, automatically transition to `Route.Code_review`.
28
+
29
+
### Exit
30
+
31
+
Escape returns to `Route.Log`. Review progress is kept in memory, keyed by `(change_id, commit_id)`.
32
+
33
+
## Review View Layout
34
+
35
+
Horizontal split-pane:
36
+
37
+
```
38
+
+-- Files ------------+-- Diff -----------------------------------+
39
+
| check src/foo.ml | diff --git a/src/bar.ml |
40
+
| > src/bar.ml | --- a/src/bar.ml |
41
+
| src/baz.ml | +++ b/src/bar.ml |
42
+
| | @@ -10,3 +10,5 @@ |
43
+
| | let foo = ... |
44
+
| | -let bar = old_thing |
45
+
| | +let bar = new_thing |
46
+
+---------------------+-------------------------------------------+
47
+
| [Shift+R] Mark reviewed [j/k] Navigate [Esc] Exit |
48
+
+---------------------------------------------------------------- +
49
+
```
50
+
51
+
- Left pane (~25% width): file list from Diff_stat.entries, with review status indicators
52
+
- Right pane (~75% width): unified diff for selected file, scrollable
53
+
- Footer: context-sensitive keybindings
54
+
55
+
### Keyboard Handling
56
+
57
+
- `j`/`k`: Navigate file list
58
+
- `Shift+R`: Mark current file as reviewed, auto-advance to next unreviewed file
59
+
- `Escape`: Exit to log view (progress kept)
60
+
- Page Up/Down, Ctrl-U/D: scroll diff pane
61
+
62
+
## Review State
63
+
64
+
### In-memory Progress
65
+
66
+
A `Map` from `(change_id * commit_id)` to `String.Set.t` (set of reviewed file paths), stored at the route level so it survives component mount/unmount cycles.
67
+
68
+
### Staleness Detection
69
+
70
+
The `commit_id` changes whenever the change's content changes (jj creates a new commit for any modification). When entering review view, compare saved `commit_id` with current. If different, clear saved progress.
71
+
72
+
### Approval Flow
73
+
74
+
When all files are marked reviewed:
75
+
1. Show confirmation prompt: "All files reviewed. Approve? [Enter] Yes [Esc] Cancel"
76
+
2. On Enter: call `Store.approve` with `commit_id` as `operation_id`
77
+
3. Return to log view
78
+
4. On Esc: stay in review view
79
+
80
+
### Re-review Requirement
81
+
82
+
Check if any approval's `operation_id` matches the current `commit_id`. If not, the change needs re-review (auto-transition triggers again).
83
+
84
+
## Data Layer Changes
85
+
86
+
### New: `Jj.fetch_file_diff`
87
+
88
+
```ocaml
89
+
val fetch_file_diff : change_id:string -> path:string -> string Lwt.t
90
+
```
91
+
92
+
Runs `jj diff -r <change_id> <path> --git` and returns raw unified diff text. Parsed minimally for Catppuccin coloring.
93
+
94
+
### Store
95
+
96
+
No schema changes. Existing `Store.approve` takes `operation_id` which we populate with `commit_id`. Add helper:
97
+
98
+
```ocaml
99
+
val is_approved_by : t -> Change_id.t -> User_id.t -> commit_id:string -> bool Lwt.t
100
+
```
101
+
102
+
## File Structure
103
+
104
+
### New Files
105
+
- `lib/tui/code_review_view.ml` — review view component
106
+
107
+
### Modified Files
108
+
- `lib/tui/lens_tui.ml` — Route type, top-level routing, auto-transition
109
+
- `lib/tui/jj.ml` / `jj.mli` — `fetch_file_diff`
110
+
- `lib/backend/store.ml` / `store.mli` — `is_approved_by` helper