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

fix(#2088): merge pr review bodies into hive-forge comments listing

Browse source
This commit is contained in:
damocles 2026-06-29 01:42:47 +02:00 • committed by mara
commit 1cff77f5fa
1 changed files with 117 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -14,6 +14,15 @@
//! 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/<n>/reviews` object, NOT the
//! issues/comments thread, so plain comment listings used to miss
//! them entirely and reviewers/authors silently lost feedback
//! (#2088). 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;
@ -48,10 +57,13 @@ pub struct Args {
pub fn run(client: &Client, args: Args) -> Result<()> {
let repo = client.repo();
let comments = match args.tail {
let thread = 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 (#2088).
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);
@ -66,6 +78,8 @@ 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();
@ -79,13 +93,80 @@ 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("");
println!("**{user} @ {ts}**: {body}");
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!();
}
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/<n>/reviews` object, not the
/// issues/comments thread, so the plain comment listing misses them
/// (#2088). 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<Value> {
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<Value>, reviews: Vec<Value>) -> Vec<Value> {
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<Vec<Value>> {
let v = client.get_json(&format!(
@ -206,4 +287,38 @@ 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 #2088 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"]);
}
}