hive-forge: count-then-page strategy for comments --tail (mara feedback #770)

read the issue's `comments` count first, compute which page contains the
tail, fetch only `ceil(n/50) + 1` pages. drops the TAIL_MAX_PAGES cap
entirely — it was paging from the WRONG end (first 1000 comments instead
of the last n) on long threads, defeating the whole purpose of --tail.
work is now bounded by n, not by thread length.

hive-forge/src/verbs/comments.rs

@@ -6,8 +6,11 @@
 //! - `--limit N` (default 50, Forgejo's cap) returns the first N
 //!   comments — same shape this verb has always had.
 //! - `--tail N` returns the *last* N comments in chronological
-//!   order. Internally pages through every comment on the issue,
-//!   then slices the tail. Use this for "what was the conclusion
+//!   order. Reads the issue's `comments` count first to compute
+//!   which page contains the tail, then fetches only `ceil(N/50) +
+//!   1` pages. No upstream cap — the work is bounded by `N`, not
+//!   by the thread's length, so it stays cheap even on threads with
+//!   thousands of comments. Use this for "what was the conclusion
 //!   on this long thread?" without scrolling through the whole
 //!   history.
 //!
@@ -20,13 +23,10 @@ use serde_json::{Value, json};
 use crate::client::Client;
 use crate::verbs::print_json;
 
-/// Hard cap on `--tail` pagination so a confused caller can't
-/// accidentally paginate forever on a runaway-comment thread.
-/// 20 pages × Forgejo's 50-per-page cap = 1000 comments which
-/// covers every issue in the hyperhive repo today with headroom.
-/// Tune up if a future thread breaches it (the truncated tail
-/// will surface as a smaller-than-expected slice).
-const TAIL_MAX_PAGES: u32 = 20;
+/// Forgejo's per-page comment cap. The API caps `limit` at 50 even
+/// if a higher value is requested; pin it explicitly so the math
+/// downstream doesn't depend on a hidden default.
+const PAGE_SIZE: u64 = 50;
 
 #[derive(ClapArgs)]
 pub struct Args {
@@ -88,35 +88,115 @@ fn fetch_head(client: &Client, repo: &str, number: u64, limit: u64) -> Result<Ve
     Ok(v.as_array().cloned().unwrap_or_default())
 }
 
-/// Fetch every comment via paginated GET, then return the last `n`
-/// items in chronological order. Forgejo orders
-/// `/issues/<n>/comments` oldest-first so the trailing slice maps
-/// directly to "most recent N".
+/// Fetch the last `n` comments on an issue/PR in chronological order.
 ///
-/// When the fetch saturates `TAIL_MAX_PAGES` (i.e. all 20 pages came
-/// back full at 50/page) we can't tell whether more comments exist
-/// upstream. Surface a stderr warning so the caller knows the
-/// "tail" is anchored to the first 1000 comments, not necessarily
-/// the actual newest of the thread. False positive: a thread with
-/// EXACTLY 1000 comments will warn even though the slice IS the
-/// real tail — acceptable noise given how rare 1000-comment threads
-/// are (closes argus 🟡 on PR #770).
+/// Forgejo orders `/issues/<n>/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: read the issue's `comments` count
+/// first 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, repo: &str, number: u64, n: usize) -> Result<Vec<Value>> {
-    let all = client.get_json_all(
-        &format!("/repos/{repo}/issues/{number}/comments?limit=50"),
-        TAIL_MAX_PAGES,
-    )?;
-    let total = all.len();
-    if total >= TAIL_MAX_PAGES as usize * 50 {
-        eprintln!(
-            "warning: hit pagination cap ({TAIL_MAX_PAGES} pages × 50 = \
-             {total} comments); --tail slice is anchored to the first \
-             {total} comments and may not include the actual newest of \
-             the thread."
-        );
+    if n == 0 {
+        return Ok(Vec::new());
     }
-    if total <= n {
-        return Ok(all);
+    let issue = client.get_json(&format!("/repos/{repo}/issues/{number}"))?;
+    let total = issue
+        .get("comments")
+        .and_then(Value::as_u64)
+        .unwrap_or(0) as usize;
+    if total == 0 {
+        return Ok(Vec::new());
+    }
+    // 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 = PAGE_SIZE as usize;
+    // 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<Value> = Vec::with_capacity(n + page_size);
+    for page in start_page..=last_page {
+        let v = client.get_json(&format!(
+            "/repos/{repo}/issues/{number}/comments?limit={PAGE_SIZE}&page={page}"
+        ))?;
+        let arr = v.as_array().cloned().unwrap_or_default();
+        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(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())
+}
+
+#[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 = PAGE_SIZE as usize;
+        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));
     }
-    Ok(all.into_iter().skip(total - n).collect())
 }