hive-forge: post inline review comments with pr reviews --at, number diff lines with pr diff --numbered

docs/4-implementation/agent-tools/forge-cli.md

@@ -609,6 +609,7 @@ List a PR's reviews, or submit one
 * `--request-changes` — Request changes on the PR (submit a `REQUEST_CHANGES` review)
 * `--comment` — Leave a comment review (submit a COMMENT review)
 * `-m`, `--body <BODY>` — Optional body / message for the review (used with --approve, --request-changes, or --comment)
+* `--at <PATH:LINE>` — An inline comment on a line of the new file: `--at path:LINE "text"`. Repeat it to put several comments in the same review. LINE is the new-file line number, which `pr diff --numbered` prints. Needs --approve, --request-changes or --comment
 * `--only <ONLY>` — When listing: narrow the inline comments shown (and leave out the reviews with none left). `unresolved` keeps every comment nobody has resolved yet: what a re-review has to check
 
   Possible values:
@@ -666,6 +667,7 @@ Print the PR's unified diff
 ###### **Options:**
 
 * `--full` — Print the unfiltered diff. By default, hive-forge collapses autogenerated-file hunks (`flake.lock`, `Cargo.lock`, …) to a placeholder so human-authored changes stay readable
+* `--numbered` — Prefix every added or unchanged line with its new-file line number, the number `pr reviews --at path:LINE` takes. Removed lines get no number: an inline comment sits on the new file
 
 
 

docs/4-implementation/agent-tools/forge.md

@@ -83,11 +83,13 @@ hive-forge pr timeline 42                  # review rows list their line comment
 hive-forge pr reviews 42 --approve         # submit APPROVED review
 hive-forge pr reviews 42 --request-changes -m "msg"  # submit REQUEST_CHANGES review
 hive-forge pr reviews 42 --comment -m "msg"  # submit COMMENT review
+hive-forge pr reviews 42 --request-changes -m "msg" --at src/a.rs:12 "off by one" --at src/b.rs:3 "why?"  # one review carrying line comments; LINE is the new-file line `pr diff --numbered` prints
 hive-forge pr assign-reviewer 42 argus      # set argus as a requested reviewer on the PR
 hive-forge pr assign-reviewer 42 argus --remove  # withdraw the request; fails if argus is still listed afterwards (a reviewer who already reviewed stays listed)
 hive-forge pr assign-committer 42 damocles  # add damocles to the PR's assignee list
 hive-forge pr diff 42                       # unified diff (lockfile hunks collapsed by default)
 hive-forge pr diff 42 --full                # include unfiltered lockfile hunks
+hive-forge pr diff 42 --numbered            # prefix added/unchanged lines with their new-file line number, for `pr reviews --at path:LINE`
 hive-forge list                             # open issues/PRs
 hive-forge list --kind pr --state all --page 2  # page 2 of all PRs (walk --page 1,2,… with --limit as page size for a repo-wide sweep)
 hive-forge list --search "trust bundle" --state all  # full-text over title AND body — the duplicate check

hive-forge/src/verbs/diff.rs

@@ -13,6 +13,8 @@
 //! lockfile without any information the reader can act on.
 //! Pass `--full` to dump the unfiltered diff.
 
+use std::fmt::Write as _;
+
 use anyhow::Result;
 use clap::Args as ClapArgs;
 use forgejo_api::structs::RepoDownloadPullDiffOrPatchQuery;
@@ -29,6 +31,11 @@ pub struct Args {
     /// placeholder so human-authored changes stay readable.
     #[arg(long)]
     full: bool,
+    /// Prefix every added or unchanged line with its new-file line number,
+    /// the number `pr reviews --at path:LINE` takes. Removed lines get no
+    /// number: an inline comment sits on the new file.
+    #[arg(long)]
+    numbered: bool,
 }
 
 pub fn run(client: &Client, args: Args) -> Result<()> {
@@ -49,10 +56,57 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
     } else {
         collapse_autogenerated(&diff)
     };
-    print!("{out}");
+    if args.numbered {
+        print!("{}", number_new_lines(&out));
+    } else {
+        print!("{out}");
+    }
     Ok(())
 }
 
+/// Width of the line-number column `number_new_lines` adds.
+const NUMBER_WIDTH: usize = 6;
+
+/// Prefix each line of every hunk with its new-file line number (added and
+/// unchanged lines) or blanks (removed lines and `\ No newline` markers), so a
+/// reviewer can read the `LINE` of `pr reviews --at path:LINE` off the diff.
+/// Everything outside a hunk (git headers, collapsed placeholders) is passed
+/// through unchanged.
+fn number_new_lines(diff: &str) -> String {
+    let blank = " ".repeat(NUMBER_WIDTH);
+    let mut out = String::with_capacity(diff.len() + diff.len() / 8);
+    let mut next: Option<u64> = None;
+    for line in diff.lines() {
+        if let Some(rest) = line.strip_prefix("@@ ") {
+            next = hunk_new_start(rest);
+            out.push_str(line);
+        } else if line.starts_with("diff --git ") {
+            next = None;
+            out.push_str(line);
+        } else if let Some(n) = next {
+            if line.starts_with('-') || line.starts_with('\\') {
+                out.push_str(&blank);
+            } else {
+                let _ = write!(out, "{n:>NUMBER_WIDTH$}");
+                next = Some(n + 1);
+            }
+            out.push(' ');
+            out.push_str(line);
+        } else {
+            out.push_str(line);
+        }
+        out.push('\n');
+    }
+    out
+}
+
+/// The new-file start line of a hunk header, from `-a,b +c,d @@ ...` (the
+/// text after the leading `@@ `): `c`.
+fn hunk_new_start(header: &str) -> Option<u64> {
+    let new = header.split_whitespace().find(|w| w.starts_with('+'))?;
+    new.trim_start_matches('+').split(',').next()?.parse().ok()
+}
+
 /// Walk a unified diff line-by-line. When a `diff --git` line
 /// names a known autogenerated file (`is_autogenerated`), suppress
 /// every line of that per-file section (the git headers AND the
@@ -224,6 +278,58 @@ fn is_autogenerated(path: &str) -> bool {
 mod tests {
     use super::*;
 
+    const TWO_FILES: &str = "\
+diff --git a/a.rs b/a.rs
+index 1..2 100644
+--- a/a.rs
++++ b/a.rs
+@@ -10,4 +20,5 @@ fn f() {
+ keep
+-gone
++new one
++new two
+ tail
+\\ No newline at end of file
+diff --git a/b.rs b/b.rs
+--- a/b.rs
++++ b/b.rs
+@@ -1 +1 @@
+-x
++y
+";
+
+    #[test]
+    fn new_file_lines_are_numbered_from_the_hunk_header() {
+        let got = number_new_lines(TWO_FILES);
+        let expected = "\
+diff --git a/a.rs b/a.rs
+index 1..2 100644
+--- a/a.rs
++++ b/a.rs
+@@ -10,4 +20,5 @@ fn f() {
+    20  keep
+       -gone
+    21 +new one
+    22 +new two
+    23  tail
+       \\ No newline at end of file
+diff --git a/b.rs b/b.rs
+--- a/b.rs
++++ b/b.rs
+@@ -1 +1 @@
+       -x
+     1 +y
+";
+        assert_eq!(got, expected);
+    }
+
+    #[test]
+    fn a_hunk_header_without_a_comma_still_gives_its_start() {
+        assert_eq!(hunk_new_start("-1 +7 @@"), Some(7));
+        assert_eq!(hunk_new_start("-3,2 +9,4 @@ ctx"), Some(9));
+        assert_eq!(hunk_new_start("garbage"), None);
+    }
+
     #[test]
     fn detects_known_lockfiles() {
         assert!(is_autogenerated("flake.lock"));

hive-forge/src/verbs/pr_reviews.rs

@@ -1,11 +1,11 @@
 //! `pr reviews <number>` — list reviews, or submit one via `--approve` /
-//! `--request-changes` / `--comment`.
+//! `--request-changes` / `--comment`, optionally with inline `--at` comments.
 
 use std::fmt::Write as _;
 
-use anyhow::{Result, bail};
+use anyhow::{Context as _, Result, bail};
 use clap::Args as ClapArgs;
-use forgejo_api::structs::CreatePullReviewOptions;
+use forgejo_api::structs::{CreatePullReviewComment, CreatePullReviewOptions};
 use serde_json::{Value, json};
 
 use crate::client::{Client, index};
@@ -33,6 +33,19 @@ pub struct Args {
     #[arg(long, short = 'm')]
     body: Option<String>,
 
+    /// An inline comment on a line of the new file: `--at path:LINE "text"`.
+    /// Repeat it to put several comments in the same review. LINE is the
+    /// new-file line number, which `pr diff --numbered` prints. Needs
+    /// --approve, --request-changes or --comment.
+    #[arg(
+        long,
+        num_args = 2,
+        value_names = ["PATH:LINE", "TEXT"],
+        action = clap::ArgAction::Append,
+        conflicts_with = "only",
+    )]
+    at: Vec<String>,
+
     /// When listing: narrow the inline comments shown (and leave out the
     /// reviews with none left). `unresolved` keeps every comment nobody has
     /// resolved yet: what a re-review has to check.
@@ -61,22 +74,65 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
     };
 
     if let Some(ev) = event {
-        submit_review(client, args.number, ev, args.body)
+        let comments = parse_inline_comments(&args.at)?;
+        submit_review(client, args.number, ev, args.body, comments)
     } else {
         if args.body.is_some() {
             bail!("--body requires one of --approve / --request-changes / --comment");
         }
+        if !args.at.is_empty() {
+            bail!("--at requires one of --approve / --request-changes / --comment");
+        }
         list_reviews(client, args.number, args.only)
     }
 }
 
-/// Submit a review event (`APPROVED` / `REQUEST_CHANGES` / `COMMENT`) and print
+/// Turn the `--at` values (flat `PATH:LINE`, `TEXT` pairs) into the inline
-/// a compact summary of the created review.
+/// comments of a review. Lines are new-file lines.
-fn submit_review(client: &Client, number: u64, event: &str, body: Option<String>) -> Result<()> {
+fn parse_inline_comments(at: &[String]) -> Result<Vec<CreatePullReviewComment>> {
+    at.chunks(2)
+        .map(|pair| {
+            let [location, text] = pair else {
+                bail!("--at takes a PATH:LINE and a TEXT");
+            };
+            let (path, line) = location
+                .rsplit_once(':')
+                .with_context(|| format!("--at {location:?}: expected PATH:LINE"))?;
+            let line: i64 =
+                line.parse().ok().filter(|l| *l > 0).with_context(|| {
+                    format!("--at {location:?}: LINE must be a positive number")
+                })?;
+            if path.is_empty() {
+                bail!("--at {location:?}: PATH is empty");
+            }
+            if text.trim().is_empty() {
+                bail!("--at {location:?}: the comment text is empty");
+            }
+            Ok(CreatePullReviewComment {
+                body: Some(text.clone()),
+                extra_lines_count: None,
+                new_position: Some(line),
+                old_position: Some(0),
+                path: Some(path.to_owned()),
+            })
+        })
+        .collect()
+}
+
+/// Submit a review event (`APPROVED` / `REQUEST_CHANGES` / `COMMENT`), with
+/// its inline `comments`, and print a compact summary of the created review.
+fn submit_review(
+    client: &Client,
+    number: u64,
+    event: &str,
+    body: Option<String>,
+    comments: Vec<CreatePullReviewComment>,
+) -> Result<()> {
     let (owner, name) = client.owner_repo()?;
+    let n_comments = comments.len();
     let payload = CreatePullReviewOptions {
         body: Some(body.unwrap_or_default()),
-        comments: None,
+        comments: (!comments.is_empty()).then_some(comments),
         commit_id: None,
         event: Some(event.to_owned()),
     };
@@ -88,6 +144,7 @@ fn submit_review(client: &Client, number: u64, event: &str, body: Option<String>
         "id": review.id,
         "state": review.state,
         "user": review.user.as_ref().and_then(|u| u.login.as_deref()),
+        "inline_comments": n_comments,
     }))
 }
 
@@ -334,6 +391,54 @@ fn list_reviews_text(client: &Client, number: u64, reviews: &[Value], only: Opti
 mod tests {
     use super::*;
 
+    fn at(pairs: &[(&str, &str)]) -> Vec<String> {
+        pairs
+            .iter()
+            .flat_map(|(loc, text)| [(*loc).to_owned(), (*text).to_owned()])
+            .collect()
+    }
+
+    #[test]
+    fn at_pairs_become_new_file_line_comments() {
+        let got = parse_inline_comments(&at(&[
+            ("src/a.rs:12", "off by one"),
+            ("docs/x:y.md:3", "colon in the path"),
+        ]))
+        .unwrap();
+        assert_eq!(got.len(), 2);
+        assert_eq!(got[0].path.as_deref(), Some("src/a.rs"));
+        assert_eq!(got[0].new_position, Some(12));
+        assert_eq!(got[0].old_position, Some(0));
+        assert_eq!(got[0].body.as_deref(), Some("off by one"));
+        assert_eq!(
+            got[1].path.as_deref(),
+            Some("docs/x:y.md"),
+            "the last colon splits"
+        );
+        assert_eq!(got[1].new_position, Some(3));
+    }
+
+    #[test]
+    fn a_malformed_at_is_refused_with_the_reason() {
+        for (pairs, needle) in [
+            (at(&[("src/a.rs", "no line")]), "expected PATH:LINE"),
+            (at(&[("src/a.rs:x", "nan")]), "positive number"),
+            (at(&[("src/a.rs:0", "zero")]), "positive number"),
+            (at(&[("src/a.rs:-4", "negative")]), "positive number"),
+            (at(&[(":5", "no path")]), "PATH is empty"),
+            (at(&[("src/a.rs:5", "  ")]), "text is empty"),
+            (vec!["src/a.rs:5".to_owned()], "PATH:LINE and a TEXT"),
+        ] {
+            let err = format!("{:#}", parse_inline_comments(&pairs).unwrap_err());
+            assert!(err.contains(needle), "{pairs:?} -> {err}");
+        }
+    }
+
+    #[test]
+    fn no_at_means_no_inline_comments() {
+        assert!(parse_inline_comments(&[]).unwrap().is_empty());
+    }
+
     fn comment(id: i64, position: u64, resolver: Option<&str>) -> Value {
         json!({
             "id": id,