vinzenz/hyperhive
vinzenz/hyperhive
Watch
0
0
Fork
You've already forked hyperhive
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

hive-forge: add --since cursor paging to comments and timeline

Browse source
This commit is contained in:
damocles 2026-08-03 20:39:39 +02:00 • committed by mara
commit 4cf647aa70
3 changed files with 159 additions and 70 deletions
Download patch file Download diff file Expand all files Collapse all files

120
hive-forge/src/verbs/comments.rs
View file

@ -12,62 +12,81 @@
//! comments.
//! - **The count of comments outside whatever window is shown is
//! always reported** — never a silent truncation.
//! - `--since <RFC3339>` 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 to fetch only
//! what's new. Mutually exclusive with `--tail`. No total exists for
//! a since-filtered query, so this uses the same over-fetch-by-one
//! trick `timeline`'s `--limit` does, and `--limit` is clamped the
//! same way (see `crate::verbs::MAX_LIMIT`).
//!
//! Review *bodies* on PRs are always merged in too (`pulls/<n>/reviews`
//! isn't the issues/comments thread, so a plain listing used to miss
//! them), tagged `[review: STATE]`.
//!
//! `--json` output is an object (`{"comments": [...], "more_before": N,
//! "more_after": N}`), not a bare array, so a script can read the
//! truncation counts too.
//! "more_after": N, "since_more": bool}`), not a bare array, so a
//! script can read the truncation info too.
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::{print_json, rfc3339};
/// 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: usize = 50;
use crate::verbs::{MAX_LIMIT, PAGE_SIZE, clamp_limit, parse_rfc3339, print_json, rfc3339};
#[derive(ClapArgs)]
pub struct Args {
/// Issue or PR number.
pub(crate) number: u64,
/// Number of comments from the start of the thread (max 50).
/// Mutually exclusive with `--tail`.
/// Number of comments from the start of the thread, or (with
/// `--since`) the most this call returns — capped at
/// [`crate::verbs::MAX_LIMIT`] in the latter case. Mutually
/// exclusive with `--tail`.
#[arg(long, default_value_t = 10, conflicts_with = "tail")]
limit: u64,
/// Return the last `N` comments (chronological). Mutually exclusive
/// with `--limit`.
#[arg(long)]
/// with `--limit`/`--since`.
#[arg(long, conflicts_with = "since")]
tail: Option<usize>,
/// 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<String>,
}
pub fn run(client: &Client, args: Args) -> Result<()> {
let repo = client.repo();
let total = fetch_total(client, args.number)?;
// Truncation counts: how many comments sit outside the window we're
// about to show, on each side. A `--tail` window has nothing after it
// (it ends at the thread's current end); a `--limit`/default window
// has nothing before it (it starts at the thread's beginning). Derived
// from `thread.len()` rather than the requested `n`/`limit` so a
// Truncation info: what sits outside the window we're about to show.
// A `--tail` window has nothing after it (it ends at the thread's
// current end); a `--limit`/default 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) = if let Some(n) = args.tail {
let thread = fetch_tail(client, args.number, n, total)?;
let more_before = total.saturating_sub(thread.len());
(thread, more_before, 0)
} else {
let thread = fetch_head(client, args.number, args.limit)?;
let more_after = total.saturating_sub(thread.len());
(thread, 0, more_after)
};
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);
let (thread, more) = fetch_since(client, args.number, since, limit)?;
(thread, 0, 0, more, clamped)
} else if let Some(n) = args.tail {
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)
} else {
let total = fetch_total(client, args.number)?;
let thread = fetch_head(client, args.number, args.limit)?;
let more_after = total.saturating_sub(thread.len());
(thread, 0, more_after, false, false)
};
// Merge in PR review bodies (empty for issues — degrades to a
// no-op) so review feedback isn't silently dropped.
let comments = merge_chronological(thread, fetch_review_bodies(client, args.number));
@ -94,6 +113,8 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
"comments": trimmed,
"more_before": more_before,
"more_after": more_after,
"since_more": since_more,
"since_limit_clamped": since_clamped,
}))
} else {
for c in &comments {
@ -112,6 +133,17 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
}
println!();
}
if since_clamped {
println!(
"(--limit {} is above the {MAX_LIMIT} cap --since can reliably detect truncation at — clamped)",
args.limit
);
}
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}");
}
@ -265,7 +297,7 @@ fn fetch_tail(client: &Client, number: u64, n: usize, total: usize) -> Result<Ve
// 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;
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;
@ -294,6 +326,36 @@ fn fetch_tail(client: &Client, number: u64, n: usize, total: usize) -> Result<Ve
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<Value>, 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::*;
@ -305,7 +367,7 @@ mod tests {
/// 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;
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;

50
hive-forge/src/verbs/mod.rs
View file

@ -69,6 +69,56 @@ pub(crate) fn rfc3339(ts: Option<OffsetDateTime>) -> Option<String> {
ts.and_then(|t| t.format(&Rfc3339).ok())
}
/// Parse a `--since`/`--before` CLI argument as RFC 3339 — the inverse of
/// [`rfc3339`], so a value copied straight from this tool's own output
/// (every row prints its `created_at` in this exact shape) round-trips
/// without reformatting. A bad value gets a message naming what was
/// typed, not a bare parser error.
pub(crate) fn parse_rfc3339(s: &str) -> Result<OffsetDateTime> {
OffsetDateTime::parse(s, &Rfc3339)
.map_err(|e| anyhow::anyhow!("`{s}` isn't a valid RFC 3339 timestamp: {e}"))
}
/// Forgejo's per-page cap, shared by every listing verb that over-fetches
/// by one to detect truncation without an exact total (`timeline`'s
/// `--limit`, `comments`' `--since`). The API silently clamps a requested
/// page size to this value, so it's pinned explicitly rather than left as
/// a hidden default downstream math could drift out of sync with.
pub(crate) const PAGE_SIZE: u64 = 50;
/// The highest `--limit` an over-fetch-by-one truncation check
/// (`fetch_limit = limit + 1`) can still detect: `PAGE_SIZE - 1`. At
/// `limit == PAGE_SIZE` the `+1` request silently clamps to `PAGE_SIZE`
/// server-side and the truncation check goes blind exactly when there's
/// the most data to miss.
pub(crate) const MAX_LIMIT: u64 = PAGE_SIZE - 1;
/// Cap `requested` at [`MAX_LIMIT`], reporting whether it had to. Pure so
/// the boundary math is unit-testable without a network call.
pub(crate) fn clamp_limit(requested: u64) -> (u64, bool) {
let limit = requested.min(MAX_LIMIT);
(limit, limit < requested)
}
#[cfg(test)]
mod page_limit_tests {
use super::{MAX_LIMIT, PAGE_SIZE, clamp_limit};
#[test]
fn clamp_limit_passes_small_requests_through() {
assert_eq!(clamp_limit(10), (10, false));
assert_eq!(clamp_limit(MAX_LIMIT), (MAX_LIMIT, false));
}
#[test]
fn clamp_limit_caps_requests_above_the_boundary() {
// Regression: `limit + 1` must never exceed Forgejo's PAGE_SIZE,
// or the over-fetch-by-one truncation check goes silently blind.
assert_eq!(clamp_limit(PAGE_SIZE), (MAX_LIMIT, true));
assert_eq!(clamp_limit(1000), (MAX_LIMIT, true));
}
}
/// Issue-vs-PR kind, for the `pr <verb>` / `issue <verb>` sub-command
/// validation.
#[derive(Clone, Copy)]

59
hive-forge/src/verbs/timeline.rs
View file

@ -18,10 +18,14 @@
//! against — so this over-fetches by one (`limit + 1`) and reports
//! "there's more" without saying how much.
//!
//! `--limit` is capped at [`MAX_LIMIT`] (`PAGE_SIZE - 1`): the
//! `--limit` is capped at [`crate::verbs::MAX_LIMIT`]: the
//! over-fetch-by-one trick needs `limit + 1` to fit inside Forgejo's
//! hard 50-per-page cap, or the response silently clamps to 50 and the
//! hard per-page cap, or the response silently clamps and the
//! truncation check can never fire even when there genuinely is more.
//!
//! `--since <RFC3339>` filters to events at or after that timestamp —
//! a cursor: feed the last-seen row's own `created_at` back in next
//! time instead of re-running with a higher `--limit`.
use anyhow::Result;
use clap::Args as ClapArgs;
@ -29,17 +33,7 @@ use forgejo_api::structs::IssueGetCommentsAndTimelineQuery;
use serde_json::{Value, json};
use crate::client::{Client, index};
use crate::verbs::print_json;
/// Forgejo's per-page cap on the timeline endpoint — same hard ceiling
/// `comments.rs`'s `PAGE_SIZE` documents for `/comments`.
const PAGE_SIZE: u64 = 50;
/// Largest `--limit` the over-fetch-by-one trick can still detect
/// truncation at: `limit + 1` must stay within [`PAGE_SIZE`], or the
/// response silently clamps to `PAGE_SIZE` and `more` reads `false` even
/// when there's genuinely more.
const MAX_LIMIT: u64 = PAGE_SIZE - 1;
use crate::verbs::{MAX_LIMIT, clamp_limit, parse_rfc3339, print_json};
#[derive(ClapArgs)]
pub struct Args {
@ -49,14 +43,11 @@ pub struct Args {
/// comment for why). Default kept small on purpose.
#[arg(long, default_value_t = 10)]
limit: u64,
}
/// Effective `--limit` plus whether the request was clamped to
/// [`MAX_LIMIT`]. Pure so the boundary math is unit-testable without a
/// network call.
fn clamp_limit(requested: u64) -> (u64, bool) {
let limit = requested.min(MAX_LIMIT);
(limit, limit < requested)
/// Only show events at or after this RFC3339 timestamp (same format
/// this verb's own output prints) — pass back the last-seen row's
/// `created_at` to fetch only what's new.
#[arg(long)]
since: Option<String>,
}
pub fn run(client: &Client, args: Args) -> Result<()> {
@ -65,17 +56,17 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
// stay within Forgejo's per-page cap or the truncation check goes
// silently blind right at the boundary — see MAX_LIMIT's doc comment.
let (limit, clamped) = clamp_limit(args.limit);
let since = args.since.as_deref().map(parse_rfc3339).transpose()?;
let query = IssueGetCommentsAndTimelineQuery {
since,
before: None,
};
// Over-fetch by one to detect truncation without an exact total —
// see the module doc comment for why there's no count query here.
let fetch_limit = limit.saturating_add(1);
let (_, events) = client
.api()
.issue_get_comments_and_timeline(
owner,
name,
index(args.number)?,
IssueGetCommentsAndTimelineQuery::default(),
)
.issue_get_comments_and_timeline(owner, name, index(args.number)?, query)
.page_size(u32::try_from(fetch_limit).unwrap_or(u32::MAX))
.send()?;
let limit = usize::try_from(limit).unwrap_or(usize::MAX);
@ -288,20 +279,6 @@ mod tests {
//! duplicating the logic" — addressed.
use super::*;
#[test]
fn clamp_limit_passes_small_requests_through() {
assert_eq!(clamp_limit(10), (10, false));
assert_eq!(clamp_limit(MAX_LIMIT), (MAX_LIMIT, false));
}
#[test]
fn clamp_limit_caps_requests_above_the_boundary() {
// Regression: `limit + 1` must never exceed Forgejo's PAGE_SIZE,
// or the over-fetch-by-one truncation check goes silently blind.
assert_eq!(clamp_limit(PAGE_SIZE), (MAX_LIMIT, true));
assert_eq!(clamp_limit(1000), (MAX_LIMIT, true));
}
#[test]
fn comment_renders_body_inline() {
let ev = serde_json::json!({