chore(build-cli): reshape bundleSize library to pure-function primitives

build-tools/packages/build-cli/src/commands/check/bundleSize.ts

@@ -3,36 +3,73 @@
  * Licensed under the MIT License.
  */
 
+import { execFileSync } from "node:child_process";
 import { Flags } from "@oclif/core";
+
+import { getArtifactForCommit } from "../../library/azureDevops/getArtifactForCommit.js";
 import { getAzureDevopsApi } from "../../library/azureDevops/getAzureDevopsApi.js";
 import {
-	ADOSizeComparator,
-	type BundleComparison,
-	bundlesContainNoChanges,
-	pickFreshestCanonicalRemote,
+	checkLocalBundleAnalysisExists,
+	compareJsonReportsByPackage,
+	extractAnalyzerJsonsFromArtifact,
+	type PackageComparison,
+	readAnalyzerJsonsFromFileSystem,
 } from "../../library/bundleSize/index.js";
-
 import { BaseCommand } from "../../library/commands/base.js";
+import { pickFreshestRemote } from "../../library/git/pickFreshestRemote.js";
 
-// Must match the "public" project + build-bundle-size-artifacts.yml (definitionId 48).
-const adoConstants = {
-	orgUrl: "https://dev.azure.com/fluidframework",
-	projectName: "public",
-	ciBuildDefinitionId: 48,
-	artifactName: "bundleAnalyzerJson",
-} as const;
-
-// Where `flub generate bundleStats` (via `npm run bundle-analysis:collect`) writes.
+// Where `flub generate bundleStats` (via `pnpm bundle-analysis:collect`) writes.
 const defaultLocalReportPath = "./artifacts/bundleAnalyzerJson";
 
 /**
  * Result serialized to stdout by `--json`. Default invocations print a
  * human-readable summary instead.
  */
-type CheckBundleSizeResult =
-	| { kind: "no-changes"; baselineCommit: string }
-	| { kind: "changes"; baselineCommit: string; comparison: BundleComparison[] }
-	| { kind: "error"; baselineCommit: string | undefined; error: string };
+interface CheckBundleSizeResult {
+	baselineCommit: string;
+	comparison: PackageComparison;
+}
+
+/**
+ * Render a {@link PackageComparison} as a flat list of human-readable lines.
+ * Skips packages whose bundles all have zero deltas.
+ *
+ * @returns The rendered lines, or an empty array when nothing changed across
+ * the whole comparison.
+ */
+function formatComparison(comparison: PackageComparison): string[] {
+	const fmt = (before: number, after: number): string => {
+		const delta = after - before;
+		const sign = delta > 0 ? "+" : "";
+		return `${before} -> ${after} (${sign}${delta})`;
+	};
+
+	const lines: string[] = [];
+	for (const [sourcePackage, bundles] of Object.entries(comparison)) {
+		const bundleLines: string[] = [];
+		for (const [bundleName, { base, compare }] of Object.entries(bundles)) {
+			if (base === undefined && compare !== undefined) {
+				bundleLines.push(
+					`    ${bundleName}: added (parsed ${compare.parsedSize}, gzip ${compare.gzipSize})`,
+				);
+			} else if (compare === undefined && base !== undefined) {
+				bundleLines.push(
+					`    ${bundleName}: removed (was parsed ${base.parsedSize}, gzip ${base.gzipSize})`,
+				);
+			} else if (base !== undefined && compare !== undefined) {
+				const parsedChanged = base.parsedSize !== compare.parsedSize;
+				const gzipChanged = base.gzipSize !== compare.gzipSize;
+				if (!parsedChanged && !gzipChanged) continue;
+				bundleLines.push(
+					`    ${bundleName}: parsed ${fmt(base.parsedSize, compare.parsedSize)}, gzip ${fmt(base.gzipSize, compare.gzipSize)}`,
+				);
+			}
+		}
+		if (bundleLines.length === 0) continue;
+		lines.push(`  ${sourcePackage}:`, ...bundleLines);
+	}
+	return lines;
+}
 
 export default class CheckBundleSize extends BaseCommand<typeof CheckBundleSize> {
 	static readonly description =
@@ -59,76 +96,86 @@ export default class CheckBundleSize extends BaseCommand<typeof CheckBundleSize>
 
 		// Auto-detect targets `main` on the canonical remote; `--target <ref>` overrides.
 		const branch = "main";
+		const canonicalUrl = /(^|[/:])microsoft\/fluidframework(\.git)?$/i;
 		let targetRef: string;
 		if (target !== undefined) {
 			targetRef = target;
 			this.log(`Using explicit target ref ${target}.`);
 		} else {
-			const remote = pickFreshestCanonicalRemote(branch) ?? "origin";
+			const remote = pickFreshestRemote(branch, (url) => canonicalUrl.test(url));
+			if (remote === undefined) {
+				this.error(
+					"Could not auto-detect a canonical remote. Add a remote pointing at microsoft/FluidFramework, or pass --target <ref> to override.",
+				);
+			}
 			targetRef = `${remote}/${branch}`;
 			this.log(`Using target ref ${targetRef}. Pass --target <ref> to override.`);
 		}
 
-		// Anonymous reads work for the public ADO project at this command's scale;
-		// automated consumers authenticate at the library layer.
-		const adoApi = getAzureDevopsApi(undefined, adoConstants.orgUrl);
-		const sizeComparator = new ADOSizeComparator(
-			adoConstants,
+		let baselineCommit: string;
+		try {
+			baselineCommit = execFileSync("git", ["merge-base", targetRef, "HEAD"], {
+				stdio: ["ignore", "pipe", "pipe"],
+			})
+				.toString()
+				.trim();
+		} catch (e) {
+			const stderr = (e as { stderr?: Buffer }).stderr?.toString().trim();
+			this.error(
+				`Could not determine merge-base for ref "${targetRef}". Ensure it is fetched locally, or pass --target <ref> to override.${
+					stderr ? `\n${stderr}` : ""
+				}`,
+			);
+		}
+		this.log(`Baseline commit: ${baselineCommit}`);
+
+		// Public ADO project — anonymous reads are fine at this command's scale.
+		const adoApi = getAzureDevopsApi(undefined, "https://dev.azure.com/fluidframework");
+		const artifactContents = await getArtifactForCommit({
 			adoApi,
-			localReportPath,
-			targetRef,
-		);
-		const comparisonResult = await sizeComparator.getSizeComparison();
-
-		if (comparisonResult.kind === "error") {
-			this.warning(comparisonResult.error);
-			return {
-				kind: "error",
-				baselineCommit: comparisonResult.baselineCommit,
-				error: comparisonResult.error,
-			};
+			// Published by the `Build - Client bundle size artifacts` pipeline.
+			artifactName: "bundleAnalyzerJson",
+			commit: baselineCommit,
+			// `Build - Client bundle size artifacts` in the `public` project.
+			// Source-of-truth: tools/pipelines/build-bundle-size-artifacts.yml.
+			definitionId: 48,
+			project: "public",
+		});
+
+		const baselineJsons = extractAnalyzerJsonsFromArtifact(artifactContents);
+		if (baselineJsons.size === 0) {
+			this.error(
+				`Baseline artifact contains no analyzer.json files for commit ${baselineCommit}.`,
+			);
 		}
 
-		if (comparisonResult.comparison.length === 0) {
-			const message =
-				"No bundles to compare — baseline artifact or local bundle reports are empty.";
-			this.warning(message);
-			return {
-				kind: "error",
-				baselineCommit: comparisonResult.baselineCommit,
-				error: message,
-			};
+		const localCheck = checkLocalBundleAnalysisExists(localReportPath);
+		// Append the `pnpm bundle-analysis:collect` hint only on the default
+		// path — overrides are populated from some source we don't know about.
+		const hint =
+			localReportPath === defaultLocalReportPath
+				? " Run `pnpm bundle-analysis:collect` to populate it."
+				: "";
+		if (localCheck === "missing") {
+			this.error(`Local bundle report directory not found at "${localReportPath}".${hint}`);
+		}
+		if (localCheck === "noAnalyzerJson") {
+			this.error(
+				`Local bundle report directory "${localReportPath}" contains no analyzer.json files.${hint}`,
+			);
 		}
+		const compareJsons = await readAnalyzerJsonsFromFileSystem(localReportPath);
 
-		const { baselineCommit, comparison } = comparisonResult;
+		const comparison = compareJsonReportsByPackage(baselineJsons, compareJsons);
+		const changeLines = formatComparison(comparison);
 
-		if (bundlesContainNoChanges(comparison)) {
+		if (changeLines.length === 0) {
 			this.log(`No bundle size changes vs baseline commit ${baselineCommit}.`);
-			return { kind: "no-changes", baselineCommit };
-		}
-
-		const fmt = (before: number, after: number, delta: number): string => {
-			const sign = delta > 0 ? "+" : "";
-			return `${before} -> ${after} (${sign}${delta})`;
-		};
-
-		this.log(`Bundle size changes vs baseline commit ${baselineCommit}:`);
-		for (const bundle of comparison) {
-			const changedMetrics = Object.entries(bundle.commonBundleMetrics).flatMap(
-				([metricName, { baseline, compare }]) => {
-					const parsedDelta = compare.parsedSize - baseline.parsedSize;
-					const gzipDelta = compare.gzipSize - baseline.gzipSize;
-					if (parsedDelta === 0 && gzipDelta === 0) return [];
-					return [
-						`    ${metricName}: parsed ${fmt(baseline.parsedSize, compare.parsedSize, parsedDelta)}, gzip ${fmt(baseline.gzipSize, compare.gzipSize, gzipDelta)}`,
-					];
-				},
-			);
-			if (changedMetrics.length === 0) continue;
-			this.log(`  ${bundle.bundleName}:`);
-			for (const line of changedMetrics) this.log(line);
+		} else {
+			this.log(`Bundle size changes vs baseline commit ${baselineCommit}:`);
+			for (const line of changeLines) this.log(line);
 		}
 
-		return { kind: "changes", baselineCommit, comparison };
+		return { baselineCommit, comparison };
 	}
 }

build-tools/packages/build-cli/src/library/azureDevops/downloadArtifact.ts

@@ -4,6 +4,7 @@
  */
 
 import { strict as assert } from "node:assert";
+import type { IncomingMessage } from "node:http";
 import type { WebApi } from "azure-devops-node-api";
 import { unzipSync } from "fflate";
 
@@ -44,6 +45,18 @@ export async function downloadArtifact(
 		buildApi.createAcceptHeader = originalCreateAcceptHeader;
 	}
 
+	// The declared `ReadableStream` return type hides it, but the SDK actually
+	// returns an `http.IncomingMessage` and doesn't throw on non-2xx responses
+	// for this endpoint — a missing artifact comes back as a 404 with a
+	// non-zip body that would otherwise blow up inside `unzipSync` with a
+	// useless "invalid zip data" error.
+	const statusCode = (artifactStream as Partial<IncomingMessage>).statusCode;
+	if (statusCode !== undefined && (statusCode < 200 || statusCode >= 300)) {
+		throw new Error(
+			`Failed to download artifact "${artifactName}" from build ${buildId} (HTTP ${statusCode})`,
+		);
+	}
+
 	const unzipped = unzipSync(await readStreamAsUint8Array(artifactStream));
 
 	// ADO wraps the artifact contents in a top-level folder named after the artifact.

build-tools/packages/build-cli/src/library/azureDevops/getArtifactForCommit.ts

@@ -0,0 +1,134 @@
+/*!
+ * Copyright (c) Microsoft Corporation and contributors. All rights reserved.
+ * Licensed under the MIT License.
+ */
+
+import type { WebApi } from "azure-devops-node-api";
+import {
+	type Build,
+	BuildResult,
+	BuildStatus,
+} from "azure-devops-node-api/interfaces/BuildInterfaces.js";
+
+import { type ArtifactContents, downloadArtifact } from "./downloadArtifact.js";
+
+// Upper bound on builds fetched when searching for one matching a target commit.
+// ADO has no API to query builds by commit SHA, so this window size determines
+// how stale a target commit can be relative to the pipeline's recent activity
+// and still be findable.
+const recentBuildsToFetch = 100;
+
+/**
+ * Wrapper around the unwieldy positional signature of ADO's `getBuilds`.
+ */
+async function getRecentBuilds(
+	adoApi: WebApi,
+	project: string,
+	definitionId: number,
+): Promise<Build[]> {
+	const buildApi = await adoApi.getBuildApi();
+	return buildApi.getBuilds(
+		project,
+		[definitionId],
+		undefined,
+		undefined,
+		undefined,
+		undefined,
+		undefined,
+		undefined,
+		undefined,
+		undefined,
+		undefined,
+		undefined,
+		undefined,
+		undefined,
+		recentBuildsToFetch,
+	);
+}
+
+/**
+ * Find a usable build for `commit` in `builds` — one with an id, status
+ * Completed, and result Succeeded. A commit can have more than one ADO build
+ * (manual re-run, partial-success retry, …), so scan all matches rather than
+ * locking onto the first one ADO returned.
+ *
+ * @returns The build id. Throws with a human-readable message when no usable
+ * build is found, prioritizing "not yet completed" over "did not succeed"
+ * since retrying later might help.
+ */
+function findBuildIdForCommit(builds: Build[], commit: string): number {
+	const candidates = builds.filter((b) => b.sourceVersion === commit);
+
+	if (candidates.length === 0) {
+		throw new Error(`No build found for commit ${commit}`);
+	}
+
+	const usable = candidates.find(
+		(b): b is Build & { id: number } =>
+			b.id !== undefined &&
+			b.status === BuildStatus.Completed &&
+			b.result === BuildResult.Succeeded,
+	);
+	if (usable !== undefined) {
+		return usable.id;
+	}
+
+	// No usable found — report the most actionable state across the candidates.
+	// "Actively running" gets priority since the user might just need to wait;
+	// Cancelling is *not* in that bucket because it's heading toward Canceled.
+	const isActivelyRunning = (b: Build): boolean =>
+		b.status === BuildStatus.NotStarted ||
+		b.status === BuildStatus.InProgress ||
+		b.status === BuildStatus.Postponed;
+	if (candidates.some(isActivelyRunning)) {
+		throw new Error(
+			`Found an in-progress build for commit ${commit}; none have succeeded yet.`,
+		);
+	}
+	if (candidates.every((b) => b.result !== BuildResult.Succeeded)) {
+		throw new Error(`All builds for commit ${commit} have completed but none succeeded.`);
+	}
+	// Reaching here means at least one candidate Succeeded but is missing an
+	// `id` (possibly alongside other failed candidates) — an ADO state anomaly
+	// that shouldn't happen in practice, but the `id` field is typed
+	// `number | undefined` so we surface it explicitly.
+	throw new Error(`No build for commit ${commit} has a usable build id.`);
+}
+
+export interface GetArtifactForCommitArgs {
+	/** A connection to the ADO API. */
+	adoApi: WebApi;
+	/** Name of the pipeline artifact to fetch. */
+	artifactName: string;
+	/** Commit whose build to look up. */
+	commit: string;
+	/** ID of the ADO pipeline whose builds to search. */
+	definitionId: number;
+	/** The ADO project name. */
+	project: string;
+}
+
+/**
+ * Look up the build for `commit` on the given ADO pipeline and return the
+ * contents of one of its artifacts.
+ *
+ * @returns The artifact's {@link ArtifactContents}. Throws with a
+ * human-readable message when no usable build is found (missing, incomplete,
+ * failed) or the artifact can't be downloaded.
+ */
+export async function getArtifactForCommit(
+	args: GetArtifactForCommitArgs,
+): Promise<ArtifactContents> {
+	const { adoApi, artifactName, commit, definitionId, project } = args;
+
+	const builds = await getRecentBuilds(adoApi, project, definitionId);
+	const buildId = findBuildIdForCommit(builds, commit);
+
+	try {
+		return await downloadArtifact(adoApi, project, buildId, artifactName);
+	} catch (e) {
+		throw new Error(`Could not download artifact "${artifactName}" for commit ${commit}`, {
+			cause: e,
+		});
+	}
+}