fix: reduce perf comparator false positives

packages/app/script/compare-perf.ts

@@ -2,8 +2,10 @@ import fs from "node:fs/promises"
 import path from "node:path"
 import {
   comparePerfBaselines,
+  perfFailureKey,
   renderPerfBaselineComment,
   type PerfBaselineComparison,
+  type PerfFailureKey,
   type PerfScenarioSummary,
 } from "../src/testing/perf-metrics"
 
@@ -23,14 +25,37 @@ async function readPerfFile(filePath: string) {
   return payload
 }
 
-async function readFailureScenarioKeys(filePath: string) {
+async function readFailureScope(filePath: string): Promise<{ scenarioKeys: string[]; failureKeys: PerfFailureKey[] }> {
   const payload = JSON.parse(await fs.readFile(filePath, "utf8")) as PerfBaselineComparison
   if (!Array.isArray(payload.scenarios)) {
     throw new Error(`Expected a perf comparison with scenarios in ${filePath}`)
   }
-  return payload.scenarios
-    .filter((scenario) => scenario.failures.length > 0)
-    .map((scenario) => `${scenario.profile}:${scenario.scenario}`)
+  if (!Array.isArray(payload.failures)) {
+    throw new Error(`Expected a perf comparison with failures in ${filePath}`)
+  }
+  const scenarioKeys = new Set<string>()
+  const failureKeys = new Set<PerfFailureKey>()
+
+  for (const failure of payload.failures) {
+    if (failure.startsWith("missing_base_scenario:")) {
+      scenarioKeys.add(failure.slice("missing_base_scenario:".length))
+    } else if (failure.startsWith("missing_head_scenario:")) {
+      scenarioKeys.add(failure.slice("missing_head_scenario:".length))
+    }
+  }
+
+  for (const scenario of payload.scenarios) {
+    if (scenario.failures.length === 0) continue
+    scenarioKeys.add(`${scenario.profile}:${scenario.scenario}`)
+    for (const failure of scenario.failures) {
+      failureKeys.add(perfFailureKey({ profile: scenario.profile, scenario: scenario.scenario, metric: failure }))
+    }
+  }
+
+  return {
+    scenarioKeys: [...scenarioKeys],
+    failureKeys: [...failureKeys],
+  }
 }
 
 async function inferFailureScenarioSource(input: { outputPath?: string; failuresFromPath?: string }) {
@@ -59,12 +84,17 @@ async function main() {
   }
 
   const failuresSourcePath = await inferFailureScenarioSource({ outputPath, failuresFromPath })
-  const [base, head, scenarioKeys] = await Promise.all([
+  const [base, head, failureScope] = await Promise.all([
     readPerfFile(basePath),
     readPerfFile(headPath),
-    failuresSourcePath ? readFailureScenarioKeys(failuresSourcePath) : undefined,
+    failuresSourcePath ? readFailureScope(failuresSourcePath) : undefined,
   ])
-  const comparison = comparePerfBaselines({ base, head, scenarioKeys })
+  const comparison = comparePerfBaselines({
+    base,
+    head,
+    scenarioKeys: failureScope?.scenarioKeys,
+    failureKeys: failureScope?.failureKeys,
+  })
 
   if (outputPath) {
     await fs.mkdir(path.dirname(outputPath), { recursive: true })
@@ -79,6 +109,7 @@ async function main() {
     pass: comparison.pass,
     failures: comparison.failures,
     warnings: comparison.warnings,
+    confirmation: comparison.confirmation,
   }
   console.log(JSON.stringify(summary, null, 2))
 

packages/app/src/testing/compare-perf-script.test.ts

@@ -0,0 +1,172 @@
+import { afterEach, describe, expect, test } from "bun:test"
+import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"
+import { tmpdir } from "node:os"
+import { join } from "node:path"
+import type { PerfBaselineComparison, PerfProfile, PerfScenarioSummary } from "./perf-metrics"
+
+const tempRoots: string[] = []
+
+afterEach(async () => {
+  await Promise.all(tempRoots.splice(0).map((root) => rm(root, { recursive: true, force: true })))
+})
+
+function scenario(input: {
+  branch: string
+  profile?: PerfProfile
+  scenario?: string
+  interaction?: number
+  frameMax?: number
+}): PerfScenarioSummary {
+  const interaction = input.interaction ?? 40
+  return {
+    branch: input.branch,
+    profile: input.profile ?? "default",
+    scenario: input.scenario ?? "session-scroll-reading",
+    runs: 3,
+    interaction_ms_median: interaction,
+    interaction_ms_worst: interaction,
+    interaction_ms: interaction,
+    interaction_delay_ms: 0,
+    long_task_count: 0,
+    long_task_max_ms: 0,
+    tbt_ms: 0,
+    frame_gap_p95_ms: 16,
+    frame_gap_max_ms: input.frameMax ?? 16,
+    jank_count_50ms: 0,
+    cls: 0,
+    window_ms: 900,
+    run_details: [],
+  }
+}
+
+async function writeJson(filePath: string, payload: unknown) {
+  await writeFile(filePath, `${JSON.stringify(payload, null, 2)}\n`)
+}
+
+describe("compare-perf script", () => {
+  test("confirms only the initial metric failure key from failures-from", async () => {
+    const root = await mkdtemp(join(tmpdir(), "compare-perf-script-"))
+    tempRoots.push(root)
+    const basePath = join(root, "base.json")
+    const headPath = join(root, "head.json")
+    const initialPath = join(root, "perf-compare.json")
+    const outputPath = join(root, "perf-compare-confirm.json")
+    const initialComparison: PerfBaselineComparison = {
+      pass: false,
+      failures: ["default:session-scroll-reading:interaction_ms_median"],
+      warnings: [],
+      scenarios: [
+        {
+          profile: "default",
+          scenario: "session-scroll-reading",
+          pass: false,
+          failures: ["interaction_ms_median"],
+          warnings: [],
+          base: scenario({ branch: "base", interaction: 48 }),
+          head: scenario({ branch: "head", interaction: 76 }),
+        },
+      ],
+    }
+
+    await writeJson(basePath, [scenario({ branch: "base", interaction: 32, frameMax: 16 })])
+    await writeJson(headPath, [scenario({ branch: "head", interaction: 40, frameMax: 120 })])
+    await writeJson(initialPath, initialComparison)
+
+    const child = Bun.spawn(
+      [
+        process.execPath,
+        "script/compare-perf.ts",
+        "--base",
+        basePath,
+        "--head",
+        headPath,
+        "--output",
+        outputPath,
+        "--failures-from",
+        initialPath,
+      ],
+      {
+        cwd: process.cwd(),
+        stderr: "pipe",
+        stdout: "pipe",
+      },
+    )
+    const [exitCode, stdout, stderr] = await Promise.all([
+      child.exited,
+      new Response(child.stdout).text(),
+      new Response(child.stderr).text(),
+    ])
+
+    expect(stderr).toBe("")
+    expect(exitCode).toBe(0)
+    const summary = JSON.parse(stdout) as Pick<PerfBaselineComparison, "confirmation" | "failures" | "pass">
+    expect(summary.pass).toBe(true)
+    expect(summary.failures).toHaveLength(0)
+    expect(summary.confirmation).toEqual({
+      initialFailureKeys: ["default:session-scroll-reading:interaction_ms_median"],
+      rawConfirmedFailures: ["default:session-scroll-reading:frame_gap_max_ms_delta"],
+      intersectedFailures: [],
+    })
+    const comparison = JSON.parse(await readFile(outputPath, "utf8")) as PerfBaselineComparison
+    expect(comparison.pass).toBe(true)
+    expect(comparison.failures).toHaveLength(0)
+    expect(comparison.confirmation).toEqual({
+      initialFailureKeys: ["default:session-scroll-reading:interaction_ms_median"],
+      rawConfirmedFailures: ["default:session-scroll-reading:frame_gap_max_ms_delta"],
+      intersectedFailures: [],
+    })
+  })
+
+  test("keeps top-level missing scenario failures in the confirmation scope", async () => {
+    const root = await mkdtemp(join(tmpdir(), "compare-perf-script-"))
+    tempRoots.push(root)
+    const basePath = join(root, "base.json")
+    const headPath = join(root, "head.json")
+    const initialPath = join(root, "perf-compare.json")
+    const outputPath = join(root, "perf-compare-confirm.json")
+    const initialComparison: PerfBaselineComparison = {
+      pass: false,
+      failures: ["missing_head_scenario:default:session-scroll-reading"],
+      warnings: [],
+      scenarios: [],
+    }
+
+    await writeJson(basePath, [scenario({ branch: "base" })])
+    await writeJson(headPath, [])
+    await writeJson(initialPath, initialComparison)
+
+    const child = Bun.spawn(
+      [
+        process.execPath,
+        "script/compare-perf.ts",
+        "--base",
+        basePath,
+        "--head",
+        headPath,
+        "--output",
+        outputPath,
+        "--failures-from",
+        initialPath,
+      ],
+      {
+        cwd: process.cwd(),
+        stderr: "pipe",
+        stdout: "pipe",
+      },
+    )
+    const [exitCode, stdout, stderr] = await Promise.all([
+      child.exited,
+      new Response(child.stdout).text(),
+      new Response(child.stderr).text(),
+    ])
+
+    expect(stderr).toBe("")
+    expect(exitCode).toBe(1)
+    const summary = JSON.parse(stdout) as Pick<PerfBaselineComparison, "failures" | "pass">
+    expect(summary.pass).toBe(false)
+    expect(summary.failures).toEqual(["missing_head_scenario:default:session-scroll-reading"])
+    const comparison = JSON.parse(await readFile(outputPath, "utf8")) as PerfBaselineComparison
+    expect(comparison.pass).toBe(false)
+    expect(comparison.failures).toEqual(["missing_head_scenario:default:session-scroll-reading"])
+  })
+})