Watch
0
0
Fork
You've already forked hyperhive
0
hyperhive/hive-forge/src/verbs/timeline.rs
atlas 1d56a74aad hive-forge: fetch issue comments in one request; the endpoint has no paging
The forge's issue-comments endpoint takes no page or limit and always
returns the whole thread. #5055 wrapped view's fetch in all_pages, which
never saw a short page on a thread of 50+ comments and fetched the same
list 50 times: `issue view 3782` printed 3000 comments instead of 60.
`comments --limit K` sent K as a page size the forge ignored, so it
returned the whole thread.

view and pr status's last_comment now send one request. comments fetches
the thread once and cuts the --limit / --tail window client-side; the
X-Total-Count probe and the computed-page tail walk are gone, and the
count of comments outside the window is the length of the list.
--since is unchanged.

tests/comments_no_paging.rs runs the binary against a loopback mock
forge whose comments route ignores page and limit, as Forgejo's does.
2026-10-09 21:19:47 +02:00

702 lines
27 KiB
Rust

//! `timeline <number> [--limit N | --tail N]` — list timeline events
//! (comments + activity: labels, assignees, close/reopen, pushes, …) on
//! an issue or PR. Fills the gap where agents kept falling back to curl
//! for "who closed this?" / "when was this labelled?" archaeology.
//! Composes naturally with `view <n>` / `comments <n>` — separate verb
//! keeps the existing shapes stable.
//!
//! Same three windows as `comments`, same reasoning: `--limit N` returns
//! the first N events (oldest-first) — an explicit opt-in now, since a
//! plain `timeline` call reading as "I've read this thread" when it's
//! really "the oldest N of it" was the same silent trap `comments` had.
//! `--tail N` returns the last N (chronological) — and with neither flag
//! given, THIS is the default (N = 10): most-recent activity first, the
//! useful default for "what just happened here". `--since <RFC3339>`
//! filters to events 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
//! before (see [`crate::verbs::MAX_LIMIT`]) since a since-filtered query
//! still has no total to page against precisely.
//!
//! **The count of events outside whatever window is shown is always
//! reported** — `--tail`/default read the real total off this endpoint's
//! own `X-Total-Count` header (previously discarded; the "no total
//! exists" premise this module's docs used to state was false against
//! what `forgejo-api` actually returns), so unlike the old `--limit`-only
//! shape this is now an exact count, not a boolean "more" flag.
use anyhow::{Context, Result};
use clap::Args as ClapArgs;
use forgejo_api::structs::{IssueGetCommentsAndTimelineQuery, TimelineComment};
use serde_json::{Value, json};
use time::format_description::well_known::Rfc3339;
use crate::client::{Client, index};
use crate::verbs::{MAX_LIMIT, NullableVec, PAGE_SIZE, clamp_limit, parse_rfc3339, print_json};
#[derive(ClapArgs)]
pub struct Args {
/// Issue or PR number.
pub(crate) number: u64,
/// Return the first `N` events (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<u64>,
/// Return the last `N` events (chronological) — the most recent
/// activity. Defaults to `N` = 10 when you pass neither `--limit`
/// nor `--tail`. Mutually exclusive with `--limit`/`--since`.
#[arg(long, conflicts_with = "since")]
tail: Option<usize>,
/// 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<()> {
// Truncation info, same shape as `comments`: a `--tail`/default
// window has nothing after it; a `--limit` window has nothing
// before it; a `--since` window only knows a boolean "there's more"
// (no total for a since-filtered query).
let (events, 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 (events, more) = fetch_since(client, args.number, since, limit)?;
(events, 0, 0, more, clamped)
} else if let Some(limit) = args.limit {
let (events, total) = fetch_head(client, args.number, limit)?;
let more_after = total.saturating_sub(events.len());
(events, 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 events = fetch_tail(client, args.number, n, total)?;
let more_before = total.saturating_sub(events.len());
(events, more_before, 0, false, false)
};
// Serialize back to the API's JSON shape so the per-type render
// arms (and their tests) keep working on plain `Value`s.
let v = serde_json::to_value(&events)?;
if client.json_mode() {
return print_json(&json!({
"events": v,
"more_before": more_before,
"more_after": more_after,
"since_more": since_more,
"since_limit_clamped": since_clamped,
}));
}
let Some(events) = v.as_array() else {
return print_json(&v);
};
for ev in events {
print_event(ev);
}
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 activity 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 timeline — never a silent gap. `None` when both are zero (the
/// whole timeline rode the wire). Mirrors `comments.rs`'s helper of the
/// same name/shape.
fn truncation_note(more_before: usize, more_after: usize) -> Option<String> {
match (more_before > 0, more_after > 0) {
(false, false) => None,
(true, false) => Some(format!(
"({more_before} earlier event(s) not shown — use --limit to see the start of the thread)"
)),
(false, true) => Some(format!(
"({more_after} more event(s) not shown — use --tail N to see the latest instead)"
)),
(true, true) => Some(format!(
"({more_before} earlier + {more_after} later event(s) not shown — use --tail N)"
)),
}
}
/// Total timeline-event count for an issue/PR, read off this endpoint's
/// own `X-Total-Count` header via a cheap `page_size=1` request. No
/// cheaper alternate endpoint carries this count — this
/// request-then-discard-the-one-item IS the cheap path.
fn fetch_total(client: &Client, number: u64) -> Result<usize> {
let (owner, name) = client.owner_repo()?;
let (headers, _) = client
.api()
.issue_get_comments_and_timeline(
owner,
name,
index(number)?,
IssueGetCommentsAndTimelineQuery {
since: None,
before: None,
},
)
.page_size(1)
.send()?;
Ok(headers
.x_total_count
.and_then(|t| usize::try_from(t).ok())
.unwrap_or(0))
}
/// Fetch the first page's worth of events (oldest-first) plus the
/// thread's real total, read straight off this same request's
/// `X-Total-Count` header — one request, no separate count query, and
/// (unlike the old over-fetch-by-one trick) an exact total rather than
/// a boolean "more".
fn fetch_head(client: &Client, number: u64, limit: u64) -> Result<(Vec<TimelineComment>, usize)> {
let (owner, name) = client.owner_repo()?;
let (limit, _clamped) = clamp_limit(limit);
let (headers, events) = client
.api()
.issue_get_comments_and_timeline(
owner,
name,
index(number)?,
IssueGetCommentsAndTimelineQuery {
since: None,
before: None,
},
)
.page_size(u32::try_from(limit).unwrap_or(u32::MAX))
.send()?;
let total = headers
.x_total_count
.and_then(|t| usize::try_from(t).ok())
.unwrap_or(events.len());
Ok((events, total))
}
/// Fetch the last `n` timeline events (chronological). `total` (from
/// [`fetch_total`]) drives the pagination plan and is the caller's, not
/// fetched again here. Forgejo paginates oldest-first with no `direction=desc` knob, so this
/// starts fetching from the page that contains item `total - n` rather
/// than paging the whole thread.
fn fetch_tail(
client: &Client,
number: u64,
n: usize,
total: usize,
) -> Result<Vec<TimelineComment>> {
if n == 0 || total == 0 {
return Ok(Vec::new());
}
let (owner, name) = client.owner_repo()?;
let idx = index(number)?;
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;
let mut merged: Vec<TimelineComment> = Vec::with_capacity(n + page_size);
for page in start_page..=last_page {
let (_, arr) = client
.api()
.issue_get_comments_and_timeline(
owner,
name,
idx,
IssueGetCommentsAndTimelineQuery {
since: None,
before: None,
},
)
.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 (events added/
// removed between the count request and now) or upstream's
// playing tricks. Stop rather than spin.
break;
}
merged.extend(arr);
}
let overshoot = merged.len().saturating_sub(n);
Ok(merged.into_iter().skip(overshoot).collect())
}
/// Fetch events 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, so this uses the same over-fetch-by-one trick
/// `comments.rs`'s `fetch_since` does: ask for `limit + 1`, and a full
/// extra row means there's more. Returns `(events, more)`, `more` a
/// boolean rather than an exact count.
///
/// Deliberately bypasses the typed `forgejo-api` client
/// (`issue_get_comments_and_timeline`) rather than calling it directly:
/// this endpoint answers an empty `--since` window with a JSON `null`
/// body, and the typed client's generated response type is
/// `Vec<TimelineComment>`, which fails to deserialize `null` at all — the
/// single most common answer to "anything new since my last check?"
/// turns into a hard error instead of an empty list. The sibling
/// `issue_get_comments` endpoint `comments.rs`'s own `fetch_since` calls
/// returns `[]` for the same case and never hits this. [`NullableVec`] is
/// this codebase's existing, tested newtype for exactly this shape of
/// problem — used the same way for reactions and blocking-issue lists —
/// so this reuses it via [`Client::get_api_json`] rather than inventing a
/// second null-tolerant path.
fn fetch_since(
client: &Client,
number: u64,
since: time::OffsetDateTime,
limit: u64,
) -> Result<(Vec<TimelineComment>, bool)> {
let (owner, name) = client.owner_repo()?;
let idx = index(number)?;
let fetch_limit = limit.saturating_add(1);
let since_str = since
.format(&Rfc3339)
.context("format --since timestamp as RFC 3339")?;
let fetch_limit_str = fetch_limit.to_string();
let path = format!("/repos/{owner}/{name}/issues/{idx}/timeline");
let events: NullableVec<TimelineComment> = client.get_api_json(
&path,
&[("since", since_str.as_str()), ("limit", &fetch_limit_str)],
)?;
let events = events.0;
let limit = usize::try_from(limit).unwrap_or(usize::MAX);
let more = events.len() > limit;
let mut events = events;
events.truncate(limit);
Ok((events, more))
}
/// Render one timeline event as a single `**actor @ ts**: summary`
/// line. Comment rows inline their full body; structured event types
/// (label, assignees, close, etc.) get a one-line human summary
/// derived from the per-type fields the API populates. Unknown /
/// future types fall through to a `[<type>]` placeholder so a forge
/// schema bump doesn't panic the verb — operator still sees that the
/// event existed, with timestamp + actor.
///
/// Pure function (no I/O) so the tests below can pin the formatted
/// output for every supported event type without re-implementing the
/// per-arm dispatch. `print_event` is the only caller that adds the
/// terminating newline.
#[allow(
clippy::too_many_lines,
reason = "flat per-event-type dispatch: one arm per timeline event kind; \
splitting it would scatter the formatting without shrinking it"
)]
fn format_event(ev: &Value) -> String {
let event_type = ev.get("type").and_then(Value::as_str).unwrap_or("?");
let user = ev
.get("user")
.and_then(|u| u.get("login"))
.and_then(Value::as_str)
.unwrap_or("?");
let ts = ev.get("created_at").and_then(Value::as_str).unwrap_or("?");
let summary = match event_type {
"comment" => {
// Comments get the full body inlined — matches `comments`
// verb shape so the operator sees the same line they'd
// get from the head-of-thread listing.
ev.get("body")
.and_then(Value::as_str)
.unwrap_or("")
.to_owned()
}
"label" => {
// Forgejo encodes label add/remove via `body = "1"` (added)
// or `body = "0"` (removed). Quirky but stable.
let action = match ev.get("body").and_then(Value::as_str).unwrap_or("") {
"1" => "added",
"0" => "removed",
_ => "changed",
};
let label = ev
.get("label")
.and_then(|l| l.get("name"))
.and_then(Value::as_str)
.unwrap_or("?");
format!("{action} label `{label}`")
}
"assignees" => {
let assignee = ev
.get("assignee")
.and_then(|a| a.get("login"))
.and_then(Value::as_str)
.unwrap_or("?");
let removed = ev
.get("removed_assignee")
.and_then(Value::as_bool)
.unwrap_or(false);
if removed {
format!("unassigned @{assignee}")
} else {
format!("assigned @{assignee}")
}
}
"review_request" => {
let reviewer = ev
.get("assignee")
.and_then(|a| a.get("login"))
.and_then(Value::as_str)
.unwrap_or("?");
let removed = ev
.get("removed_assignee")
.and_then(Value::as_bool)
.unwrap_or(false);
if removed {
format!("removed review request from @{reviewer}")
} else {
format!("requested review from @{reviewer}")
}
}
"close" => "closed".to_owned(),
"reopen" => "reopened".to_owned(),
"merge" => "merged".to_owned(),
"milestone" => {
let title = ev
.get("milestone")
.and_then(|x| x.get("title"))
.and_then(Value::as_str)
.unwrap_or("?");
format!("added to milestone `{title}`")
}
"demilestone" => {
let title = ev
.get("old_milestone")
.and_then(|x| x.get("title"))
.and_then(Value::as_str)
.unwrap_or("?");
format!("removed from milestone `{title}`")
}
"pull_push" => {
// Body is JSON: `{"is_force_push":bool,"commit_ids":[...]}`.
// Defensive parse — fall through to a no-detail summary if
// the shape ever drifts.
let body_str = ev.get("body").and_then(Value::as_str).unwrap_or("");
let parsed: Option<Value> = serde_json::from_str(body_str).ok();
let n = parsed
.as_ref()
.and_then(|v| v.get("commit_ids"))
.and_then(Value::as_array)
.map_or(0, Vec::len);
let force = parsed
.as_ref()
.and_then(|v| v.get("is_force_push"))
.and_then(Value::as_bool)
.unwrap_or(false);
if force {
format!("force-pushed {n} commit(s)")
} else {
format!("pushed {n} commit(s)")
}
}
"commit_ref" => {
let sha = ev
.get("ref_commit_sha")
.and_then(Value::as_str)
.unwrap_or("");
let short: String = sha.chars().take(7).collect();
if short.is_empty() {
"referenced from a commit".to_owned()
} else {
format!("referenced from commit {short}")
}
}
"comment_ref" | "issue_ref" => "referenced from another issue/PR".to_owned(),
"add_dependency" | "remove_dependency" => {
let verb = if event_type == "add_dependency" {
"added dependency on"
} else {
"removed dependency on"
};
let dep = ev.get("dependent_issue");
let number = dep
.and_then(|d| d.get("number"))
.and_then(Value::as_u64)
.map_or_else(|| "?".to_owned(), |n| n.to_string());
let title = dep
.and_then(|d| d.get("title"))
.and_then(Value::as_str)
.unwrap_or("?");
format!("{verb} #{number} ({title})")
}
"changed_target_branch" => "changed target branch".to_owned(),
"review" => "submitted a review".to_owned(),
"lock" => "locked the conversation".to_owned(),
"unlock" => "unlocked the conversation".to_owned(),
// Future / unknown types: surface the raw label so we don't
// pretend nothing happened. Operator sees `[deploy_status]` or
// whatever new event a forge bump invents.
other => format!("[{other}]"),
};
format!("**{user} @ {ts}**: {summary}")
}
/// Print one event followed by a blank line spacer. Thin wrapper
/// around `format_event` so the tests can pin per-arm output without
/// duplicating the dispatch.
fn print_event(ev: &Value) {
println!("{}", format_event(ev));
println!();
}
#[cfg(test)]
mod tests {
//! Tests call `format_event` directly so any new event-type arm
//! added in `print_event`'s dispatch is automatically covered by
//! the rendering path (no parallel test-side dispatch to keep in
//! sync). A review flagged: "extract a `format_event(ev) ->
//! String` helper and test that function directly instead of
//! duplicating the logic" — addressed.
use super::*;
/// Pure helper mirroring the page-arithmetic in `fetch_tail`: given a
/// total event count + requested tail size, return the (`start_page`,
/// `last_page`) pair the network loop would walk — lets us pin the
/// pagination plan 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() {
assert_eq!(tail_plan(5, 2), (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 → ONE page fetch, not the whole thread.
assert_eq!(tail_plan(5000, 3), (100, 100));
}
#[test]
fn tail_plan_n_exceeds_total() {
assert_eq!(tail_plan(5, 100), (1, 1));
}
#[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() {
let note = truncation_note(12, 0).unwrap();
assert!(note.contains("12"), "{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 comment_renders_body_inline() {
let ev = serde_json::json!({
"type": "comment",
"user": { "login": "iris" },
"created_at": "2026-05-31T12:00:00Z",
"body": "looks good to me",
});
assert_eq!(
format_event(&ev),
"**iris @ 2026-05-31T12:00:00Z**: looks good to me"
);
}
#[test]
fn label_added_renders_action_and_name() {
let ev = serde_json::json!({
"type": "label",
"user": { "login": "triage" },
"created_at": "2026-05-31T12:00:00Z",
"body": "1",
"label": { "name": "area:harness" },
});
assert_eq!(
format_event(&ev),
"**triage @ 2026-05-31T12:00:00Z**: added label `area:harness`"
);
}
#[test]
fn label_removed_renders_removed_action() {
let ev = serde_json::json!({
"type": "label",
"user": { "login": "mara" },
"created_at": "2026-05-31T12:00:00Z",
"body": "0",
"label": { "name": "needs-review" },
});
assert_eq!(
format_event(&ev),
"**mara @ 2026-05-31T12:00:00Z**: removed label `needs-review`"
);
}
#[test]
fn close_event_renders_one_word_summary() {
let ev = serde_json::json!({
"type": "close",
"user": { "login": "mara" },
"created_at": "2026-05-31T12:00:00Z",
});
assert_eq!(format_event(&ev), "**mara @ 2026-05-31T12:00:00Z**: closed");
}
#[test]
fn assignees_added_and_removed() {
let added = serde_json::json!({
"type": "assignees",
"user": { "login": "triage" },
"created_at": "2026-05-31T12:00:00Z",
"assignee": { "login": "damocles" },
"removed_assignee": false,
});
assert_eq!(
format_event(&added),
"**triage @ 2026-05-31T12:00:00Z**: assigned @damocles"
);
let removed = serde_json::json!({
"type": "assignees",
"user": { "login": "triage" },
"created_at": "2026-05-31T12:00:00Z",
"assignee": { "login": "damocles" },
"removed_assignee": true,
});
assert_eq!(
format_event(&removed),
"**triage @ 2026-05-31T12:00:00Z**: unassigned @damocles"
);
}
#[test]
fn pull_push_counts_commits_and_marks_force() {
let normal = serde_json::json!({
"type": "pull_push",
"user": { "login": "damocles" },
"created_at": "2026-05-31T12:00:00Z",
"body": r#"{"is_force_push":false,"commit_ids":["a","b","c"]}"#,
});
assert_eq!(
format_event(&normal),
"**damocles @ 2026-05-31T12:00:00Z**: pushed 3 commit(s)"
);
let forced = serde_json::json!({
"type": "pull_push",
"user": { "login": "damocles" },
"created_at": "2026-05-31T12:00:00Z",
"body": r#"{"is_force_push":true,"commit_ids":["a"]}"#,
});
assert_eq!(
format_event(&forced),
"**damocles @ 2026-05-31T12:00:00Z**: force-pushed 1 commit(s)"
);
}
#[test]
fn commit_ref_truncates_sha_to_seven() {
let ev = serde_json::json!({
"type": "commit_ref",
"user": { "login": "damocles" },
"created_at": "2026-05-31T12:00:00Z",
"ref_commit_sha": "abcdef0123456789abcdef0123456789abcdef01",
});
assert_eq!(
format_event(&ev),
"**damocles @ 2026-05-31T12:00:00Z**: referenced from commit abcdef0"
);
}
#[test]
fn add_dependency_renders_target_issue() {
let ev = serde_json::json!({
"type": "add_dependency",
"user": { "login": "mara" },
"created_at": "2026-05-31T12:00:00Z",
"dependent_issue": { "number": 2850, "title": "extract swarm controller" },
});
assert_eq!(
format_event(&ev),
"**mara @ 2026-05-31T12:00:00Z**: added dependency on #2850 (extract swarm controller)" // lint:allow: sample issue number in a test fixture string, not a tracker tag
);
}
#[test]
fn remove_dependency_renders_target_issue() {
let ev = serde_json::json!({
"type": "remove_dependency",
"user": { "login": "mara" },
"created_at": "2026-05-31T12:00:00Z",
"dependent_issue": { "number": 2850, "title": "extract swarm controller" },
});
assert_eq!(
format_event(&ev),
"**mara @ 2026-05-31T12:00:00Z**: removed dependency on #2850 (extract swarm controller)" // lint:allow: sample issue number in a test fixture string, not a tracker tag
);
}
#[test]
fn unknown_event_type_renders_bracketed_placeholder() {
// Future-proofing: a forge schema bump that adds a new event
// type shouldn't silently swallow the row.
let ev = serde_json::json!({
"type": "deploy_status",
"user": { "login": "ci-bot" },
"created_at": "2026-05-31T12:00:00Z",
});
assert_eq!(
format_event(&ev),
"**ci-bot @ 2026-05-31T12:00:00Z**: [deploy_status]"
);
}
#[test]
fn missing_user_falls_back_to_placeholder() {
// Defensive: forge has been known to omit `user` on bot events.
let ev = serde_json::json!({
"type": "close",
"created_at": "2026-05-31T12:00:00Z",
});
assert_eq!(format_event(&ev), "**? @ 2026-05-31T12:00:00Z**: closed");
}
}