//! Which of a PR's reviews still stand: the rule `hive-forge pr //! assign-reviewer` and the notification router's `request-reviewer` sink //! share, so a request is skipped or made on the same grounds in both. //! //! The forge keeps a review's `state` (`APPROVED`, `REQUEST_CHANGES`, …) after //! the head moves or the review is dismissed, so reading the state alone //! reports a review that no longer applies as still good. [`latest_reviews`] //! folds a PR's reviews to one verdict per reviewer, and //! [`ReviewInfo::superseded`] says whether it still counts. //! //! Re-requesting a review from a reviewer whose review stands makes the forge //! dismiss it, approval included. [`standing`] is the check a caller makes //! before it requests. /// One review as the forge lists it, reduced to what the rule reads. Every /// field is optional because the forge's listing is. #[derive(Clone, Copy, Debug, Default)] pub struct Listed<'a> { pub login: Option<&'a str>, pub state: Option<&'a str>, /// The commit the review was submitted against. pub commit_id: Option<&'a str>, /// The forge's own `stale` flag. pub stale: Option, pub dismissed: Option, } /// One reviewer's latest verdict on a PR, plus the `stale` / `dismissed` bits. /// /// `stale` is set by the forge when the PR head commit changed after this /// review was submitted; `dismissed` is set when the review was explicitly /// dismissed. Either way the review no longer applies to the current head even /// though its `state` string still reads `APPROVED` / `REQUEST_CHANGES`. /// /// ⚠️ **`stale` here is not the forge's flag alone.** That flag is eventually /// consistent: seconds after a push it still reports the pre-push answer, so a /// verdict against the previous head reads as current in exactly the window /// where a push has just happened. [`latest_reviews`] therefore ORs it with a /// direct comparison of the review's own `commit_id` against the PR head: the /// flag is right *eventually*, the comparison is right *immediately*. #[derive(Clone, Debug, PartialEq, Eq)] pub struct ReviewInfo { pub login: String, pub state: String, pub stale: bool, pub dismissed: bool, } impl ReviewInfo { /// True when the review no longer applies to the current head: stale /// (head moved) or dismissed. Such a verdict neither blocks a merge nor /// counts as a fresh approval. #[must_use] pub fn superseded(&self) -> bool { self.stale || self.dismissed } } /// `login`'s review that still stands, if it has one. Its presence is the /// reason not to request a review from `login` again. #[must_use] pub fn standing<'a>(reviews: &'a [ReviewInfo], login: &str) -> Option<&'a ReviewInfo> { reviews.iter().find(|r| r.login == login && !r.superseded()) } /// Whether this review was submitted against a commit that is no longer the PR /// head: the race-free half of the staleness check. /// /// Fails **closed on unknowns**, i.e. "not stale": either side missing means /// the head's move can't be shown, and guessing wrong in that direction costs /// one redundant re-review, where the other direction would void every /// verdict on the PR and invite a re-request that dismisses a real approval. #[must_use] pub fn reviewed_older_head(reviewed_sha: Option<&str>, head_sha: Option<&str>) -> bool { let (Some(head), Some(reviewed)) = (head_sha, reviewed_sha) else { return false; }; // Both emptiness checks matter, and for the same reason: a blank string is // a value the forge sent, not a sha it has. Treating one as real would make // every review compare unequal and mark the whole PR stale. !head.is_empty() && !reviewed.is_empty() && reviewed != head } /// Latest non-comment review per reviewer. `reviews` come oldest first, so a /// later verdict from the same reviewer supersedes an earlier one; `COMMENT` /// and `PENDING` reviews carry no verdict and are skipped. `head_sha` is the /// PR's current head, or `None` when it couldn't be read, which leaves the /// forge's `stale` flag alone to decide. #[must_use] pub fn latest_reviews<'a>( reviews: impl IntoIterator>, head_sha: Option<&str>, ) -> Vec { let mut latest: Vec = Vec::new(); for r in reviews { let Some(login) = r.login else { continue }; let verdict = r.state.unwrap_or(""); if verdict == "COMMENT" || verdict == "PENDING" || verdict.is_empty() { continue; } let stale = r.stale.unwrap_or(false) || reviewed_older_head(r.commit_id, head_sha); let dismissed = r.dismissed.unwrap_or(false); if let Some(slot) = latest.iter_mut().find(|info| info.login == login) { verdict.clone_into(&mut slot.state); slot.stale = stale; slot.dismissed = dismissed; } else { latest.push(ReviewInfo { login: login.to_owned(), state: verdict.to_owned(), stale, dismissed, }); } } latest } #[cfg(test)] mod tests { use super::*; fn review<'a>(login: &'a str, state: &'a str, commit: &'a str) -> Listed<'a> { Listed { login: Some(login), state: Some(state), commit_id: Some(commit), ..Listed::default() } } #[test] fn review_on_an_older_commit_is_stale() { // The forge still reports `stale: false` here in the seconds after a // push, so the comparison has to catch it. assert!(reviewed_older_head(Some("81292f14"), Some("f4c47088"))); } #[test] fn review_on_the_current_head_is_not_stale() { assert!(!reviewed_older_head(Some("f4c47088"), Some("f4c47088"))); } /// Every unknown fails *toward* keeping the verdict. Voiding every review /// on a forge that stopped reporting one of these would be a far louder /// wrong answer than one redundant re-review. #[test] fn unknown_commit_or_head_is_not_stale() { assert!(!reviewed_older_head(None, Some("f4c47088"))); assert!(!reviewed_older_head(Some("81292f14"), None)); assert!(!reviewed_older_head(Some(""), Some("f4c47088"))); // Absent and blank have to behave the same on BOTH sides: a blank head // that counted as real would mark every review on the PR stale. assert!(!reviewed_older_head(Some("81292f14"), Some(""))); assert!(!reviewed_older_head(Some(""), Some(""))); } #[test] fn a_review_of_the_current_head_stands_and_one_of_an_older_head_does_not() { let reviews = latest_reviews( [ review("review-classifier", "APPROVED", "new"), review("argus", "APPROVED", "old"), ], Some("new"), ); assert_eq!( standing(&reviews, "review-classifier").map(|r| r.state.as_str()), Some("APPROVED") ); // The forge's flag is still false for argus: only the commit says so. assert!(standing(&reviews, "argus").is_none()); // Control: a reviewer who never reviewed has nothing standing. assert!(standing(&reviews, "iris").is_none()); } #[test] fn a_dismissed_or_flagged_review_does_not_stand() { let reviews = latest_reviews( [ Listed { dismissed: Some(true), ..review("a", "APPROVED", "new") }, Listed { stale: Some(true), ..review("b", "REQUEST_CHANGES", "new") }, review("c", "APPROVED", "new"), ], Some("new"), ); assert!(standing(&reviews, "a").is_none()); assert!(standing(&reviews, "b").is_none()); assert!(standing(&reviews, "c").is_some(), "control"); } #[test] fn the_latest_verdict_per_reviewer_wins_and_comments_carry_none() { let reviews = latest_reviews( [ review("a", "REQUEST_CHANGES", "old"), review("a", "APPROVED", "new"), review("a", "COMMENT", "new"), review("b", "PENDING", "new"), Listed { login: None, ..review("x", "APPROVED", "new") }, ], Some("new"), ); assert_eq!( reviews, [ReviewInfo { login: "a".to_owned(), state: "APPROVED".to_owned(), stale: false, dismissed: false, }] ); } /// With the head unread, the forge's flag decides alone, so a review of an /// unknown commit stands. #[test] fn an_unread_head_leaves_the_forges_flag_to_decide() { let reviews = latest_reviews([review("a", "APPROVED", "old")], None); assert!(standing(&reviews, "a").is_some()); } }