feat: cache conclusion and stats in database

apps/backend/db/migrations/20241231154644_build-status.js

@@ -0,0 +1,19 @@
+/**
+ * @param {import('knex').Knex} knex
+ */
+export const up = async (knex) => {
+  await knex.schema.alterTable("builds", async (table) => {
+    table.enum("conclusion", ["no-changes", "changes-detected"]);
+    table.jsonb("stats");
+  });
+};
+
+/**
+ * @param {import('knex').Knex} knex
+ */
+export const down = async (knex) => {
+  await knex.schema.alterTable("builds", async (table) => {
+    table.dropColumn("conclusion");
+    table.dropColumn("stats");
+  });
+};

apps/backend/db/structure.sql

@@ -226,7 +226,10 @@ CREATE TABLE public.builds (
     metadata jsonb,
     "baseBranchResolvedFrom" text,
     "parentCommits" jsonb,
+    conclusion text,
+    stats jsonb,
     CONSTRAINT "builds_baseBranchResolvedFrom_check" CHECK (("baseBranchResolvedFrom" = ANY (ARRAY['user'::text, 'pull-request'::text, 'project'::text]))),
+    CONSTRAINT builds_conclusion_check CHECK ((conclusion = ANY (ARRAY['no-changes'::text, 'changes-detected'::text]))),
     CONSTRAINT builds_mode_check CHECK ((mode = ANY (ARRAY['ci'::text, 'monitoring'::text]))),
     CONSTRAINT builds_type_check CHECK ((type = ANY (ARRAY['reference'::text, 'check'::text, 'orphan'::text])))
 );
@@ -2919,4 +2922,5 @@ INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('2024120
 INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241201100628_remove-github-access-token.js', 1, NOW());
 INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241201210158_google-account.js', 1, NOW());
 INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241201222408_gitlab-last-loggedat.js', 1, NOW());
-INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241215091232_project-default-role.js', 1, NOW());
+INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241215091232_project-default-role.js', 1, NOW());
+INSERT INTO public.knex_migrations(name, batch, migration_time) VALUES ('20241231154644_build-status.js', 1, NOW());

apps/backend/src/api/schema/primitives/build.ts

@@ -46,13 +46,13 @@ function getBuildNotificationTypeFromBuildStatus(
       return "diff-accepted";
     case "rejected":
       return "diff-rejected";
-    case "diffDetected":
+    case "changes-detected":
       return "diff-detected";
     case "pending":
       return "queued";
     case "progress":
       return "progress";
-    case "stable":
+    case "no-changes":
       return "no-diff-detected";
     default:
       return null;

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

@@ -66,7 +66,8 @@ export const getCommentBody = async (props: {
         invariant(status, "unknown build status");
 
         const label = getBuildLabel(build.type, status);
-        const review = status === "diffDetected" ? ` ([Review](${url}))` : "";
+        const review =
+          status === "changes-detected" ? ` ([Review](${url}))` : "";
         const name = getBuildName({
           build,
           project: hasMultipleProjects ? build.project : null,

apps/backend/src/build/concludeBuild.ts

@@ -0,0 +1,49 @@
+import { assertNever } from "@argos/util/assertNever";
+import { invariant } from "@argos/util/invariant";
+
+import { job as buildNotificationJob } from "@/build-notification/job.js";
+import { transaction } from "@/database/index.js";
+import { Build, BuildConclusion, BuildNotification } from "@/database/models";
+
+/**
+ * Concludes the build by updating the conclusion and the stats.
+ * Called when all diffs are processed.
+ */
+export async function concludeBuild(input: { build: Build }) {
+  const statuses = await Build.getStatuses([input.build]);
+  const [[conclusion], [stats]] = await Promise.all([
+    Build.getConclusions([input.build.id], statuses),
+    Build.getStats([input.build.id]),
+  ]);
+  invariant(stats !== undefined, "No stats found");
+  invariant(conclusion !== undefined, "No conclusion found");
+  // If the build is not yet concluded, we don't want to update it.
+  if (conclusion === null) {
+    return;
+  }
+  const [, buildNotification] = await transaction(async (trx) => {
+    return Promise.all([
+      Build.query(trx).findById(input.build.id).patch({
+        conclusion,
+        stats,
+      }),
+      BuildNotification.query(trx).insert({
+        buildId: input.build.id,
+        type: getNotificationType(conclusion),
+        jobStatus: "pending",
+      }),
+    ]);
+  });
+  await buildNotificationJob.push(buildNotification.id);
+}
+
+function getNotificationType(conclusion: BuildConclusion) {
+  switch (conclusion) {
+    case "changes-detected":
+      return "diff-detected";
+    case "no-changes":
+      return "no-diff-detected";
+    default:
+      return assertNever(conclusion);
+  }
+}

apps/backend/src/build/createBuild.ts

@@ -168,10 +168,7 @@ export async function createBuild(params: {
     },
   );
 
-  await pushBuildNotification({
-    buildId: build.id,
-    type: "queued",
-  });
+  await pushBuildNotification({ buildId: build.id, type: "queued" });
 
   return build;
 }

apps/backend/src/build/job.ts

@@ -8,19 +8,18 @@ import { createModelJob, UnretryableError } from "@/job-core/index.js";
 import { job as screenshotDiffJob } from "@/screenshot-diff/index.js";
 import { updateStripeUsage } from "@/stripe/index.js";
 
+import { concludeBuild } from "./concludeBuild.js";
 import { createBuildDiffs } from "./createBuildDiffs.js";
 
 /**
  * Pushes the diffs to the screenshot-diff job queue.
  * If there is no diff to proceed, it pushes a notification.
  */
-async function pushDiffs(buildId: string, screenshotDiffs: ScreenshotDiff[]) {
-  if (screenshotDiffs.length === 0) {
-    await pushBuildNotification({ buildId, type: "no-diff-detected" });
-    return;
-  }
-
-  const toProcessedDiffIds = screenshotDiffs
+async function pushDiffs(input: {
+  build: Build;
+  screenshotDiffs: ScreenshotDiff[];
+}) {
+  const toProcessedDiffIds = input.screenshotDiffs
     .filter(({ jobStatus }) => jobStatus !== "complete")
     .map(({ id }) => id);
 
@@ -29,18 +28,7 @@ async function pushDiffs(buildId: string, screenshotDiffs: ScreenshotDiff[]) {
     return;
   }
 
-  await ScreenshotDiff.fetchGraph(screenshotDiffs, "compareScreenshot");
-  const statuses = await Promise.all(
-    screenshotDiffs.map((screenshotDiff) => screenshotDiff.$getDiffStatus()),
-  );
-
-  const type = statuses.some(
-    (status) => status === "added" || status === "removed",
-  )
-    ? ("diff-detected" as const)
-    : ("no-diff-detected" as const);
-
-  await pushBuildNotification({ buildId, type });
+  await concludeBuild({ build: input.build });
 }
 
 /**
@@ -109,8 +97,8 @@ export async function performBuild(build: Build) {
       .withGraphFetched("[gitlabProject, account]")
       .throwIfNotFound(),
     pushBuildNotification({ buildId: build.id, type: "progress" }),
-    createBuildDiffs(build).then(async (diffs) => {
-      pushDiffs(build.id, diffs);
+    createBuildDiffs(build).then(async (screenshotDiffs) => {
+      await pushDiffs({ build, screenshotDiffs });
     }),
   ]);
 

apps/backend/src/build/label.ts

@@ -11,7 +11,7 @@ function getBuildStatusLabel(status: BuildAggregatedStatus): string {
       return "👍 Changes approved";
     case "aborted":
       return "🙅 Build aborted";
-    case "diffDetected":
+    case "changes-detected":
       return "⚠️ Changes detected";
     case "error":
       return "❌ An error happened";
@@ -23,7 +23,7 @@ function getBuildStatusLabel(status: BuildAggregatedStatus): string {
       return "🚜 Diffing screenshots";
     case "rejected":
       return "👎 Changes rejected";
-    case "stable":
+    case "no-changes":
       return "✅ No changes detected";
     default:
       assertNever(status);

apps/backend/src/database/models/Build.ts

@@ -42,11 +42,11 @@ const BuildStatusSchema = z.enum([
 ]);
 type BuildStatus = z.infer<typeof BuildStatusSchema>;
 
-const BuildConclusionSchema = z.enum(["stable", "diffDetected"]);
-type BuildConclusion = z.infer<typeof BuildConclusionSchema> | null;
+const BuildConclusionSchema = z.enum(["no-changes", "changes-detected"]);
+export type BuildConclusion = z.infer<typeof BuildConclusionSchema>;
 
 const BuildReviewStatusSchema = z.enum(["accepted", "rejected"]);
-type BuildReviewStatus = z.infer<typeof BuildReviewStatusSchema> | null;
+type BuildReviewStatus = z.infer<typeof BuildReviewStatusSchema>;
 
 export const BuildAggregatedStatusSchema = z.union([
   BuildReviewStatusSchema,
@@ -121,6 +121,38 @@ export class Build extends Model {
       metadata: {
         oneOf: [BuildMetadataJsonSchema, { type: "null" }],
       },
+      conclusion: {
+        oneOf: [
+          { type: "null" },
+          { type: "string", enum: ["no-changes", "changes-detected"] },
+        ],
+      },
+      stats: {
+        oneOf: [
+          { type: "null" },
+          {
+            type: "object",
+            properties: {
+              failure: { type: "integer" },
+              added: { type: "integer" },
+              unchanged: { type: "integer" },
+              changed: { type: "integer" },
+              removed: { type: "integer" },
+              total: { type: "integer" },
+              retryFailure: { type: "integer" },
+            },
+            required: [
+              "failure",
+              "added",
+              "unchanged",
+              "changed",
+              "removed",
+              "total",
+              "retryFailure",
+            ],
+          },
+        ],
+      },
     },
   });
 
@@ -148,6 +180,8 @@ export class Build extends Model {
   runAttempt!: number | null;
   partial!: boolean | null;
   metadata!: BuildMetadata | null;
+  conclusion!: BuildConclusion | null;
+  stats!: BuildStats | null;
 
   static override get relationMappings(): RelationMappings {
     return {
@@ -352,7 +386,7 @@ export class Build extends Model {
   static async getConclusions(
     buildIds: string[],
     statuses: BuildStatus[],
-  ): Promise<BuildConclusion[]> {
+  ): Promise<(BuildConclusion | null)[]> {
     const completeBuildIds = buildIds.filter(
       (_, index) => statuses[index] === "complete",
     );
@@ -379,9 +413,9 @@ export class Build extends Model {
         (diff) => diff.buildId === buildId,
       );
       if (buildDiffCount && buildDiffCount.count > 0) {
-        return "diffDetected";
+        return "changes-detected";
       }
-      return "stable";
+      return "no-changes";
     });
   }
 
@@ -390,10 +424,10 @@ export class Build extends Model {
    */
   static async getReviewStatuses(
     buildIds: string[],
-    conclusions: BuildConclusion[],
-  ): Promise<BuildReviewStatus[]> {
+    conclusions: (BuildConclusion | null)[],
+  ): Promise<(BuildReviewStatus | null)[]> {
     const diffDetectedBuildIds = buildIds.filter(
-      (_buildId, index) => conclusions[index] === "diffDetected",
+      (_buildId, index) => conclusions[index] === "changes-detected",
     );
 
     const screenshotDiffs = diffDetectedBuildIds.length
@@ -404,7 +438,7 @@ export class Build extends Model {
       : [];
 
     return buildIds.map((buildId, index) => {
-      if (conclusions[index] !== "diffDetected") {
+      if (conclusions[index] !== "changes-detected") {
         return null;
       }
 
@@ -449,14 +483,17 @@ export class Build extends Model {
     return `${config.get("server.url")}${pathname}`;
   }
 
-  static async getAggregatedBuildStatuses(builds: Build[]) {
+  static async getAggregatedBuildStatuses(
+    builds: Build[],
+  ): Promise<BuildAggregatedStatus[]> {
     const buildIds = builds.map((build) => build.id);
     const statuses = await Build.getStatuses(builds);
     const conclusions = await Build.getConclusions(buildIds, statuses);
     const reviewStatuses = await Build.getReviewStatuses(buildIds, conclusions);
-    return builds.map(
-      (_build, index) =>
-        reviewStatuses[index] || conclusions[index] || statuses[index],
-    ) as BuildAggregatedStatus[];
+    return builds.map((_build, index) => {
+      const status =
+        reviewStatuses[index] || conclusions[index] || statuses[index];
+      return BuildAggregatedStatusSchema.parse(status);
+    });
   }
 }