+35

lib/backend/store.ml

··· 163 163 Lwt.return []) 164 164 ;; 165 165 166 166 + let latest_approval_by t change_id user_id = 167 167 + Lwt.catch 168 168 + (fun () -> 169 169 + let open Lwt.Syntax in 170 170 + let open Lwt.Infix in 171 171 + Log.debug (fun m -> 172 172 + m 173 173 + "fetching latest approval: change=%s user=%s" 174 174 + (Change_id.to_string change_id) 175 175 + (User_id.to_string user_id)); 176 176 + let* tree = 177 177 + find_tree 178 178 + t 179 179 + [ "approve"; Change_id.to_string change_id; User_id.to_string user_id ] 180 180 + in 181 181 + match tree with 182 182 + | None -> Lwt.return_none 183 183 + | Some tree -> 184 184 + let* children = Tree.list tree [] in 185 185 + let+ approvals = 186 186 + Lwt.all 187 187 + (List.map children ~f:(fun (_, tree) -> 188 188 + Tree.get tree [] >|= Entry.approve_exn)) 189 189 + in 190 190 + List.last approvals) 191 191 + (fun exn -> 192 192 + Log.err (fun m -> 193 193 + m 194 194 + "failed to fetch latest approval for %s/%s: %s" 195 195 + (Change_id.to_string change_id) 196 196 + (User_id.to_string user_id) 197 197 + (Exn.to_string exn)); 198 198 + Lwt.return_none) 199 199 + ;; 200 200 + 166 201 let is_approved_by t change_id user_id commit_id = 167 202 let open Lwt.Syntax in 168 203 let* all_approvals = approvals t change_id in

+2

lib/backend/store.mli

··· 34 34 val request : t -> Change_id.t -> User_id.t -> unit Lwt.t 35 35 val pending_requests : t -> Change_id.t -> Request.t list Lwt.t 36 36 val approve : t -> Change_id.t -> User_id.t -> Commit_id.t -> unit Lwt.t 37 37 + 37 38 val approvals : t -> Change_id.t -> Approval.t list Lwt.t 39 39 + val latest_approval_by : t -> Change_id.t -> User_id.t -> Approval.t option Lwt.t 38 40 val is_approved_by : t -> Change_id.t -> User_id.t -> Commit_id.t -> bool Lwt.t

+65 -19

lib/tui/code_review_view.ml

··· 90 90 path)) 91 91 ;; 92 92 93 93 - let render_footer ~(flavor : Catppuccin.Flavor.t) ~reviewed_count ~total_count = 93 93 + let render_footer ~(flavor : Catppuccin.Flavor.t) ~reviewed_count ~total_count ~is_interdiff = 94 94 let shortcut = shortcut ~flavor in 95 95 + let c color = Catppuccin.color ~flavor color in 95 96 let progress = 96 97 View.text 97 97 - ~attrs:[ Attr.fg (Catppuccin.color ~flavor Blue) ] 98 98 + ~attrs:[ Attr.fg (c Blue) ] 98 99 (sprintf " (%d/%d reviewed)" reviewed_count total_count) 99 100 in 101 101 + let interdiff_indicator = 102 102 + if is_interdiff 103 103 + then 104 104 + View.hcat 105 105 + [ View.text " " 106 106 + ; View.text ~attrs:[ Attr.fg (c Peach); Attr.bold ] "[interdiff]" 107 107 + ] 108 108 + else View.none 109 109 + in 100 110 View.hcat 101 111 [ shortcut "j/k" "select file" 102 112 ; View.text " " ··· 106 116 ; View.text " " 107 117 ; shortcut "q/esc" "back" 108 118 ; progress 119 119 + ; interdiff_indicator 109 120 ] 110 121 ;; 111 122 ··· 133 144 View.center prompt_box ~within:dimensions 134 145 ;; 135 146 136 136 - let diff_pane ~dimensions ~change_id ~selected_file_path (graph @ local) = 147 147 + let diff_pane 148 148 + ~dimensions 149 149 + ~change_id 150 150 + ~prev_tip 151 151 + ~(commit_id : Commit_id.t Bonsai.t) 152 152 + ~selected_file_path 153 153 + (graph @ local) 154 154 + = 137 155 let diff_poll_input = 138 138 - let%arr selected_file_path and change_id in 139 139 - selected_file_path, change_id 156 156 + let%arr selected_file_path and change_id and prev_tip and commit_id in 157 157 + selected_file_path, change_id, prev_tip, commit_id 140 158 in 141 159 let file_diff = 142 160 Bonsai.Edge.Poll.effect_on_change 143 161 Bonsai.Edge.Poll.Starting.empty 144 162 diff_poll_input 145 145 - ~equal_input:[%equal: string option * Change_id.t] 163 163 + ~equal_input: 164 164 + [%equal: string option * Change_id.t * Commit_id.t option * Commit_id.t] 146 165 ~effect: 147 166 (Bonsai.return 148 148 - @@ fun (path_opt, change_id) -> 167 167 + @@ fun (path_opt, change_id, prev_tip, commit_id) -> 149 168 match path_opt with 150 169 | None -> Ui_effect.return None 151 170 | Some path -> 152 171 let open Effect.Let_syntax in 153 172 let%bind diff = 154 154 - Effect.of_lwt_thunk (fun () -> Jj.fetch_file_diff change_id ~path) 173 173 + Effect.of_lwt_thunk (fun () -> 174 174 + match prev_tip with 175 175 + | Some from_tip -> 176 176 + Jj.fetch_file_interdiff ~from_tip ~to_tip:commit_id ~path 177 177 + | None -> Jj.fetch_file_diff change_id ~path) 155 178 in 156 179 return (Some diff)) 157 180 graph ··· 189 212 let component 190 213 ~(dimensions : Dimensions.t Bonsai.t) 191 214 ~(change_id : Change_id.t Bonsai.t) 215 215 + ~(commit_id : Commit_id.t Bonsai.t) 216 216 + ~(prev_tip : Commit_id.t option Bonsai.t) 192 217 ~(on_exit : unit Effect.t Bonsai.t) 193 218 ~(on_approve : unit Effect.t Bonsai.t) 194 219 ~(reviewed_files : String.Set.t Bonsai.t) 195 220 ~(set_reviewed_files : (String.Set.t -> unit Effect.t) Bonsai.t) 196 221 (graph @ local) 197 222 = 198 198 - (* Fetch diff_stat internally *) 199 199 - let diff_stat = 223 223 + (* Fetch file list: use interdiff when re-reviewing, full diff_stat otherwise *) 224 224 + let file_list_poll_input = 225 225 + let%arr change_id and prev_tip and commit_id in 226 226 + change_id, prev_tip, commit_id 227 227 + in 228 228 + let file_paths_result = 200 229 Bonsai.Edge.Poll.effect_on_change 201 230 Bonsai.Edge.Poll.Starting.empty 202 202 - change_id 203 203 - ~equal_input:[%equal: Change_id.t] 231 231 + file_list_poll_input 232 232 + ~equal_input:[%equal: Change_id.t * Commit_id.t option * Commit_id.t] 204 233 ~effect: 205 234 (Bonsai.return 206 206 - @@ fun change_id -> Effect.of_lwt_thunk (fun () -> Jj.fetch_diff_stat change_id) 207 207 - ) 235 235 + @@ fun (change_id, prev_tip, commit_id) -> 236 236 + match prev_tip with 237 237 + | Some from_tip -> 238 238 + let open Effect.Let_syntax in 239 239 + let%bind paths = 240 240 + Effect.of_lwt_thunk (fun () -> 241 241 + Jj.fetch_interdiff_file_paths ~from_tip ~to_tip:commit_id) 242 242 + in 243 243 + return (`Paths paths) 244 244 + | None -> 245 245 + let open Effect.Let_syntax in 246 246 + let%bind stat = 247 247 + Effect.of_lwt_thunk (fun () -> Jj.fetch_diff_stat change_id) 248 248 + in 249 249 + return 250 250 + (`Paths 251 251 + (List.map stat.entries ~f:(fun (e : Jj.Diff_stat.Entry.t) -> e.path)))) 208 252 graph 209 253 in 210 254 let file_paths = 211 211 - match%sub diff_stat with 255 255 + match%sub file_paths_result with 212 256 | None -> Bonsai.return [] 213 213 - | Some diff_stat -> 214 214 - let%arr diff_stat in 215 215 - List.map diff_stat.entries ~f:(fun (e : Jj.Diff_stat.Entry.t) -> e.path) 257 257 + | Some (`Paths paths) -> 258 258 + let%arr paths in 259 259 + paths 216 260 in 217 261 let num_files = 218 262 let%arr file_paths in ··· 253 297 } 254 298 in 255 299 let diff_scroll_view, diff_inject = 256 256 - diff_pane ~dimensions:diff_dims ~change_id ~selected_file_path graph 300 300 + diff_pane ~dimensions:diff_dims ~change_id ~prev_tip ~commit_id ~selected_file_path graph 257 301 in 258 302 (* Keyboard handler *) 259 303 let visible_height = ··· 353 397 and showing_approval_prompt 354 398 and diff_scroll_view 355 399 and file_paths 400 400 + and prev_tip 356 401 and flavor = Catppuccin.flavor graph in 357 402 let c color = Catppuccin.color ~flavor color in 358 403 let content_width = dimensions.width - (2 * pad_w) in ··· 378 423 ~flavor 379 424 ~reviewed_count:(Set.length reviewed_files) 380 425 ~total_count:(List.length file_paths) 426 426 + ~is_interdiff:(Option.is_some prev_tip) 381 427 in 382 428 let main_content = View.hcat [ file_list; separator; diff_scroll_view ] in 383 429 let bg =

+29

lib/tui/jj.ml

··· 202 202 let fetch_file_diff change_id ~path = 203 203 run_jj [ "diff"; "-r"; Change_id.to_string change_id; path; "--git" ] 204 204 ;; 205 205 + 206 206 + let fetch_interdiff_file_paths ~from_tip ~to_tip = 207 207 + let open Lwt.Infix in 208 208 + run_jj 209 209 + [ "interdiff" 210 210 + ; "--from" 211 211 + ; Commit_id.to_string from_tip 212 212 + ; "--to" 213 213 + ; Commit_id.to_string to_tip 214 214 + ; "--name-only" 215 215 + ] 216 216 + >|= fun raw -> 217 217 + raw 218 218 + |> String.split_lines 219 219 + |> List.filter ~f:(fun s -> not (String.is_empty (String.strip s))) 220 220 + ;; 221 221 + 222 222 + let fetch_file_interdiff ~from_tip ~to_tip ~path = 223 223 + run_jj 224 224 + [ "interdiff" 225 225 + ; "--from" 226 226 + ; Commit_id.to_string from_tip 227 227 + ; "--to" 228 228 + ; Commit_id.to_string to_tip 229 229 + ; "--git" 230 230 + ; "--" 231 231 + ; path 232 232 + ] 233 233 + ;;

+13

lib/tui/jj.mli

··· 71 71 72 72 (** Fetch a unified diff for a specific file in a change. *) 73 73 val fetch_file_diff : Change_id.t -> path:string -> string Lwt.t 74 74 + 75 75 + (** Fetch file paths changed between two review snapshots via [jj interdiff]. *) 76 76 + val fetch_interdiff_file_paths 77 77 + : from_tip:Commit_id.t 78 78 + -> to_tip:Commit_id.t 79 79 + -> string list Lwt.t 80 80 + 81 81 + (** Fetch a unified interdiff for a specific file between two review snapshots. *) 82 82 + val fetch_file_interdiff 83 83 + : from_tip:Commit_id.t 84 84 + -> to_tip:Commit_id.t 85 85 + -> path:string 86 86 + -> string Lwt.t

+31 -9

lib/tui/lens_tui.ml

··· 49 49 | Code_review of 50 50 { change_id : Change_id.t 51 51 ; commit_id : Commit_id.t 52 52 + ; prev_tip : Commit_id.t option 52 53 } 53 54 [@@deriving equal, sexp_of] 54 55 end ··· 73 74 let%arr set_route in 74 75 fun (change : Jj.Change.t) -> 75 76 let open Effect.Let_syntax in 76 76 - let%bind should_review = 77 77 + let%bind review_info = 77 78 Effect.of_lwt_thunk (fun () -> 78 79 let open Lwt.Syntax in 79 80 let* reqs = ··· 85 86 in 86 87 if is_reviewer 87 88 then 88 88 - let+ already_approved = 89 89 + let* already_approved = 89 90 Lens_backend.Store.is_approved_by 90 91 store 91 92 change.change_id 92 93 user_id 93 94 change.commit_id 94 95 in 95 95 - not already_approved 96 96 - else Lwt.return false) 96 96 + if already_approved 97 97 + then Lwt.return `Already_approved 98 98 + else 99 99 + let+ prev = 100 100 + Lens_backend.Store.latest_approval_by 101 101 + store 102 102 + change.change_id 103 103 + user_id 104 104 + in 105 105 + let prev_tip = 106 106 + match prev with 107 107 + | Some (a : Lens_backend.Store.Approval.t) 108 108 + when not (Commit_id.equal a.commit_id change.commit_id) -> 109 109 + Some a.commit_id 110 110 + | _ -> None 111 111 + in 112 112 + `Should_review prev_tip 113 113 + else Lwt.return `Not_reviewer) 97 114 in 98 98 - if should_review 99 99 - then 115 115 + match review_info with 116 116 + | `Should_review prev_tip -> 100 117 set_route 101 118 (Code_review 102 102 - { change_id = change.change_id; commit_id = change.commit_id }) 103 103 - else Effect.Ignore 119 119 + { change_id = change.change_id 120 120 + ; commit_id = change.commit_id 121 121 + ; prev_tip 122 122 + }) 123 123 + | `Already_approved | `Not_reviewer -> Effect.Ignore 104 124 in 105 125 let ~view, ~handler = 106 126 Log_screen.component ~dimensions ~store ~exit ~enter_review ~review_generation graph 107 127 in 108 128 let%arr view and handler in 109 129 view, handler 110 110 - | Code_review { change_id; commit_id } -> 130 130 + | Code_review { change_id; commit_id; prev_tip } -> 111 131 let reviewed_files = 112 132 let%arr reviewed_files_map and change_id and commit_id in 113 133 let key = [%string "%{change_id#Change_id}:%{commit_id#Commit_id}"] in ··· 140 160 Code_review_view.component 141 161 ~dimensions 142 162 ~change_id 163 163 + ~commit_id 164 164 + ~prev_tip 143 165 ~on_exit 144 166 ~on_approve 145 167 ~reviewed_files