//! `comments [--limit N | --tail N]` — list comments on an //! issue or PR. Replaces the curl fallback. //! //! - `--limit N` returns the first N comments (oldest-first) — an //! explicit opt-in: a plain `comments` call used to read as "I've read //! this thread" when it was really "the oldest N of it", so the //! default moved to `--tail`'s window instead (see below). //! - `--tail N` returns the *last* N comments, chronological — and with //! neither flag given, THIS is the default (N = 10): most-recent //! activity. Reads the issue's `comments` count first to compute which //! page holds the tail, then fetches only `ceil(N/50) + 1` pages — //! bounded by `N`, not thread length. //! - **The count of comments outside whatever window is shown is //! always reported** — never a silent truncation. //! - `--since ` filters to comments at or after that //! timestamp instead of a head/tail window — a cursor: feed the //! last-seen row's own `created_at` back in next time. Mutually //! exclusive with `--tail`; `--limit` clamps like `timeline`'s own //! does (see `crate::verbs::MAX_LIMIT`). //! //! Review *bodies* on PRs are always merged in too (`pulls//reviews` //! isn't the issues/comments thread), tagged `[review: STATE]`, with a //! `(N line comment(s) — see 'pr reviews')` pointer when a review has //! inline comments — this verb never inlines those, `pr reviews` does. //! //! `--json` output is an object (`{"comments": [...], "more_before": N, //! "more_after": N, "since_more": bool}`), not a bare array, so a //! script can read the truncation info too. `--show-reactions` adds a //! per-comment reaction summary — see that flag's own doc for the cost. use anyhow::Result; use clap::Args as ClapArgs; use forgejo_api::structs::IssueGetCommentsQuery; use serde_json::{Value, json}; use time::OffsetDateTime; use crate::client::{Client, index}; use crate::notify; use crate::verbs::{ MAX_LIMIT, PAGE_SIZE, clamp_limit, comment_reactions, parse_rfc3339, print_json, reaction_summary, rfc3339, }; #[derive(ClapArgs)] pub struct Args { /// Issue or PR number. pub(crate) number: u64, /// Number of comments from the start of the thread (oldest-first), /// or (with `--since`) the most this call returns — capped at /// [`crate::verbs::MAX_LIMIT`] in the latter case. An explicit /// opt-in: with neither this nor `--tail` given, the default is the /// newest 10 (see `--tail`). Mutually exclusive with `--tail`. #[arg(long, conflicts_with = "tail")] limit: Option, /// Return the last `N` comments (chronological) — the most recent /// activity. This is the default (`N` = 10) when neither `--limit` /// nor `--tail` is given. Mutually exclusive with `--limit`/`--since`. #[arg(long, conflicts_with = "since")] tail: Option, /// Only show comments at or after this RFC3339 timestamp (same /// format this verb's own output prints). Mutually exclusive with /// `--tail`. #[arg(long)] since: Option, /// Fetch + display each shown comment's reaction summary. Costs one /// extra request per comment shown — opt-in, not the default. #[arg(long)] show_reactions: bool, } pub fn run(client: &Client, args: Args) -> Result<()> { let repo = client.repo()?; // Truncation info: what sits outside the window we're about to show. // A `--tail`/default window has nothing after it (it ends at the // thread's current end); a `--limit` window has nothing before it (it // starts at the thread's beginning); a `--since` window is a boolean // "there's more" (no total exists for a since-filtered query) rather // than an exact count. `more_before`/`more_after` are derived from // `thread.len()` rather than the requested `n`/`limit` so a // shrunk-under-us thread (comments deleted mid-fetch) still reports // accurately instead of the number we merely asked for. let (thread, more_before, more_after, since_more, since_clamped) = if let Some(since_str) = &args.since { let since = parse_rfc3339(since_str)?; let (limit, clamped) = clamp_limit(args.limit.unwrap_or(10)); let (thread, more) = fetch_since(client, args.number, since, limit)?; (thread, 0, 0, more, clamped) } else if let Some(limit) = args.limit { let total = fetch_total(client, args.number)?; let thread = fetch_head(client, args.number, limit)?; let more_after = total.saturating_sub(thread.len()); (thread, 0, more_after, false, false) } else { // Default (no `--limit`/`--tail`/`--since`): same window as // an explicit `--tail 10` — the newest activity, not the // oldest. `args.tail` still wins if the agent passed a // different count explicitly. let n = args.tail.unwrap_or(10); let total = fetch_total(client, args.number)?; let thread = fetch_tail(client, args.number, n, total)?; let more_before = total.saturating_sub(thread.len()); (thread, more_before, 0, false, false) }; // Merge in PR review bodies (empty for issues — degrades to a // no-op) so review feedback isn't silently dropped. let mut comments = merge_chronological(thread, fetch_review_bodies(client, args.number)); // Reading the thread clears its unread notification so the // read-before-comment guard (in `comment`) lets a reply through. notify::mark_read_best_effort(client, repo, args.number); let (owner, name) = client.owner_repo()?; attach_reactions(client, owner, name, &mut comments, args.show_reactions); if client.json_mode() { let trimmed: Vec = comments .iter() .map(|c| { json!({ "id": c.get("id"), "user": c.get("user").and_then(|u| u.get("login")), "created_at": c.get("created_at"), "updated_at": c.get("updated_at"), "body": c.get("body"), "url": c.get("html_url"), "kind": c.get("kind").and_then(Value::as_str).unwrap_or("comment"), "state": c.get("state"), "comments_count": c.get("comments_count"), "attachments": attachment_json_from_value(c), "reactions": c.get("reactions").cloned().unwrap_or_else(|| json!([])), }) }) .collect(); print_json(&json!({ "comments": trimmed, "more_before": more_before, "more_after": more_after, "since_more": since_more, "since_limit_clamped": since_clamped, })) } else { for c in &comments { let user = c .get("user") .and_then(|u| u.get("login")) .and_then(Value::as_str) .unwrap_or("?"); let ts = c.get("created_at").and_then(Value::as_str).unwrap_or("?"); let body = c.get("body").and_then(Value::as_str).unwrap_or(""); if c.get("kind").and_then(Value::as_str) == Some("review") { let state = c.get("state").and_then(Value::as_str).unwrap_or("?"); let n = c.get("comments_count").and_then(Value::as_i64).unwrap_or(0); if n > 0 { println!( "**{user} @ {ts}** [review: {state}] ({n} line comment(s) — see `hive-forge pr reviews {}`): {body}", args.number ); } else { println!("**{user} @ {ts}** [review: {state}]: {body}"); } } else { println!("**{user} @ {ts}**: {body}"); } for line in attachment_lines_from_value(c) { println!("{line}"); } if let Some(summary) = c .get("reactions") .and_then(Value::as_array) .and_then(|r| reaction_summary(r)) { println!("[reactions: {summary}]"); } println!(); } if since_clamped { println!( "(--limit {} is above the {MAX_LIMIT} cap --since can reliably detect truncation at — clamped)", args.limit.unwrap_or(10) ); } if since_more { println!( "(more comments since this timestamp not shown — raise --limit or bump --since; exact count not available)" ); } if let Some(note) = truncation_note(more_before, more_after) { println!("{note}"); } Ok(()) } } /// A one-line truncation note when the shown window doesn't cover the /// whole thread — never a silent gap. `None` when both are zero (the /// whole thread rode the wire). Returns a `String` rather than printing /// directly so the three cases are unit-testable. fn truncation_note(more_before: usize, more_after: usize) -> Option { match (more_before > 0, more_after > 0) { (false, false) => None, (true, false) => Some(format!( "({more_before} earlier comment(s) not shown — use --limit to see the start of the thread)" )), (false, true) => Some(format!( "({more_after} more comment(s) not shown — use --tail N to see the latest instead)" )), (true, true) => Some(format!( "({more_before} earlier + {more_after} later comment(s) not shown — use --tail N)" )), } } /// Fetch + attach each shown comment's reaction list under a /// `"reactions"` key, mutating in place so both the `--json` and /// pretty-print branches see the same fetch. No-op when `show_reactions` /// is false (the default) — the whole point of gating this behind a /// flag is that a plain `comments` call costs nothing extra. Review /// entries are skipped (synthesized, never have real reactions); a /// per-comment fetch failure is swallowed rather than failing the whole /// listing over one comment. fn attach_reactions( client: &Client, owner: &str, name: &str, comments: &mut [Value], show_reactions: bool, ) { if !show_reactions { return; } for c in comments.iter_mut() { if c.get("kind").and_then(Value::as_str) == Some("review") { continue; } let Some(id) = c.get("id").and_then(Value::as_u64) else { continue; }; if let Ok(reactions) = comment_reactions(client, owner, name, id) && let Some(obj) = c.as_object_mut() { obj.insert("reactions".to_owned(), json!(reactions)); } } } /// Attachment display lines for a single comment's already-serialized /// `assets` field — the shape this module's merge/render pipeline works /// on (comments arrive as `Value`s, not the typed `Comment` struct, once /// merged with synthesized review entries). Review entries carry no /// `assets` field at all, so they naturally yield nothing here. Mirrors /// [`crate::verbs::attachment_line`]'s `[file: ] ` format. fn attachment_lines_from_value(c: &Value) -> Vec { c.get("assets") .and_then(Value::as_array) .into_iter() .flatten() .filter_map(|a| { let name = a.get("name").and_then(Value::as_str).unwrap_or("?"); let url = a.get("browser_download_url").and_then(Value::as_str)?; Some(format!("[file: {name}] {url}")) }) .collect() } /// JSON form of [`attachment_lines_from_value`]'s data, for `--json` /// output — same `{"name", "url"}` shape [`crate::verbs::attachment_json`] /// produces from the typed struct. fn attachment_json_from_value(c: &Value) -> Vec { c.get("assets") .and_then(Value::as_array) .into_iter() .flatten() .map(|a| { json!({ "name": a.get("name"), "url": a.get("browser_download_url"), }) }) .collect() } /// Total comment count on an issue/PR, straight off the issue object. /// Used both to plan `--tail`'s pagination and to report how many /// comments sit outside whatever window ends up shown. fn fetch_total(client: &Client, number: u64) -> Result { let (owner, name) = client.owner_repo()?; let issue = client .api() .issue_get_issue(owner, name, index(number)?) .send()?; Ok(issue .comments .and_then(|c| usize::try_from(c).ok()) .unwrap_or(0)) } /// Serialize a typed comment page back to the JSON `Value` shape the /// merge + render pipeline works on (the structs serialize to the API /// wire shape, so downstream field access is unchanged). fn to_values(items: Vec) -> Result> { items .into_iter() .map(|c| serde_json::to_value(&c).map_err(Into::into)) .collect() } /// Fetch a PR's review *bodies* and normalise them to the comment /// shape so they merge alongside issue-thread comments. /// /// Review summaries live in the `pulls//reviews` object, not the /// issues/comments thread, so the plain comment listing misses them /// — the review-body gap this fixes. Best-effort: returns empty on any /// error — notably when `number` is an issue (no reviews endpoint) — /// so callers degrade gracefully. Skips PENDING reviews (not yet /// visible to others) and reviews with neither a body nor line /// comments (a bare approval adds nothing to the thread). Deliberately /// does NOT inline the per-line comments themselves: merging them into /// this glance-first thread would spam anyone who just wants status. /// `comments_count` carries a pointer to `pr reviews ` instead, /// which prints the full per-line detail — see that verb /// (`pr_reviews.rs`) for the exhaustive form. `created_at` /// is synthesised from the review's `submitted_at` so the chronological /// merge sorts uniformly; `kind:"review"` + the review `state` tag the /// entry for display. fn fetch_review_bodies(client: &Client, number: u64) -> Vec { let Ok((owner, name)) = client.owner_repo() else { return Vec::new(); }; let Ok(idx) = index(number) else { return Vec::new(); }; let reviews = client .api() .repo_list_pull_reviews(owner, name, idx) .send() .map(|(_, reviews)| reviews) .unwrap_or_default(); reviews .into_iter() .filter_map(|r| { let state = r.state.as_deref().unwrap_or("").to_owned(); if state == "PENDING" { return None; } let comments_count = r.comments_count.unwrap_or(0); if r.body.as_deref().unwrap_or("").trim().is_empty() && comments_count == 0 { return None; } let submitted = rfc3339(r.submitted_at); Some(json!({ "id": r.id, "user": r.user, "created_at": submitted, "updated_at": submitted, "body": r.body, "html_url": r.html_url, "kind": "review", "state": state, "comments_count": comments_count, })) }) .collect() } /// Merge issue-thread comments with review bodies and sort the /// combined list chronologically by `created_at`. ISO-8601 timestamps /// sort lexicographically in time order, so a plain string compare is /// correct; the sort is stable, so same-timestamp entries keep fetch /// order. fn merge_chronological(mut items: Vec, reviews: Vec) -> Vec { items.extend(reviews); items.sort_by(|a, b| { let ka = a.get("created_at").and_then(Value::as_str).unwrap_or(""); let kb = b.get("created_at").and_then(Value::as_str).unwrap_or(""); ka.cmp(kb) }); items } /// Fetch the first page's worth of comments (existing behaviour). fn fetch_head(client: &Client, number: u64, limit: u64) -> Result> { let (owner, name) = client.owner_repo()?; let (_, comments) = client .api() .issue_get_comments( owner, name, index(number)?, IssueGetCommentsQuery::default(), ) .page_size(u32::try_from(limit).unwrap_or(u32::MAX)) .send()?; to_values(comments) } /// Fetch the last `n` comments on an issue/PR in chronological order. /// `total` (the thread's comment count, from [`fetch_total`]) drives the /// pagination plan and is the caller's, not fetched again here. /// /// Forgejo orders `/issues//comments` oldest-first and has no /// `direction=desc` knob, so naive "page everything and slice" pages /// from the WRONG end on long threads — the first 1000 comments /// instead of the last `n`. Fix: use `total` to know how many exist, /// then start paginating from the page that contains item /// `total - n`. Work is bounded by `ceil(n/50) + 1` page fetches, /// regardless of thread length. fn fetch_tail(client: &Client, number: u64, n: usize, total: usize) -> Result> { if n == 0 || total == 0 { return Ok(Vec::new()); } let (owner, name) = client.owner_repo()?; let idx = index(number)?; // Cap `n` at the actual total so the math below stays in range // when the caller asks for more comments than exist. let n = n.min(total); let page_size = usize::try_from(PAGE_SIZE).unwrap_or(usize::MAX); // 0-based index of the first comment we want; integer-divide to // get the 1-based page that contains it. let start_idx = total - n; let start_page = (start_idx / page_size) + 1; let last_page = (total - 1) / page_size + 1; let mut merged: Vec = Vec::with_capacity(n + page_size); for page in start_page..=last_page { let (_, arr) = client .api() .issue_get_comments(owner, name, idx, IssueGetCommentsQuery::default()) .page(u32::try_from(page).unwrap_or(u32::MAX)) .page_size(u32::try_from(PAGE_SIZE).unwrap_or(u32::MAX)) .send()?; if arr.is_empty() { // Page came back empty — either we miscounted (comments // deleted between the issue GET and now) or upstream's // playing tricks. Stop rather than spin. break; } merged.extend(to_values(arr)?); } // The first fetched page contains items from `start_page` × 50 // back; we overshoot by `start_idx % 50` items. Slice the tail // to exactly `n` (or fewer if the count shrank under us). let overshoot = merged.len().saturating_sub(n); Ok(merged.into_iter().skip(overshoot).collect()) } /// Fetch comments at or after `since`, capped at `limit` (already /// clamped to [`crate::verbs::MAX_LIMIT`] by the caller). No total exists /// for a since-filtered query — Forgejo's API reports none — so this /// uses the same over-fetch-by-one trick `timeline`'s `--limit` does: /// ask for `limit + 1`, and a full extra row means there's more. Returns /// `(comments, more)`, `more` a boolean rather than an exact count. fn fetch_since( client: &Client, number: u64, since: OffsetDateTime, limit: u64, ) -> Result<(Vec, bool)> { let (owner, name) = client.owner_repo()?; let query = IssueGetCommentsQuery { since: Some(since), ..Default::default() }; let fetch_limit = limit.saturating_add(1); let (_, comments) = client .api() .issue_get_comments(owner, name, index(number)?, query) .page_size(u32::try_from(fetch_limit).unwrap_or(u32::MAX)) .send()?; let mut values = to_values(comments)?; let limit = usize::try_from(limit).unwrap_or(usize::MAX); let more = values.len() > limit; values.truncate(limit); Ok((values, more)) } #[cfg(test)] mod tests { use super::*; /// Pure helper mirroring the page-arithmetic in `fetch_tail`: /// given a total comment count + requested tail size, return /// the (`start_page`, `last_page`) pair the network loop would /// walk. Lets us pin the pagination plan — the part that's /// easy to off-by-one — without touching the network. fn tail_plan(total: usize, n: usize) -> (usize, usize) { let n = n.min(total); let page_size = usize::try_from(PAGE_SIZE).unwrap_or(usize::MAX); let start_idx = total - n; let start_page = (start_idx / page_size) + 1; let last_page = (total - 1) / page_size + 1; (start_page, last_page) } #[test] fn tail_plan_small_thread() { // 5 total, tail 2 → page 1 covers everything; trim happens // via the merged.len() - n overshoot calculation, not pages. assert_eq!(tail_plan(5, 2), (1, 1)); } #[test] fn tail_plan_exact_page_boundary() { // 50 total, tail 3 → all on page 1 (items 1..50). assert_eq!(tail_plan(50, 3), (1, 1)); } #[test] fn tail_plan_crosses_page_boundary() { // 51 total, tail 3 → start_idx=48 lives on page 1, item 51 // lives on page 2; fetch both. assert_eq!(tail_plan(51, 3), (1, 2)); } #[test] fn tail_plan_large_thread_bounded_pages() { // 5000 total, tail 3 → start_idx=4997 lives on page 100, // last_page=100. ONE page fetch on a 5000-comment thread — // the whole point of swapping to count-then-page (vs the // old "page everything, then slice the wrong end"). assert_eq!(tail_plan(5000, 3), (100, 100)); } #[test] fn tail_plan_large_thread_spans_two_pages() { // 5000 total, tail 60 → start_idx=4940 on page 99, item 5000 // on page 100. Two fetches even for n > PAGE_SIZE. assert_eq!(tail_plan(5000, 60), (99, 100)); } #[test] fn tail_plan_n_exceeds_total() { // 5 total, tail 100 → cap n at total; same plan as the // small-thread case above. assert_eq!(tail_plan(5, 100), (1, 1)); } #[test] fn merge_interleaves_reviews_by_timestamp() { // A review submitted between two comments must land between // them, not appended at the end — that's the whole fix. let comments = vec![ json!({"created_at": "2026-06-29T01:00:00Z", "body": "c1"}), json!({"created_at": "2026-06-29T01:20:00Z", "body": "c2"}), ]; let reviews = vec![json!({"created_at": "2026-06-29T01:10:00Z", "body": "r1", "kind": "review"})]; let merged = merge_chronological(comments, reviews); let bodies: Vec<&str> = merged .iter() .map(|v| v.get("body").and_then(Value::as_str).unwrap()) .collect(); assert_eq!(bodies, vec!["c1", "r1", "c2"]); } #[test] fn truncation_note_silent_when_nothing_hidden() { assert_eq!(truncation_note(0, 0), None); } #[test] fn truncation_note_after_only_points_at_tail() { let note = truncation_note(0, 40).unwrap(); assert!(note.contains("40"), "{note}"); assert!(note.contains("--tail"), "{note}"); } #[test] fn truncation_note_before_only_points_at_limit() { // Regression: this used to (wrongly) suggest `--tail` — the flag // that produced the very window you're already looking at when // more_before is nonzero. `--limit` is what actually reaches the // start of the thread. let note = truncation_note(12, 0).unwrap(); assert!(note.contains("12"), "{note}"); assert!(note.contains("earlier"), "{note}"); assert!(note.contains("--limit"), "{note}"); } #[test] fn truncation_note_both_sides_mentions_both_counts() { let note = truncation_note(3, 7).unwrap(); assert!(note.contains('3') && note.contains('7'), "{note}"); } #[test] fn merge_with_no_reviews_is_identity() { // Issues have no reviews → fetch_review_bodies returns empty → // the merge must leave the comment thread untouched. let comments = vec![ json!({"created_at": "2026-06-29T01:00:00Z", "body": "c1"}), json!({"created_at": "2026-06-29T01:20:00Z", "body": "c2"}), ]; let merged = merge_chronological(comments, vec![]); let bodies: Vec<&str> = merged .iter() .map(|v| v.get("body").and_then(Value::as_str).unwrap()) .collect(); assert_eq!(bodies, vec!["c1", "c2"]); } }