diff --git a/hive-forge/src/verbs/comments.rs b/hive-forge/src/verbs/comments.rs index 51fadc21..610dc1c6 100644 --- a/hive-forge/src/verbs/comments.rs +++ b/hive-forge/src/verbs/comments.rs @@ -14,16 +14,6 @@ //! on this long thread?" without scrolling through the whole //! history. //! -//! For PRs, review *bodies* (the summary text submitted with an -//! approve / request-changes / comment review) are merged in too: -//! they live in the `pulls//reviews` object, NOT the -//! issues/comments thread, so plain comment listings used to miss -//! them entirely and reviewers/authors silently lost feedback — -//! the gap this fix closes. They're always included regardless of -//! `--limit`/`--tail` -//! (reviews are few + high-signal) and tagged `[review: STATE]` so -//! they're distinguishable from issue-thread comments. -//! //! Use the global `--json` flag for JSON output. use anyhow::Result; @@ -58,13 +48,10 @@ pub struct Args { pub fn run(client: &Client, args: Args) -> Result<()> { let repo = client.repo(); - let thread = match args.tail { + let comments = match args.tail { Some(n) => fetch_tail(client, repo, args.number, n)?, None => fetch_head(client, repo, args.number, args.limit)?, }; - // 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, repo, 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); @@ -79,8 +66,6 @@ pub fn run(client: &Client, args: Args) -> Result<()> { "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"), }) }) .collect(); @@ -94,81 +79,13 @@ pub fn run(client: &Client, args: Args) -> Result<()> { .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("?"); - println!("**{user} @ {ts}** [review: {state}]: {body}"); - } else { - println!("**{user} @ {ts}**: {body}"); - } + println!("**{user} @ {ts}**: {body}"); println!(); } Ok(()) } } -/// 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 -/// empty-body reviews (a bare approval adds nothing to the thread). -/// `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, repo: &str, number: u64) -> Vec { - let reviews = client - .get_json(&format!("/repos/{repo}/pulls/{number}/reviews")) - .ok() - .and_then(|v| v.as_array().cloned()) - .unwrap_or_default(); - reviews - .into_iter() - .filter_map(|r| { - let state = r.get("state").and_then(Value::as_str).unwrap_or(""); - if state == "PENDING" { - return None; - } - if r.get("body") - .and_then(Value::as_str) - .unwrap_or("") - .trim() - .is_empty() - { - return None; - } - Some(json!({ - "id": r.get("id"), - "user": r.get("user"), - "created_at": r.get("submitted_at"), - "updated_at": r.get("submitted_at"), - "body": r.get("body"), - "html_url": r.get("html_url"), - "kind": "review", - "state": state, - })) - }) - .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, repo: &str, number: u64, limit: u64) -> Result> { let v = client.get_json(&format!( @@ -289,38 +206,4 @@ mod tests { // 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 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"]); - } }