Merge pull request #1490 from rust-lang/github-hide-comments

Hide old `rust-timer` comments when a summary is posted

Author: Kobzol

site/src/github.rs

@@ -8,6 +8,15 @@ use serde::Deserialize;
 
 type BoxedError = Box<dyn std::error::Error + Send + Sync>;
 
+pub const RUST_REPO_GITHUB_API_URL: &str = "https://api.github.com/repos/rust-lang/rust";
+pub const RUST_REPO_GITHUB_GRAPH_URL: &str = "https://api.github.com/graphql";
+
+/// Comments that are temporary and do not add any value once there has been a new development
+/// (a rustc build or a perf. run was finished) are marked with this comment.
+///
+/// They are removed once a perf. run comparison summary is posted on a PR.
+pub const COMMENT_MARK_TEMPORARY: &str = "<!-- rust-timer: temporary -->";
+
 pub use comparison_summary::post_finished;
 
 /// Enqueues try build artifacts and posts a message about them on the original rollup PR
@@ -234,6 +243,7 @@ pub async fn enqueue_shas(
             ));
         }
     }
+    msg.push_str(&format!("\n{COMMENT_MARK_TEMPORARY}"));
     main_client.post_comment(pr_number, msg).await;
     Ok(())
 }

site/src/github/client.rs

@@ -1,6 +1,7 @@
 use anyhow::Context;
 use chrono::{DateTime, Utc};
 use http::header::USER_AGENT;
+use serde::de::DeserializeOwned;
 
 use crate::{api::github::Issue, load::SiteCtxt};
 
@@ -226,6 +227,124 @@ impl Client {
         }
     }
 
+    pub async fn get_comments(&self, pull_request: u32) -> anyhow::Result<Vec<ResponseComment>> {
+        const QUERY: &str = "query($owner: String!, $repo: String!, $pr: Int!, $cursor: String) {
+                repository(owner: $owner, name: $repo) {
+                    pullRequest(number: $pr) {
+                        comments(first: 100, after: $cursor) {
+                            nodes {
+                                id
+                                body
+                                isMinimized
+                                viewerDidAuthor
+                            }
+                            pageInfo {
+                                endCursor
+                            }
+                        }
+                    }
+                }
+            }";
+
+        #[derive(Debug, serde::Deserialize)]
+        struct Response {
+            repository: ResponseRepo,
+        }
+        #[derive(Debug, serde::Deserialize)]
+        #[serde(rename_all = "camelCase")]
+        struct ResponseRepo {
+            pull_request: ResponsePR,
+        }
+        #[derive(Debug, serde::Deserialize)]
+        struct ResponsePR {
+            comments: ResponseComments,
+        }
+        #[derive(Debug, serde::Deserialize)]
+        #[serde(rename_all = "camelCase")]
+        struct ResponseComments {
+            nodes: Vec<ResponseComment>,
+            page_info: GraphPageInfo,
+        }
+
+        let owner = "rust-lang";
+        let repo = "rust";
+
+        let mut comments = Vec::new();
+        let mut cursor = None;
+        loop {
+            let mut resp: Response = self
+                .graphql(
+                    QUERY,
+                    serde_json::json!({
+                        "owner": owner,
+                        "repo": repo,
+                        "pr": pull_request,
+                        "cursor": cursor,
+                    }),
+                )
+                .await?;
+            cursor = resp.repository.pull_request.comments.page_info.end_cursor;
+            comments.append(&mut resp.repository.pull_request.comments.nodes);
+
+            if cursor.is_none() {
+                break;
+            }
+        }
+        Ok(comments)
+    }
+
+    pub async fn hide_comment(&self, comment_id: &str, reason: &str) -> anyhow::Result<()> {
+        #[derive(serde::Deserialize)]
+        struct MinimizeData {}
+
+        const MINIMIZE: &str = "mutation($node_id: ID!, $reason: ReportedContentClassifiers!) {
+            minimizeComment(input: {subjectId: $node_id, classifier: $reason}) {
+                __typename
+            }
+        }";
+
+        self.graphql::<Option<MinimizeData>, _>(
+            MINIMIZE,
+            serde_json::json!({
+                "node_id": comment_id,
+                "reason": reason,
+            }),
+        )
+        .await?;
+        Ok(())
+    }
+
+    async fn graphql<T: DeserializeOwned, V: serde::Serialize>(
+        &self,
+        query: &str,
+        variables: V,
+    ) -> anyhow::Result<T> {
+        #[derive(serde::Serialize)]
+        struct GraphPayload<'a, V> {
+            query: &'a str,
+            variables: V,
+        }
+
+        let response: GraphResponse<T> = self
+            .inner
+            .post(&self.repository_url)
+            .json(&GraphPayload { query, variables })
+            .send()
+            .await?
+            .error_for_status()?
+            .json()
+            .await?;
+
+        if response.errors.is_empty() {
+            Ok(response.data)
+        } else {
+            Err(anyhow::anyhow!(
+                "GraphQL query failed: {}",
+                response.errors[0].message
+            ))
+        }
+    }
+
     async fn send(
         &self,
         request: reqwest::RequestBuilder,
@@ -238,6 +357,34 @@ impl Client {
     }
 }
 
+#[derive(serde::Deserialize)]
+struct GraphResponse<T> {
+    data: T,
+    #[serde(default)]
+    errors: Vec<GraphError>,
+}
+
+#[derive(Debug, serde::Deserialize)]
+struct GraphError {
+    message: String,
+}
+
+#[derive(Debug, serde::Deserialize)]
+#[serde(rename_all = "camelCase")]
+struct GraphPageInfo {
+    end_cursor: Option<String>,
+}
+
+#[derive(Debug, serde::Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct ResponseComment {
+    pub id: String,
+    pub body: String,
+    pub is_minimized: bool,
+    // Is the account that fetches this comment also the original author of the comment?
+    pub viewer_did_author: bool,
+}
+
 #[derive(Debug, serde::Deserialize)]
 pub struct CreatePrResponse {
     pub number: u32,

site/src/github/comparison_summary.rs

@@ -6,6 +6,7 @@ use crate::load::SiteCtxt;
 
 use database::{ArtifactId, QueuedCommit};
 
+use crate::github::{COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL, RUST_REPO_GITHUB_GRAPH_URL};
 use std::collections::HashSet;
 use std::fmt::Write;
 
@@ -57,26 +58,45 @@ pub async fn post_finished(ctxt: &SiteCtxt) {
             assert_eq!(completed, queued_commit);
 
             let is_master_commit = master_commits.contains(&queued_commit.sha);
-            post_comparison_comment(ctxt, queued_commit, is_master_commit).await;
+            if let Err(error) = post_comparison_comment(ctxt, queued_commit, is_master_commit).await
+            {
+                log::error!("Cannot post comparison comment: {error:?}");
+            }
         }
     }
 }
 
 /// Posts a comment to GitHub summarizing the comparison of the queued commit with its parent
 ///
 /// `is_master_commit` is used to differentiate messages for try runs and post-merge runs.
-async fn post_comparison_comment(ctxt: &SiteCtxt, commit: QueuedCommit, is_master_commit: bool) {
-    let client = super::client::Client::from_ctxt(
-        ctxt,
-        "https://api.github.com/repos/rust-lang/rust".to_owned(),
-    );
+async fn post_comparison_comment(
+    ctxt: &SiteCtxt,
+    commit: QueuedCommit,
+    is_master_commit: bool,
+) -> anyhow::Result<()> {
+    let client = super::client::Client::from_ctxt(ctxt, RUST_REPO_GITHUB_API_URL.to_owned());
     let pr = commit.pr;
     let body = match summarize_run(ctxt, commit, is_master_commit).await {
         Ok(message) => message,
         Err(error) => error,
     };
 
     client.post_comment(pr, body).await;
+
+    let graph_client =
+        super::client::Client::from_ctxt(ctxt, RUST_REPO_GITHUB_GRAPH_URL.to_owned());
+    for comment in graph_client.get_comments(pr).await? {
+        // If this bot is the author of the comment, the comment is not yet minimized and it is
+        // a temporary comment, minimize it.
+        if comment.viewer_did_author
+            && !comment.is_minimized
+            && comment.body.contains(COMMENT_MARK_TEMPORARY)
+        {
+            log::debug!("Hiding comment {}", comment.id);
+            graph_client.hide_comment(&comment.id, "OUTDATED").await?;
+        }
+    }
+    Ok(())
 }
 
 fn make_comparison_url(commit: &QueuedCommit, stat: Metric) -> String {

site/src/request_handlers/github.rs

@@ -1,5 +1,8 @@
 use crate::api::{github, ServerResult};
-use crate::github::{client, enqueue_shas, parse_homu_comment, rollup_pr_number, unroll_rollup};
+use crate::github::{
+    client, enqueue_shas, parse_homu_comment, rollup_pr_number, unroll_rollup,
+    COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL,
+};
 use crate::load::SiteCtxt;
 
 use std::sync::Arc;
@@ -29,10 +32,7 @@ async fn handle_push(ctxt: Arc<SiteCtxt>, push: github::Push) -> ServerResult<gi
         &ctxt,
         "https://api.github.com/repos/rust-lang-ci/rust".to_owned(),
     );
-    let main_repo_client = client::Client::from_ctxt(
-        &ctxt,
-        "https://api.github.com/repos/rust-lang/rust".to_owned(),
-    );
+    let main_repo_client = client::Client::from_ctxt(&ctxt, RUST_REPO_GITHUB_API_URL.to_owned());
     if push.r#ref != "refs/heads/master" || push.sender.login != "bors" {
         return Ok(github::Response);
     }
@@ -70,10 +70,7 @@ async fn handle_issue(
     issue: github::Issue,
     comment: github::Comment,
 ) -> ServerResult<github::Response> {
-    let main_client = client::Client::from_ctxt(
-        &ctxt,
-        "https://api.github.com/repos/rust-lang/rust".to_owned(),
-    );
+    let main_client = client::Client::from_ctxt(&ctxt, RUST_REPO_GITHUB_API_URL.to_owned());
     let ci_client = client::Client::from_ctxt(
         &ctxt,
         "https://api.github.com/repos/rust-lang-ci/rust".to_owned(),
@@ -112,7 +109,10 @@ async fn handle_rust_timer(
         main_client
             .post_comment(
                 issue.number,
-                "Insufficient permissions to issue commands to rust-timer.",
+                format!(
+                    "Insufficient permissions to issue commands to rust-timer.
+{COMMENT_MARK_TEMPORARY}"
+                ),
             )
             .await;
         return Ok(github::Response);
@@ -129,9 +129,13 @@ async fn handle_rust_timer(
         main_client
             .post_comment(
                 issue.number,
-                "Awaiting bors try build completion.
+                format!(
+                    "Awaiting bors try build completion.
+
+@rustbot label: +S-waiting-on-perf
 
-@rustbot label: +S-waiting-on-perf",
+{COMMENT_MARK_TEMPORARY}"
+                ),
             )
             .await;
         return Ok(github::Response);