diff --git a/README.md b/README.md
index d2aa67a..061ab50 100644
--- a/README.md
+++ b/README.md
@@ -3,7 +3,7 @@
- A portable, efficeint tool for reviewing code
+ A portable, efficient tool for reviewing code
## Vision
@@ -31,3 +31,4 @@ brew install sdavisde/tap/redquill
## Documentation
- [`docs/forge-setup.md`](docs/forge-setup.md) — Pull Requests tab: supported providers, zero-config detection, hosted-instance setup, troubleshooting.
+- [`docs/diff-summary.md`](docs/diff-summary.md) — the review-wide summary model: what counts as churn, how binary and rename-only files are treated, and where the numbers come from.
diff --git a/docs/diff-summary.md b/docs/diff-summary.md
new file mode 100644
index 0000000..6afcf7b
--- /dev/null
+++ b/docs/diff-summary.md
@@ -0,0 +1,49 @@
+# Review summary model
+
+`diff::summarize` rolls a whole review up into one `ReviewSummary`, so the
+question "how big is this, and where should I start?" is answerable without
+walking hunks at a render surface.
+
+## What it reports
+
+| Field | Meaning |
+| --- | --- |
+| `files` | Every file in the review — binary and rename-only included. |
+| `binary_files` | How many of those are binary. |
+| `content_files` | Files whose content actually changed (added, deleted, modified) and that carry at least one changed line. |
+| `stat` | Added/removed lines summed across the review. |
+| `largest_file` | The file with the most changed lines, as a `Hotspot { path, stat }`. |
+| `largest_hunk` | The biggest single hunk's changed-line count, across all files. |
+
+## Counting rules
+
+**Churn, not net.** `DiffStat::total()` is `added + removed`, so a line
+rewritten in place counts twice — once on each side. That's deliberate: the
+measure exists to estimate how much there is to *read*, and rewriting a line
+means reading two. `DiffStat::net()` is there for the cases that want the
+signed difference instead.
+
+**Context lines never count.** A hunk with two changed lines and forty lines
+of context is a two-line hunk as far as the summary is concerned. This
+matches `Hunk::stats`, which has always excluded context.
+
+**Binary files count as files, never as lines.** A line count over binary
+content is meaningless, so binary files land in `files` and `binary_files`
+and are skipped entirely for `stat`, `largest_file`, and `largest_hunk`.
+This is the same call `stat_display` makes when it renders `bin` instead of
+`+0 -0`.
+
+**Rename-only changes count as files, not content changes.** A pure rename
+carries no hunks, so it contributes to `files` but not to `content_files` —
+`FileChangeKind::is_content_change` draws that line.
+
+**Ties keep the earlier file.** When two files carry identical churn,
+`largest_file` reports whichever came first in the input. Callers keep the
+file list path-sorted, so the tiebreak is stable and path-ordered rather
+than arbitrary.
+
+## Where the numbers come from
+
+`build_review` computes the summary once, on the background snapshot build,
+and hands it to the UI on `ReviewSnapshot`. Render surfaces read the
+precomputed value; nothing recomputes it per frame.
diff --git a/src/diff/file.rs b/src/diff/file.rs
index 5b8ff3a..e8c1844 100644
--- a/src/diff/file.rs
+++ b/src/diff/file.rs
@@ -35,6 +35,16 @@ impl FileChangeKind {
}
}
+ /// Whether this kind implies the file's content changed. A rename or
+ /// copy may carry hunks or be path-only; every other kind always
+ /// carries content.
+ pub fn is_content_change(self) -> bool {
+ matches!(
+ self,
+ FileChangeKind::Added | FileChangeKind::Deleted | FileChangeKind::Modified
+ )
+ }
+
/// Derives the change kind from a raw patch's header text and metadata.
fn from_raw(patch: &RawFilePatch) -> FileChangeKind {
if patch.raw.contains("\nnew file mode ") {
diff --git a/src/diff/mod.rs b/src/diff/mod.rs
index 144e172..49619bb 100644
--- a/src/diff/mod.rs
+++ b/src/diff/mod.rs
@@ -12,12 +12,15 @@
//! - [`FileDiff::stats`]/[`Hunk::stats`] count added/removed lines, and
//! [`stat_display`] decides how a file's counts should render (real
//! counts, binary, or omitted).
+//! - [`summarize`] rolls a whole review's files up into a [`ReviewSummary`]:
+//! file/binary counts, total churn, and the largest file and hunk.
mod error;
mod file;
mod hunk;
mod line;
mod stat;
+mod summary;
mod word;
pub use error::DiffParseError;
@@ -25,4 +28,5 @@ pub use file::{FileChangeKind, FileDiff};
pub use hunk::{Hunk, parse_hunks};
pub use line::{DiffLine, LineOrigin};
pub use stat::{DiffStat, StatDisplay, stat_display};
+pub use summary::{Hotspot, ReviewSummary, summarize};
pub use word::{WordSpan, pair_hunk_lines, word_diff};
diff --git a/src/diff/stat.rs b/src/diff/stat.rs
index 6263b46..8581557 100644
--- a/src/diff/stat.rs
+++ b/src/diff/stat.rs
@@ -17,6 +17,25 @@ pub struct DiffStat {
pub removed: usize,
}
+impl DiffStat {
+ /// Total changed lines — added plus removed. The churn measure: a line
+ /// rewritten in place counts twice, once on each side, which is what
+ /// makes it a reasonable proxy for how much there is to read.
+ pub fn total(self) -> usize {
+ self.added + self.removed
+ }
+
+ /// Lines gained minus lines lost. Negative for a net deletion.
+ pub fn net(self) -> isize {
+ self.added as isize - self.removed as isize
+ }
+
+ /// Whether nothing changed on either side.
+ pub fn is_empty(self) -> bool {
+ self.added == 0 && self.removed == 0
+ }
+}
+
impl std::ops::AddAssign for DiffStat {
fn add_assign(&mut self, other: DiffStat) {
self.added += other.added;
@@ -47,6 +66,12 @@ impl Hunk {
acc
})
}
+
+ /// How many of this hunk's lines actually changed — its churn, context
+ /// excluded. Distinct from [`Hunk::new_count`], which spans context too.
+ pub fn changed_lines(&self) -> usize {
+ self.stats().total()
+ }
}
impl FileDiff {
@@ -87,7 +112,7 @@ pub enum StatDisplay {
pub fn stat_display(file: &FileDiff, stat: DiffStat) -> StatDisplay {
if file.is_binary {
StatDisplay::Binary
- } else if file.hunks.is_empty() || (stat.added == 0 && stat.removed == 0) {
+ } else if file.hunks.is_empty() || stat.is_empty() {
StatDisplay::Omitted
} else {
StatDisplay::Counts(stat)
@@ -141,6 +166,57 @@ mod tests {
);
}
+ #[test]
+ fn hunk_changed_lines_excludes_context() {
+ let h = hunk(vec![
+ line(LineOrigin::Context),
+ line(LineOrigin::Context),
+ line(LineOrigin::Added),
+ line(LineOrigin::Removed),
+ ]);
+ assert_eq!(h.changed_lines(), 2);
+ }
+
+ // -- DiffStat arithmetic --
+
+ #[test]
+ fn total_and_net_treat_a_rewritten_line_differently() {
+ let stat = DiffStat {
+ added: 4,
+ removed: 4,
+ };
+ assert_eq!(stat.total(), 8);
+ assert_eq!(stat.net(), 0);
+ }
+
+ #[test]
+ fn net_goes_negative_for_a_deletion_heavy_stat() {
+ let stat = DiffStat {
+ added: 1,
+ removed: 6,
+ };
+ assert_eq!(stat.net(), -5);
+ }
+
+ #[test]
+ fn is_empty_only_when_neither_side_changed() {
+ assert!(DiffStat::default().is_empty());
+ assert!(
+ !DiffStat {
+ added: 0,
+ removed: 1
+ }
+ .is_empty()
+ );
+ assert!(
+ !DiffStat {
+ added: 1,
+ removed: 0
+ }
+ .is_empty()
+ );
+ }
+
// -- FileDiff::stats --
#[test]
diff --git a/src/diff/summary.rs b/src/diff/summary.rs
new file mode 100644
index 0000000..5f2b283
--- /dev/null
+++ b/src/diff/summary.rs
@@ -0,0 +1,254 @@
+//! Review-wide aggregation over a set of [`FileDiff`]s: how many files
+//! changed, how they break down by change kind, the total added/removed
+//! counts, and which single file and hunk carry the most churn.
+//!
+//! Pure data over the diff model — the counts a reviewer wants before
+//! deciding where to start reading. Nothing here does I/O or knows about
+//! rendering.
+
+use super::file::FileDiff;
+use super::hunk::Hunk;
+use super::stat::DiffStat;
+
+/// Where the largest single unit of churn sits, so a reviewer can jump
+/// straight at the part of the review that most likely needs attention.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct Hotspot {
+ /// The file's current (b-side) path.
+ pub path: String,
+ /// The file's total added/removed counts.
+ pub stat: DiffStat,
+}
+
+/// Review-wide counts over every file in a review.
+///
+/// Binary files contribute to `files` and `binary_files` but never to
+/// `stat`: a line count over binary content is meaningless, and
+/// [`super::stat_display`] already renders them as a placeholder rather
+/// than as `+0 -0`.
+#[derive(Debug, Clone, PartialEq, Eq, Default)]
+pub struct ReviewSummary {
+ /// Total number of files in the review, binary and rename-only included.
+ pub files: usize,
+ /// How many files are binary.
+ pub binary_files: usize,
+ /// How many files changed content (added, deleted, or modified) — a
+ /// rename or copy with no hunks is excluded.
+ pub content_files: usize,
+ /// Added/removed lines summed across every file.
+ pub stat: DiffStat,
+ /// The file carrying the most changed lines, or `None` for an empty
+ /// review or one with no counted lines at all. Ties go to the file that
+ /// comes first in the input order, which callers keep path-sorted.
+ pub largest_file: Option,
+ /// The largest single hunk's changed-line count, across all files.
+ pub largest_hunk: usize,
+}
+
+impl ReviewSummary {
+ /// Whether there is nothing at all to review.
+ pub fn is_empty(&self) -> bool {
+ self.files == 0
+ }
+}
+
+/// Aggregates `files` into a [`ReviewSummary`].
+///
+/// One pass over the files and their hunks; the caller's own per-file stats
+/// are recomputed here rather than threaded in, so this stays usable from
+/// anywhere holding a slice of [`FileDiff`].
+pub fn summarize(files: &[FileDiff]) -> ReviewSummary {
+ let mut summary = ReviewSummary {
+ files: files.len(),
+ ..ReviewSummary::default()
+ };
+
+ for file in files {
+ if file.is_binary {
+ summary.binary_files += 1;
+ continue;
+ }
+
+ let stat = file.stats();
+ if stat.is_empty() {
+ continue;
+ }
+
+ if file.kind.is_content_change() {
+ summary.content_files += 1;
+ }
+ summary.stat += stat;
+
+ let largest_here = file
+ .hunks
+ .iter()
+ .map(Hunk::changed_lines)
+ .max()
+ .unwrap_or(0);
+ summary.largest_hunk = summary.largest_hunk.max(largest_here);
+
+ let bigger = match &summary.largest_file {
+ Some(current) => stat.total() > current.stat.total(),
+ None => true,
+ };
+ if bigger {
+ summary.largest_file = Some(Hotspot {
+ path: file.path.clone(),
+ stat,
+ });
+ }
+ }
+
+ summary
+}
+
+#[cfg(test)]
+mod tests {
+ use super::super::file::FileChangeKind;
+ use super::super::line::{DiffLine, LineOrigin};
+ use super::*;
+
+ fn line(origin: LineOrigin) -> DiffLine {
+ DiffLine {
+ origin,
+ old_line: None,
+ new_line: None,
+ content: String::new(),
+ no_newline: false,
+ }
+ }
+
+ /// `added`/`removed`/`context` counts of same-shaped lines in one hunk.
+ fn hunk(added: usize, removed: usize, context: usize) -> Hunk {
+ let mut lines = Vec::new();
+ lines.extend((0..removed).map(|_| line(LineOrigin::Removed)));
+ lines.extend((0..added).map(|_| line(LineOrigin::Added)));
+ lines.extend((0..context).map(|_| line(LineOrigin::Context)));
+ Hunk {
+ old_start: 1,
+ old_count: (removed + context) as u32,
+ new_start: 1,
+ new_count: (added + context) as u32,
+ section: None,
+ lines,
+ }
+ }
+
+ fn file(path: &str, kind: FileChangeKind, hunks: Vec) -> FileDiff {
+ FileDiff {
+ path: path.to_string(),
+ old_path: None,
+ kind,
+ is_binary: false,
+ hunks,
+ }
+ }
+
+ fn binary(path: &str) -> FileDiff {
+ FileDiff {
+ path: path.to_string(),
+ old_path: None,
+ kind: FileChangeKind::Modified,
+ is_binary: true,
+ hunks: Vec::new(),
+ }
+ }
+
+ #[test]
+ fn empty_review_summarizes_to_nothing() {
+ let summary = summarize(&[]);
+ assert!(summary.is_empty());
+ assert_eq!(summary.largest_file, None);
+ assert_eq!(summary.largest_hunk, 0);
+ }
+
+ #[test]
+ fn totals_sum_added_and_removed_across_files() {
+ let files = vec![
+ file(
+ "a.rs",
+ FileChangeKind::Modified,
+ vec![hunk(3, 1, 2), hunk(1, 0, 4)],
+ ),
+ file("b.rs", FileChangeKind::Added, vec![hunk(5, 0, 0)]),
+ ];
+ let summary = summarize(&files);
+ assert_eq!(
+ summary.stat,
+ DiffStat {
+ added: 9,
+ removed: 1
+ }
+ );
+ assert_eq!(summary.files, 2);
+ assert_eq!(summary.content_files, 2);
+ }
+
+ #[test]
+ fn binary_files_are_counted_but_never_add_lines() {
+ let files = vec![
+ file("a.rs", FileChangeKind::Modified, vec![hunk(2, 2, 1)]),
+ binary("logo.png"),
+ ];
+ let summary = summarize(&files);
+ assert_eq!(summary.files, 2);
+ assert_eq!(summary.binary_files, 1);
+ assert_eq!(summary.content_files, 1);
+ assert_eq!(
+ summary.stat,
+ DiffStat {
+ added: 2,
+ removed: 2
+ }
+ );
+ }
+
+ #[test]
+ fn rename_with_no_hunks_counts_as_a_file_but_not_a_content_change() {
+ let files = vec![file("new.rs", FileChangeKind::Renamed, Vec::new())];
+ let summary = summarize(&files);
+ assert_eq!(summary.files, 1);
+ assert_eq!(summary.content_files, 0);
+ assert_eq!(summary.stat, DiffStat::default());
+ assert_eq!(summary.largest_file, None);
+ }
+
+ #[test]
+ fn largest_file_is_the_one_with_the_most_changed_lines() {
+ let files = vec![
+ file("small.rs", FileChangeKind::Modified, vec![hunk(1, 1, 0)]),
+ file("big.rs", FileChangeKind::Modified, vec![hunk(4, 3, 0)]),
+ file("mid.rs", FileChangeKind::Modified, vec![hunk(2, 2, 0)]),
+ ];
+ let summary = summarize(&files);
+ let hotspot = summary.largest_file.expect("a hotspot");
+ assert_eq!(hotspot.path, "big.rs");
+ assert_eq!(
+ hotspot.stat,
+ DiffStat {
+ added: 4,
+ removed: 3
+ }
+ );
+ }
+
+ #[test]
+ fn largest_file_tie_keeps_the_earlier_file() {
+ let files = vec![
+ file("a.rs", FileChangeKind::Modified, vec![hunk(2, 2, 0)]),
+ file("b.rs", FileChangeKind::Modified, vec![hunk(2, 2, 0)]),
+ ];
+ let summary = summarize(&files);
+ assert_eq!(summary.largest_file.expect("a hotspot").path, "a.rs");
+ }
+
+ #[test]
+ fn largest_hunk_ignores_context_lines() {
+ let files = vec![file(
+ "a.rs",
+ FileChangeKind::Modified,
+ vec![hunk(1, 1, 40), hunk(3, 2, 0)],
+ )];
+ assert_eq!(summarize(&files).largest_hunk, 5);
+ }
+}
diff --git a/src/ui/app.rs b/src/ui/app.rs
index d806385..e1333f9 100644
--- a/src/ui/app.rs
+++ b/src/ui/app.rs
@@ -12,7 +12,7 @@ use crate::annotate::{AnnotationStore, Source, Target};
#[cfg(test)]
use crate::annotate::Side;
use crate::config::{Config, ConfigWarning};
-use crate::diff::{DiffStat, FileDiff, StatDisplay};
+use crate::diff::{DiffStat, FileDiff, ReviewSummary, StatDisplay};
use crate::git::{
BranchStatus, CommitLogEntry, CommitSummary, DiffTarget, LocalBranch, RawFilePatch, RemoteOp,
StagingMode, StashEntry, commit_command_line, remote_command,
@@ -309,6 +309,11 @@ pub struct App {
/// `view.files`, refreshed alongside `stats`. Shown in the git panel's
/// bottom counts line and the review banner.
pub total_stats: DiffStat,
+ /// The review-wide roll-up over `view.files`, refreshed alongside
+ /// `stats`: file/binary counts, total churn, and the largest file and
+ /// hunk. The git panel's counts line reads the binary count from here;
+ /// the hotspot fields are what a "start here" jump will read next.
+ pub summary: ReviewSummary,
/// Per-path [`ReviewStatus`] driving the accept/defer markers and the
/// review banner's progress count (see [`super::review_ops`]),
/// mirroring how `staged_states` drives the
@@ -753,6 +758,8 @@ pub(super) struct SuspendedView {
pub(super) stats: HashMap,
/// `target`'s aggregate added/removed counts.
pub(super) total_stats: DiffStat,
+ /// `target`'s review-wide roll-up.
+ pub(super) summary: ReviewSummary,
}
/// Which mutating background git operation is in flight (see
@@ -811,7 +818,8 @@ impl App {
.iter()
.map(|f| (f.path.clone(), crate::diff::stat_display(f, f.stats())))
.collect();
- let total_stats: DiffStat = files.iter().map(FileDiff::stats).sum();
+ let summary = crate::diff::summarize(&files);
+ let total_stats: DiffStat = summary.stat;
let mut app = App {
view: DiffViewState::new(files),
help: HelpOverlayState::new(),
@@ -831,6 +839,7 @@ impl App {
staged_states: HashMap::new(),
stats,
total_stats,
+ summary,
review_states: HashMap::new(),
staging_cursor: 0,
staging_filter: None,
@@ -930,6 +939,7 @@ impl App {
app.staged_states = snapshot.staged_states;
app.stats = snapshot.stats;
app.total_stats = snapshot.total;
+ app.summary = snapshot.summary;
app.target = target;
app.stage_ops = Some(ops);
app.recompute_untracked();
diff --git a/src/ui/file_view.rs b/src/ui/file_view.rs
index da708f1..af36588 100644
--- a/src/ui/file_view.rs
+++ b/src/ui/file_view.rs
@@ -23,7 +23,7 @@
use std::collections::HashMap;
-use crate::diff::{FileDiff, stat_display};
+use crate::diff::{FileDiff, stat_display, summarize};
use crate::git::DiffTarget;
use super::app::{App, Mode, SuspendedView};
@@ -76,6 +76,7 @@ impl App {
// `stat_display`'s own rule stays the single source of truth.
let file_stat = file.stats();
let file_stats_map = HashMap::from([(file.path.clone(), stat_display(&file, file_stat))]);
+ let file_summary = summarize(std::slice::from_ref(&file));
if self.suspended_file_view.is_none() {
self.file_view_return_mode = return_mode;
@@ -89,6 +90,7 @@ impl App {
staged_states: std::mem::take(&mut self.staged_states),
stats: std::mem::replace(&mut self.stats, file_stats_map),
total_stats: std::mem::replace(&mut self.total_stats, file_stat),
+ summary: std::mem::replace(&mut self.summary, file_summary),
});
} else {
self.target = target;
@@ -96,6 +98,7 @@ impl App {
self.patches = vec![None];
self.stats = file_stats_map;
self.total_stats = file_stat;
+ self.summary = file_summary;
}
// The just-suspended (or just-replaced) content shares the highlight
@@ -140,6 +143,7 @@ impl App {
self.staged_states = suspended.staged_states;
self.stats = suspended.stats;
self.total_stats = suspended.total_stats;
+ self.summary = suspended.summary;
self.highlight_cache.clear();
self.rebuild_rows();
self.mode = self.file_view_return_mode;
diff --git a/src/ui/git_panel.rs b/src/ui/git_panel.rs
index 3bce834..302f2e3 100644
--- a/src/ui/git_panel.rs
+++ b/src/ui/git_panel.rs
@@ -641,6 +641,11 @@ pub fn render(frame: &mut Frame, area: Rect, app: &App, keymap: &Keymap) {
),
dim("]".to_string()),
];
+ // Binary files sit in the file count but contribute nothing to `+A -R`,
+ // so call them out rather than leaving the counts looking short.
+ if app.summary.binary_files > 0 {
+ counts_spans.push(dim(format!(" [{} bin]", app.summary.binary_files)));
+ }
if !app.staged.is_empty() {
counts_spans.push(dim(format!(" [{} staged]", app.staged.len())));
}
@@ -1010,6 +1015,7 @@ impl App {
staged_states: std::mem::replace(&mut self.staged_states, snapshot.staged_states),
stats: std::mem::replace(&mut self.stats, snapshot.stats),
total_stats: std::mem::replace(&mut self.total_stats, snapshot.total),
+ summary: std::mem::replace(&mut self.summary, snapshot.summary),
});
} else {
self.target = target;
@@ -1019,6 +1025,7 @@ impl App {
self.staged_states = snapshot.staged_states;
self.stats = snapshot.stats;
self.total_stats = snapshot.total;
+ self.summary = snapshot.summary;
}
self.active_commit = header;
self.recompute_untracked();
@@ -1047,6 +1054,7 @@ impl App {
self.staged_states = suspended.staged_states;
self.stats = suspended.stats;
self.total_stats = suspended.total_stats;
+ self.summary = suspended.summary;
self.active_commit = None;
self.recompute_untracked();
self.highlight_cache.clear();
diff --git a/src/ui/refresh.rs b/src/ui/refresh.rs
index 048cfc4..1b02cbe 100644
--- a/src/ui/refresh.rs
+++ b/src/ui/refresh.rs
@@ -286,6 +286,7 @@ impl App {
self.staged_states = snapshot.staged_states;
self.stats = snapshot.stats;
self.total_stats = snapshot.total;
+ self.summary = snapshot.summary;
self.recompute_untracked();
self.refresh_repo_state();
diff --git a/src/ui/stage_ops.rs b/src/ui/stage_ops.rs
index ae4e802..09874dc 100644
--- a/src/ui/stage_ops.rs
+++ b/src/ui/stage_ops.rs
@@ -11,7 +11,10 @@ use std::process::Command;
use thiserror::Error;
-use crate::diff::{DiffParseError, DiffStat, FileChangeKind, FileDiff, StatDisplay, stat_display};
+use crate::diff::{
+ DiffParseError, DiffStat, FileChangeKind, FileDiff, ReviewSummary, StatDisplay, stat_display,
+ summarize,
+};
use crate::forge::{
self, CredentialChecker, ForgeError, GhCredentialChecker, GlabCredentialChecker, ProviderKind,
ProviderResolution, PullRequest, ResolutionCache, Thread, UnresolvedReason,
@@ -1127,7 +1130,12 @@ pub struct ReviewSnapshot {
/// `files`) fall back to [`StatDisplay::Omitted`].
pub stats: HashMap,
/// The aggregate added/removed counts across every file in `files`.
+ /// Same value as `summary.stat`, kept as its own field so the render
+ /// surfaces that only want the totals don't reach through the summary.
pub total: DiffStat,
+ /// The review-wide roll-up: file and binary counts, total churn, and the
+ /// largest file and hunk in the review.
+ pub summary: ReviewSummary,
}
/// A single file's staged state, derived from its `git status` index-side
@@ -1304,14 +1312,13 @@ pub fn build_review(
let (files, patches): (Vec, Vec