hive-forge: collapse autogenerated diffs to a single one-line summary (#222)

hive-forge/src/verbs/diff.rs

@@ -1,14 +1,16 @@
 //! `diff <pr> [repo]` — print the unified diff for a PR.
 //!
-//! By default the hunks for known autogenerated lockfiles
+//! By default any per-file section whose target path matches a
-//! (`flake.lock`, `Cargo.lock`, `package-lock.json`, …) are
+//! known autogenerated lockfile (`flake.lock`, `Cargo.lock`,
-//! collapsed to a `[<path>: +N -M (autogenerated; pass --full for
+//! `package-lock.json`, …) is collapsed to a single
-//! content)]` placeholder so a `flake.lock` rev bump doesn't drown
+//! `[<path>: contents changed (+N -M, --full for content)]`
-//! the human-authored changes in 5 000 lines of lock churn
+//! line so a `flake.lock` rev bump doesn't drown the human-
-//! (#222). The header (`diff --git`, `index`, `---`, `+++`,
+//! authored changes in 5 000 lines of lock churn (#222). The
-//! rename / mode metadata) is preserved so the reader can still
+//! per-file git headers (`diff --git`, `index`, `---`, `+++`,
-//! see WHICH lockfiles changed; the +/- counts give a `diff
+//! and any rename / mode metadata) are suppressed alongside the
-//! --stat`-style magnitude (excluding the `@@` hunk header).
+//! hunks since the placeholder already carries the file path and
+//! the +/- magnitude — the headers add four lines of noise per
+//! lockfile without any information the reader can act on.
 //! Pass `--full` to dump the unfiltered diff.
 
 use anyhow::Result;
@@ -40,13 +42,12 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
     Ok(())
 }
 
-/// Walk a unified diff line-by-line. For each per-file section
+/// Walk a unified diff line-by-line. When a `diff --git` line
-/// whose target path matches a known autogenerated file
+/// names a known autogenerated file (`is_autogenerated`), suppress
-/// (`is_autogenerated`), drop every line from the first hunk
+/// every line of that per-file section (the git headers AND the
-/// header (`@@`) onward and emit a single `diff --stat`-style
+/// hunks) and emit a single `[file.lock: contents changed (+N -M,
-/// `[file.lock: +N -M (autogenerated, --full for content)]`
+/// --full for content)]` line in their place. Non-autogenerated
-/// placeholder before the next file. Non-autogenerated files
+/// files pass through unchanged.
-/// pass through unchanged.
 fn collapse_autogenerated(diff: &str) -> String {
     let mut out = String::with_capacity(diff.len());
     let mut state: Option<CollapseState> = None;
@@ -61,23 +62,23 @@ fn collapse_autogenerated(diff: &str) -> String {
             if let Some(p) = path.as_deref()
                 && is_autogenerated(p)
             {
+                // Suppress the entire per-file block (headers and
+                // hunks alike). The placeholder we emit on flush
+                // carries the path, so the git headers are pure
+                // noise.
                 state = Some(CollapseState::new(p.to_owned()));
+                continue;
             }
             out.push_str(line);
             out.push('\n');
             continue;
         }
         if let Some(s) = state.as_mut() {
-            // First `@@` marks the boundary between file header and
+            // Inside an autogenerated section. Tally `+`/`-` body
-            // hunk content; everything from this point is
+            // lines for the placeholder; drop everything else
-            // suppressed (and tallied) while `state` is Some.
+            // (headers, `@@` markers, context lines).
-            if !s.in_body && line.starts_with("@@") {
+            if s.in_body || line.starts_with("@@") {
                 s.in_body = true;
-            }
-            if s.in_body {
-                // The hunk header `@@` itself counts as a body line
-                // for the +/− tally only via its descendant content
-                // lines; skip it for the counters.
                 if !line.starts_with("@@") {
                     match line.as_bytes().first() {
                         Some(b'+') => s.added += 1,
@@ -85,8 +86,8 @@ fn collapse_autogenerated(diff: &str) -> String {
                         _ => {}
                     }
                 }
-                continue;
             }
+            continue;
         }
         out.push_str(line);
         out.push('\n');
@@ -97,11 +98,11 @@ fn collapse_autogenerated(diff: &str) -> String {
     out
 }
 
-/// Per-file accumulator for the collapsed-hunk placeholder. Tracks
+/// Per-file accumulator for the collapsed placeholder. Tracks
-/// the file's display name plus `+`/`−` line counts (excluding the
+/// the file's display name plus `+`/`-` line counts (excluding
-/// `@@` hunk header), so the placeholder shows operator-meaningful
+/// the `@@` hunk header), so the placeholder shows operator-
-/// magnitude instead of "N lines omitted" (which mixed context +
+/// meaningful magnitude instead of "N lines omitted" (which
-/// added + removed indistinguishably).
+/// mixed context + added + removed indistinguishably).
 struct CollapseState {
     path: String,
     in_body: bool,
@@ -121,7 +122,7 @@ impl CollapseState {
 
     fn placeholder(&self) -> String {
         format!(
-            "[{}: +{} -{} (autogenerated; pass --full for content)]\n",
+            "[{}: contents changed (+{} -{}, --full for content)]\n",
             self.path, self.added, self.removed
         )
     }
@@ -247,18 +248,23 @@ index 3333..4444 100644
 +fn main() { println!(\"hi\"); }
 ";
         let out = collapse_autogenerated(diff);
-        assert!(out.contains("diff --git a/Cargo.lock"));
+        // Per-file git headers for the lockfile are suppressed —
-        assert!(out.contains("--- a/Cargo.lock"));
+        // the placeholder already carries the path, so the four
-        assert!(out.contains("+++ b/Cargo.lock"));
+        // header lines are pure noise (4 lines per lockfile, stacks
+        // fast on a multi-lockfile PR).
+        assert!(!out.contains("diff --git a/Cargo.lock"));
+        assert!(!out.contains("--- a/Cargo.lock"));
+        assert!(!out.contains("+++ b/Cargo.lock"));
         assert!(!out.contains("[[package]]"));
         // Stat-style placeholder: 1 added (`+name = "new"`), 1
         // removed (`-name = "old"`); context line and `@@` header
         // don't count.
         assert!(
-            out.contains("[Cargo.lock: +1 -1"),
+            out.contains("[Cargo.lock: contents changed (+1 -1, --full for content)]"),
-            "expected stat placeholder, got: {out}"
+            "expected one-line placeholder, got: {out}"
         );
         // Non-lockfile file passes through untouched.
+        assert!(out.contains("diff --git a/src/main.rs"));
         assert!(out.contains("fn main() {}"));
         assert!(out.contains("println!(\"hi\")"));
     }
@@ -300,13 +306,17 @@ index 1111..2222 100644
 +lock-line-two-bumped
 ";
         let out = collapse_autogenerated(diff);
+        // README.md (non-lockfile) passes through with its full
+        // header + hunks intact.
+        assert!(out.contains("diff --git a/README.md"));
         assert!(out.contains("-old"));
         assert!(out.contains("+new"));
-        assert!(out.contains("diff --git a/flake.lock"));
+        // Lockfile is fully suppressed — no headers, no content.
+        assert!(!out.contains("diff --git a/flake.lock"));
         assert!(!out.contains("lock-line-one"));
         assert!(
-            out.contains("[flake.lock: +1 -1"),
+            out.contains("[flake.lock: contents changed (+1 -1, --full for content)]"),
-            "expected stat placeholder, got: {out}"
+            "expected one-line placeholder, got: {out}"
         );
     }
 
@@ -331,7 +341,7 @@ index 1111..2222 100644
 ";
         let out = collapse_autogenerated(diff);
         assert!(
-            out.contains("[flake.lock: +4 -1"),
+            out.contains("[flake.lock: contents changed (+4 -1, --full for content)]"),
             "expected +4 -1, got: {out}"
         );
     }
@@ -401,13 +411,14 @@ index 1111..2222 100644
 +new
 ";
         let out = collapse_autogenerated(diff);
-        // Content must be suppressed (lockfile detected).
+        // Entire per-file block suppressed (lockfile detected).
+        assert!(!out.contains("diff --git"), "headers leaked: {out}");
         assert!(!out.contains("old\n"), "lock content leaked: {out}");
         assert!(!out.contains("+new"), "lock content leaked: {out}");
         // Placeholder uses the parsed b-side path (without quotes).
         assert!(
-            out.contains("[odd dir/Cargo.lock: +1 -1"),
+            out.contains("[odd dir/Cargo.lock: contents changed (+1 -1, --full for content)]"),
-            "expected stat placeholder for quoted-path lockfile, got: {out}"
+            "expected one-line placeholder for quoted-path lockfile, got: {out}"
         );
     }
 }