refactor: avoid computing stats and conclusion

apps/backend/src/build-notification/comment.ts

@@ -28,9 +28,9 @@ function getBuildName(input: { build: Build; project: Project | null }) {
   return input.build.name;
 }
 
-export const getCommentBody = async (props: {
+export async function getCommentBody(props: {
   commit: string;
-}): Promise<string> => {
+}): Promise<string> {
   const builds = await Build.query()
     .distinctOn("builds.name", "builds.projectId")
     .joinRelated("compareScreenshotBucket")
@@ -44,45 +44,45 @@ export const getCommentBody = async (props: {
 
   const hasMultipleProjects =
     new Set(builds.map((build) => build.projectId)).size > 1;
-  const aggregateStatuses = await Build.getAggregatedBuildStatuses(builds);
-  const buildRows = await Promise.all(
-    builds
-      // Filter out builds that are not associated with a project that has PR comments enabled.
-      // We don't filter at SQL level because we still want to know if it's linked to multiple projects
-      // to determine if we should prepend the project name to the build name.
-      .filter((build) => {
-        invariant(build.project, "Relation `project` should be fetched");
-        return build.project.prCommentEnabled;
-      })
-      .map(async (build, index) => {
-        invariant(build.project, "Relation `project` should be fetched");
+  const [aggregateStatuses, urls] = await Promise.all([
+    Build.getAggregatedBuildStatuses(builds),
+    Promise.all(builds.map((build) => build.getUrl())),
+  ]);
+  const buildRows = builds
+    // Filter out builds that are not associated with a project that has PR comments enabled.
+    // We don't filter at SQL level because we still want to know if it's linked to multiple projects
+    // to determine if we should prepend the project name to the build name.
+    .filter((build) => {
+      invariant(build.project, "Relation `project` should be fetched");
+      return build.project.prCommentEnabled;
+    })
+    .map((build, index) => {
+      invariant(build.project, "Relation `project` should be fetched");
 
-        const [stats, url] = await Promise.all([
-          getStatsMessage(build.id),
-          build.getUrl(),
-        ]);
+      const url = urls[index];
+      invariant(url, "missing build URL");
 
-        const status = aggregateStatuses[index];
-        invariant(status, "unknown build status");
+      const status = aggregateStatuses[index];
+      invariant(status, "missing build status");
 
-        const label = getBuildLabel(build.type, status);
-        const review =
-          status === "changes-detected" ? ` ([Review](${url}))` : "";
-        const name = getBuildName({
-          build,
-          project: hasMultipleProjects ? build.project : null,
-        });
+      const stats = build.stats ? getStatsMessage(build.stats) : null;
 
-        return `| **${name}** ([Inspect](${url})) | ${label}${review} | ${
-          stats || "-"
-        } | ${formatDate(build.updatedAt)} |`;
-      }),
-  );
+      const label = getBuildLabel(build.type, status);
+      const review = status === "changes-detected" ? ` ([Review](${url}))` : "";
+      const name = getBuildName({
+        build,
+        project: hasMultipleProjects ? build.project : null,
+      });
+
+      return `| **${name}** ([Inspect](${url})) | ${label}${review} | ${
+        stats || "-"
+      } | ${formatDate(build.updatedAt)} |`;
+    });
   return [
     getCommentHeader(),
     "",
     `| Build | Status | Details | Updated (UTC) |`,
     `| :---- | :----- | :------ | :------------ |`,
     ...buildRows.sort(),
   ].join("\n");
-};
+}

apps/backend/src/build-notification/notification.ts

@@ -123,23 +123,28 @@ export function getNotificationStates(input: {
   }
 }
 
+function getBuildStatsMessage(build: Build): string {
+  invariant(build.stats, "Build should be concluded");
+  return getStatsMessage(build.stats);
+}
+
 /**
  * Get the notification description for each platform based on the build
  * notification type and if it's a auto-approved build.
  */
-async function getNotificationDescription(input: {
+function getNotificationDescription(input: {
   buildNotificationType: BuildNotification["type"];
-  buildId: string;
+  build: Build;
   isAutoApproved: boolean;
-}): Promise<string> {
-  const { buildNotificationType, buildId, isAutoApproved } = input;
+}): string {
+  const { buildNotificationType, build, isAutoApproved } = input;
   switch (buildNotificationType) {
     case "queued":
       return "Build is queued";
     case "progress":
       return "Build in progress...";
     case "no-diff-detected": {
-      const statsMessage = await getStatsMessage(buildId);
+      const statsMessage = getBuildStatsMessage(build);
       if (!statsMessage) {
         if (isAutoApproved) {
           return "Auto-approved, no changes found";
@@ -152,18 +157,18 @@ async function getNotificationDescription(input: {
       return `${statsMessage} — no changes found`;
     }
     case "diff-detected": {
-      const statsMessage = await getStatsMessage(buildId);
+      const statsMessage = getBuildStatsMessage(build);
       if (isAutoApproved) {
         return `${statsMessage}, automatically approved`;
       }
       return `${statsMessage} — waiting for your decision`;
     }
     case "diff-accepted": {
-      const statsMessage = await getStatsMessage(buildId);
+      const statsMessage = getBuildStatsMessage(build);
       return `${statsMessage} — approved`;
     }
     case "diff-rejected": {
-      const statsMessage = await getStatsMessage(buildId);
+      const statsMessage = getBuildStatsMessage(build);
       return `${statsMessage} — rejected`;
     }
     default: {
@@ -179,18 +184,16 @@ export async function getNotificationPayload(input: {
   buildNotification: Pick<BuildNotification, "type">;
   build: Build;
 }): Promise<NotificationPayload> {
-  const [description, context] = await Promise.all([
-    getNotificationDescription({
-      buildNotificationType: input.buildNotification.type,
-      buildId: input.build.id,
-      isAutoApproved: input.build.type === "reference",
-    }),
-    getStatusContext(input.build),
-  ]);
+  const description = getNotificationDescription({
+    buildNotificationType: input.buildNotification.type,
+    build: input.build,
+    isAutoApproved: input.build.type === "reference",
+  });
   const states = getNotificationStates({
     buildNotificationType: input.buildNotification.type,
     isAutoApproved: input.build.type === "reference",
   });
+  const context = await getStatusContext(input.build);
 
   return {
     description,

apps/backend/src/build/concludeBuild.ts

@@ -13,8 +13,8 @@ export async function concludeBuild(input: { buildId: string }) {
   const { buildId } = input;
   const statuses = await Build.getScreenshotDiffsStatuses([buildId]);
   const [[conclusion], [stats]] = await Promise.all([
-    Build.getConclusions([buildId], statuses),
-    Build.getStats([buildId]),
+    Build.computeConclusions([buildId], statuses),
+    Build.computeStats([buildId]),
   ]);
   invariant(stats !== undefined, "No stats found");
   invariant(conclusion !== undefined, "No conclusion found");

apps/backend/src/build/stats.ts

@@ -1,15 +1,11 @@
-import { invariant } from "@argos/util/invariant";
-
 import { Build } from "@/database/models/index.js";
 
 /**
  * Get a message for the build stats.
  * Example: 40 changed, 20 added, 10 removed, 5 failures
  * Around 45 characters max.
  */
-export async function getStatsMessage(buildId: string): Promise<string> {
-  const [stats] = await Build.getStats([buildId]);
-  invariant(stats, "Build stats not found");
+export function getStatsMessage(stats: NonNullable<Build["stats"]>): string {
   const parts = [];
   if (stats.changed > 0) {
     parts.push(`${stats.changed} changed`);