Delete reports by commit SHA; combine PR comments

.sqlx/query-cad10eac152f1f6ab49343a232b493d5599fe2f99367c8d88652be15a6491f74.json

@@ -0,0 +1,20 @@
+{
+  "db_name": "SQLite",
+  "query": "\n            SELECT version\n            FROM reports JOIN projects ON reports.project_id = projects.id\n            WHERE projects.owner = ? COLLATE NOCASE AND projects.repo = ? COLLATE NOCASE\n                  AND git_commit = ? COLLATE NOCASE\n            ORDER BY version\n            ",
+  "describe": {
+    "columns": [
+      {
+        "name": "version",
+        "ordinal": 0,
+        "type_info": "Text"
+      }
+    ],
+    "parameters": {
+      "Right": 3
+    },
+    "nullable": [
+      false
+    ]
+  },
+  "hash": "cad10eac152f1f6ab49343a232b493d5599fe2f99367c8d88652be15a6491f74"
+}

crates/db/src/lib.rs

@@ -180,6 +180,33 @@ impl Database {
         Ok(())
     }
 
+    pub async fn get_versions_for_commit(
+        &self,
+        owner: &str,
+        repo: &str,
+        commit: &str,
+    ) -> Result<Vec<String>> {
+        let mut conn = self.pool.acquire().await?;
+        let versions = sqlx::query!(
+            r#"
+            SELECT version
+            FROM reports JOIN projects ON reports.project_id = projects.id
+            WHERE projects.owner = ? COLLATE NOCASE AND projects.repo = ? COLLATE NOCASE
+                  AND git_commit = ? COLLATE NOCASE
+            ORDER BY version
+            "#,
+            owner,
+            repo,
+            commit
+        )
+        .fetch_all(&mut *conn)
+        .await?
+        .into_iter()
+        .map(|row| row.version)
+        .collect();
+        Ok(versions)
+    }
+
     pub async fn get_report(
         &self,
         owner: &str,

crates/github/src/changes.rs

@@ -285,6 +285,23 @@ fn generate_changes_list(changes: Vec<ChangeLine>, out: &mut String) {
     }
 }
 
+pub fn generate_missing_report_comment(
+    version: &str,
+    from_commit: Option<&Commit>,
+    to_commit: Option<&Commit>,
+) -> String {
+    format!(
+        "### Report for {} ({} - {})\n\n[!] Report not found. Did the build succeed?\n\n",
+        version,
+        from_commit.map_or("<none>", |c| &c.sha[..7]),
+        to_commit.map_or("<none>", |c| &c.sha[..7])
+    )
+}
+
+pub fn generate_combined_comment(version_comments: Vec<String>) -> String {
+    version_comments.join("---\n\n")
+}
+
 pub fn generate_comment(
     from: &Report,
     to: &Report,
@@ -390,3 +407,39 @@ pub fn generate_comment(
     }
     comment
 }
+
+#[cfg(test)]
+mod tests {
+    use decomp_dev_core::models::Commit;
+    use time::UtcDateTime;
+
+    use super::*;
+
+    #[test]
+    fn test_generate_missing_report_comment() {
+        let commit = Commit {
+            sha: "abc1234567890".to_string(),
+            message: Some("Test commit".to_string()),
+            timestamp: UtcDateTime::UNIX_EPOCH,
+        };
+        let comment = generate_missing_report_comment("GALE01", Some(&commit), Some(&commit));
+        assert_eq!(
+            comment,
+            "### Report for GALE01 (abc1234 - abc1234)\n\n[!] Report not found. Did the build succeed?\n\n"
+        );
+    }
+
+    #[test]
+    fn test_commit_sha_truncation() {
+        let long_commit = Commit {
+            sha: "abcdef1234567890abcdef1234567890abcdef12".to_string(),
+            message: Some("Long commit SHA".to_string()),
+            timestamp: UtcDateTime::UNIX_EPOCH,
+        };
+        let comment =
+            generate_missing_report_comment("GALE01", Some(&long_commit), Some(&long_commit));
+        // Should truncate SHA to 7 characters
+        assert!(comment.contains("(abcdef1 - abcdef1)"));
+        assert!(!comment.contains("abcdef1234567890"));
+    }
+}

crates/github/src/webhook.rs

@@ -28,7 +28,10 @@ use sha2::Sha256;
 
 use crate::{
     GitHub, ProcessWorkflowRunResult,
-    changes::{generate_changes, generate_comment},
+    changes::{
+        generate_changes, generate_combined_comment, generate_comment,
+        generate_missing_report_comment,
+    },
     commit_from_head_commit, process_workflow_run,
 };
 
@@ -290,8 +293,22 @@ async fn handle_workflow_run_completed(
             let issues = client.issues_by_id(repository_id);
             // Only fetch first page for now
             let existing_comments = issues.list_comments(pull_request.number).send().await?;
+
+            // Get all versions that exist on the base branch to check for missing reports
+            let base_versions = state
+                .db
+                .get_versions_for_commit(
+                    &project_info.project.owner,
+                    &project_info.project.repo,
+                    &base_commit.sha,
+                )
+                .await?;
+
+            let mut version_comments = Vec::new();
+
+            // Process existing artifacts from PR
             for artifact in &artifacts {
-                let Some(cached_report) = state
+                let cached_report = state
                     .db
                     .get_report(
                         &project_info.project.owner,
@@ -299,45 +316,76 @@ async fn handle_workflow_run_completed(
                         &base_commit.sha,
                         &artifact.version,
                     )
-                    .await?
-                else {
+                    .await?;
+
+                if let Some(cached_report) = cached_report {
+                    let report_file = state.db.upgrade_report(&cached_report).await?;
+                    let report = report_file.report.flatten();
+                    let changes = generate_changes(&report, &artifact.report)?;
+                    version_comments.push(generate_comment(
+                        &report,
+                        &artifact.report,
+                        Some(&report_file.version),
+                        Some(&report_file.commit),
+                        Some(&commit),
+                        changes,
+                    ));
+                } else {
                     tracing::warn!(
-                        "No report found for pull request {} (base {}) and version {}",
+                        "No base report found for pull request {} (base {}) and version {}",
                         pull_request.id,
                         pull_request.base.sha,
                         artifact.version
                     );
-                    continue;
-                };
-                let report_file = state.db.upgrade_report(&cached_report).await?;
-                let report = report_file.report.flatten();
-                let changes = generate_changes(&report, &artifact.report)?;
-                let comment_text = generate_comment(
-                    &report,
-                    &artifact.report,
-                    Some(&report_file.version),
-                    Some(&report_file.commit),
-                    Some(&commit),
-                    changes,
-                );
-                let existing_comment = existing_comments
+                    version_comments.push(generate_missing_report_comment(
+                        &artifact.version,
+                        Some(&base_commit),
+                        Some(&commit),
+                    ));
+                }
+            }
+
+            // Check for versions that exist on base but are missing from PR
+            for base_version in &base_versions {
+                if !artifacts.iter().any(|a| a.version == *base_version) {
+                    version_comments.push(generate_missing_report_comment(
+                        base_version,
+                        Some(&base_commit),
+                        Some(&commit),
+                    ));
+                }
+            }
+
+            if !version_comments.is_empty() {
+                let combined_comment = generate_combined_comment(version_comments);
+
+                // Find the first existing comment that contains "Report for" any version
+                let existing_report_comments: Vec<_> = existing_comments
                     .items
                     .iter()
-                    .find(|comment| {
+                    .filter(|comment| {
                         // TODO check author ID
-                        comment.body.as_ref().is_some_and(|body| {
-                            body.contains(format!("Report for {}", artifact.version).as_str())
-                        })
+                        comment.body.as_ref().is_some_and(|body| body.contains("### Report for "))
                     })
-                    .map(|comment| comment.id);
-                if let Some(existing_comment) = existing_comment {
+                    .collect();
+
+                if let Some(first_comment) = existing_report_comments.first() {
+                    // Update the first comment
                     issues
-                        .update_comment(existing_comment, comment_text)
+                        .update_comment(first_comment.id, combined_comment)
                         .await
                         .context("Failed to update existing comment")?;
+
+                    // Delete any additional report comments
+                    for comment in existing_report_comments.iter().skip(1) {
+                        if let Err(e) = issues.delete_comment(comment.id).await {
+                            tracing::warn!("Failed to delete old comment {}: {}", comment.id, e);
+                        }
+                    }
                 } else {
+                    // Create new comment
                     issues
-                        .create_comment(pull_request.number, comment_text)
+                        .create_comment(pull_request.number, combined_comment)
                         .await
                         .context("Failed to create comment")?;
                 }

crates/web/src/handlers/manage.rs

@@ -553,6 +553,16 @@ async fn render_manage_project(
                             small { "Fetches any missing report artifacts." }
                         }
                     }
+                    form.mt-spacing action=(format!("/manage/{}/{}/delete-commit", project_info.project.owner, project_info.project.repo)) method="post" data-loading="Deleting..." {
+                        label {
+                            "Delete reports"
+                            fieldset role="group" {
+                                input name="commit_sha" type="text" placeholder="Full commit SHA (40 characters)" pattern="[a-f0-9]{40}" required;
+                                button.outline type="submit" { "Delete" }
+                            }
+                            small { "Delete all reports for a specific commit. Must be the full 40-character SHA." }
+                        }
+                    }
                 }
             }
             (ctx.footer(Some(current_user)))
@@ -673,20 +683,19 @@ pub async fn manage_project_refresh(
 }
 
 #[derive(Deserialize)]
-pub struct DeleteCommitParams {
-    #[serde(flatten)]
-    project: ProjectParams,
-    commit: String,
+pub struct DeleteCommitForm {
+    commit_sha: String,
 }
 
 pub async fn delete_commit(
-    Path(params): Path<DeleteCommitParams>,
+    Path(params): Path<ProjectParams>,
     State(state): State<AppState>,
     current_user: CurrentUser,
     session: Session,
+    Form(form): Form<DeleteCommitForm>,
 ) -> Result<Response, AppError> {
     let Some(info) =
-        state.db.get_project_info(&params.project.owner, &params.project.repo, None).await?
+        state.db.get_project_info(&params.owner, &params.repo, None).await?
     else {
         return Err(AppError::Status(StatusCode::NOT_FOUND));
     };
@@ -694,13 +703,13 @@ pub async fn delete_commit(
         return Err(AppError::Status(StatusCode::FORBIDDEN));
     }
     let num_reports_deleted =
-        state.db.delete_reports_by_commit(info.project.id, &params.commit).await?;
+        state.db.delete_reports_by_commit(info.project.id, &form.commit_sha).await?;
     let message = if num_reports_deleted > 0 {
         Message::Info(format!("Deleted {num_reports_deleted} reports"))
     } else {
         Message::Error("No reports found. Is the commit SHA correct?".to_string())
     };
     session.insert(&format!("manage_{}_message", info.project.id), message).await?;
-    let redirect_url = format!("/manage/{}/{}", params.project.owner, params.project.repo);
+    let redirect_url = format!("/manage/{}/{}", params.owner, params.repo);
     Ok(Redirect::to(&redirect_url).into_response())
 }

crates/web/src/handlers/mod.rs

@@ -5,7 +5,7 @@ use axum::{
     extract::{DefaultBodyLimit, Request},
     http::{HeaderMap, HeaderValue, header, header::Entry},
     response::Response,
-    routing::{delete, get, post},
+    routing::{get, post},
 };
 use decomp_dev_images::image_mime_from_ext;
 use mime::Mime;
@@ -69,7 +69,7 @@ pub fn build_router() -> Router<AppState> {
                 .layer(DefaultBodyLimit::max(50 * 1000 * 1000 /* 50MB */)),
         )
         .route("/manage/{owner}/{repo}/refresh", post(manage::manage_project_refresh))
-        .route("/manage/{owner}/{repo}/commit/{commit}", delete(manage::delete_commit))
+        .route("/manage/{owner}/{repo}/delete-commit", post(manage::delete_commit))
         .route("/og.png", get(decomp_dev_images::get_og))
         .route("/", get(project::get_projects))
         .route("/projects", get(project::get_projects))

css/main.scss

@@ -593,4 +593,8 @@ label:has(> input[type="checkbox"]) {
       text-align: center;
     }
   }
+}
+
+.mt-spacing {
+  margin-top: var(--pico-spacing);
 }

js/api.tsx

@@ -1,6 +1,6 @@
-import { StrictMode, useState, useEffect, useMemo, useRef } from 'react';
-import { createRoot } from 'react-dom/client';
 import hljs from 'highlight.js/lib/core';
+import { StrictMode, useEffect, useMemo, useRef, useState } from 'react';
+import { createRoot } from 'react-dom/client';
 import 'highlight.js/styles/hybrid.css';
 import styles from './api.module.css';
 

js/projects.ts

@@ -1,65 +1,67 @@
 const platforms: Map<string, HTMLInputElement> = new Map();
 
 function updateProjectVisibility() {
-    const selectedPlatforms: string[] = [];
-    let allSelected = true;
-    for (const [platform, checkbox] of platforms.entries()) {
-        if (checkbox.checked) {
-            selectedPlatforms.push(platform);
-        } else {
-            allSelected = false;
-        }
-    }
-    if (selectedPlatforms.length === 0) {
-        allSelected = true;
-        for (const cb of platforms.values()) {
-            cb.checked = true;
-        }
-    }
-    const url = new URL(window.location.href);
-    if (allSelected) {
-        if (url.searchParams.has('platform')) {
-            url.searchParams.delete('platform');
-            window.location.replace(url);
-        }
+  const selectedPlatforms: string[] = [];
+  let allSelected = true;
+  for (const [platform, checkbox] of platforms.entries()) {
+    if (checkbox.checked) {
+      selectedPlatforms.push(platform);
     } else {
-        url.searchParams.set('platform', selectedPlatforms.join(','));
-        window.location.replace(url.toString().replace(/%2C/g, ','));
+      allSelected = false;
     }
+  }
+  if (selectedPlatforms.length === 0) {
+    allSelected = true;
+    for (const cb of platforms.values()) {
+      cb.checked = true;
+    }
+  }
+  const url = new URL(window.location.href);
+  if (allSelected) {
+    if (url.searchParams.has('platform')) {
+      url.searchParams.delete('platform');
+      window.location.replace(url);
+    }
+  } else {
+    url.searchParams.set('platform', selectedPlatforms.join(','));
+    window.location.replace(url.toString().replace(/%2C/g, ','));
+  }
 }
 
 document.querySelectorAll('.platform-item').forEach((item) => {
-    const checkbox = item.querySelector('input[type="checkbox"]') as HTMLInputElement | null;
-    const button = item.querySelector('button') as HTMLButtonElement | null;
-    if (!checkbox || !button) {
-        return;
+  const checkbox = item.querySelector(
+    'input[type="checkbox"]',
+  ) as HTMLInputElement | null;
+  const button = item.querySelector('button') as HTMLButtonElement | null;
+  if (!checkbox || !button) {
+    return;
+  }
+  platforms.set(checkbox.value, checkbox);
+  checkbox.addEventListener('click', (e) => e.stopPropagation());
+  checkbox.addEventListener('change', () => updateProjectVisibility());
+  button.addEventListener('click', () => {
+    let allUnchecked = checkbox.checked;
+    if (allUnchecked) {
+      for (const cb of platforms.values()) {
+        if (cb !== checkbox && cb.checked) {
+          allUnchecked = false;
+          break;
+        }
+      }
     }
-    platforms.set(checkbox.value, checkbox);
-    checkbox.addEventListener('click', (e) => e.stopPropagation());
-    checkbox.addEventListener('change', () => updateProjectVisibility());
-    button.addEventListener('click', () => {
-        let allUnchecked = checkbox.checked;
-        if (allUnchecked) {
-            for (const cb of platforms.values()) {
-                if (cb !== checkbox && cb.checked) {
-                    allUnchecked = false;
-                    break;
-                }
-            }
+    if (allUnchecked) {
+      for (const cb of platforms.values()) {
+        cb.checked = true;
+      }
+    } else {
+      // Enable this checkbox and disable all others
+      checkbox.checked = true;
+      for (const cb of platforms.values()) {
+        if (cb !== checkbox) {
+          cb.checked = false;
         }
-        if (allUnchecked) {
-            for (const cb of platforms.values()) {
-                cb.checked = true;
-            }
-        } else {
-            // Enable this checkbox and disable all others
-            checkbox.checked = true;
-            for (const cb of platforms.values()) {
-                if (cb !== checkbox) {
-                    cb.checked = false;
-                }
-            }
-        }
-        updateProjectVisibility();
-    });
+      }
+    }
+    updateProjectVisibility();
+  });
 });

js/treemap.ts

@@ -463,7 +463,7 @@ const checkFilterTermMatches = (term: string, unit: Unit): boolean => {
 
 window.drawTreemap = drawTreemap;
 
-(function () {
+(() => {
   const url = new URL(window.location.href);
   const filterFromUrl = url.searchParams.get('filter');
   if (filterFromUrl) {

rsbuild.config.ts

@@ -1,7 +1,7 @@
 import { defineConfig } from '@rsbuild/core';
+import { pluginReact } from '@rsbuild/plugin-react';
 import { pluginSass } from '@rsbuild/plugin-sass';
 import { pluginTypeCheck } from '@rsbuild/plugin-type-check';
-import { pluginReact } from '@rsbuild/plugin-react';
 
 export default defineConfig({
   source: {