refactor: centralize version comparison in pi-utils

packages/coding-agent/src/cli/update-cli.ts

@@ -10,7 +10,7 @@ import * as os from "node:os";
 import * as path from "node:path";
 import { Transform } from "node:stream";
 import { pipeline } from "node:stream/promises";
-import { $env, $which, APP_NAME, isEnoent, VERSION } from "@oh-my-pi/pi-utils";
+import { $env, $which, APP_NAME, compareVersions, isEnoent, VERSION } from "@oh-my-pi/pi-utils";
 import { $ } from "bun";
 import chalk from "chalk";
 import { theme } from "../modes/theme/theme";
@@ -498,24 +498,6 @@ async function getLatestRelease(): Promise<ReleaseInfo> {
 	};
 }
 
-/**
- * Compare semver versions. Returns:
- * - negative if a < b
- * - 0 if a == b
- * - positive if a > b
- */
-function compareVersions(a: string, b: string): number {
-	const pa = a.split(".").map(Number);
-	const pb = b.split(".").map(Number);
-
-	for (let i = 0; i < Math.max(pa.length, pb.length); i++) {
-		const na = pa[i] || 0;
-		const nb = pb[i] || 0;
-		if (na !== nb) return na - nb;
-	}
-	return 0;
-}
-
 interface BunInstallCachePruneResult {
 	scannedPackages: number;
 	removedEntries: number;

packages/coding-agent/src/utils/changelog.ts

@@ -198,9 +198,10 @@ function parseChangelogContent(content: string): ChangelogEntry[] {
 }
 
 /**
- * Compare versions. Returns: -1 if v1 < v2, 0 if v1 === v2, 1 if v1 > v2
+ * Compare changelog entries by their parsed version parts.
+ * Returns: -1 if v1 < v2, 0 if v1 === v2, 1 if v1 > v2
  */
-export function compareVersions(v1: ChangelogEntry, v2: ChangelogEntry): number {
+export function compareChangelogEntries(v1: ChangelogEntry, v2: ChangelogEntry): number {
 	if (v1.major !== v2.major) return v1.major - v2.major;
 	if (v1.minor !== v2.minor) return v1.minor - v2.minor;
 	return v1.patch - v2.patch;
@@ -232,7 +233,7 @@ export function getNewEntries(entries: ChangelogEntry[], lastVersion: string): C
 		return [];
 	}
 
-	return entries.filter(entry => compareVersions(entry, parsedLastVersion) > 0);
+	return entries.filter(entry => compareChangelogEntries(entry, parsedLastVersion) > 0);
 }
 
 /**
@@ -328,7 +329,7 @@ export async function resolveStartupChangelogForDisplay(options: {
 	}
 	if (options.mode === "hidden") {
 		const currentVersion = parseChangelogVersion(options.currentVersion);
-		if (currentVersion && compareVersions(currentVersion, parsedLastVersion) > 0) {
+		if (currentVersion && compareChangelogEntries(currentVersion, parsedLastVersion) > 0) {
 			await writeLastChangelogVersion(options.currentVersion, options.agentDir);
 		}
 		return undefined;

packages/coding-agent/src/web/scrapers/hackage.ts

@@ -1,4 +1,4 @@
-import { tryParseJson } from "@oh-my-pi/pi-utils";
+import { compareVersions, tryParseJson } from "@oh-my-pi/pi-utils";
 import type { RenderResult, SpecialHandler } from "./types";
 import { buildResult, loadPage } from "./types";
 
@@ -20,17 +20,6 @@ interface ParsedCabal {
 	stability?: string;
 }
 
-function compareVersions(a: string, b: string): number {
-	const aParts = a.split(".").map(part => Number.parseInt(part, 10) || 0);
-	const bParts = b.split(".").map(part => Number.parseInt(part, 10) || 0);
-	const max = Math.max(aParts.length, bParts.length);
-	for (let i = 0; i < max; i++) {
-		const delta = (aParts[i] || 0) - (bParts[i] || 0);
-		if (delta !== 0) return delta;
-	}
-	return 0;
-}
-
 function extractCabalField(content: string, fieldName: string): string | undefined {
 	const pattern = new RegExp(`^${fieldName}:\\s*(.*)$`, "im");
 	const match = content.match(pattern);

packages/utils/src/index.ts

@@ -34,6 +34,7 @@ export * from "./tab-spacing";
 export * from "./temp";
 export * from "./tls-fetch";
 export * from "./type-guards";
+export * from "./version";
 export * from "./which";
 
 function isPlainObject(val: object): val is Record<string, unknown> {

packages/utils/src/version.ts

@@ -0,0 +1,99 @@
+const DIGITS = /^\d+$/;
+
+/**
+ * Compare two version strings.
+ *
+ * Canonical comparator that supersedes the historical in-repo copies
+ * (update-cli, hackage scraper, release scripts):
+ * - inputs are trimmed and at most one leading `v`/`V` is stripped
+ * - dot-separated segments are compared numerically, missing trailing
+ *   segments count as 0, so `1.2` === `1.2.0` and any segment count works
+ * - a SemVer-2.0 prerelease suffix sorts before the plain release
+ *   (`1.0.0-beta` < `1.0.0`); prerelease identifiers follow SemVer order
+ *   (numeric < alphanumeric, numeric compared by value, alphanumeric
+ *   compared lexically, longer sets of equal fields win)
+ * - SemVer build metadata begins at the first `+` and does not participate
+ *   in precedence; it is stripped before core/prerelease parsing
+ * - malformed numeric segments compare as 0 (`1.2.x` === `1.2.0`)
+ * - never throws; returns only -1 | 0 | 1
+ */
+export function compareVersions(a: string, b: string): number {
+	const pa = parseVersion(a);
+	const pb = parseVersion(b);
+
+	const core = compareNumericParts(pa.core, pb.core);
+	if (core !== 0) return core;
+
+	return comparePrerelease(pa.prerelease, pb.prerelease);
+}
+
+interface ParsedVersion {
+	core: string[];
+	prerelease: string[] | null;
+}
+
+function parseVersion(version: string): ParsedVersion {
+	const trimmed = version.trim();
+	const stripped = trimmed.startsWith("v") || trimmed.startsWith("V") ? trimmed.slice(1) : trimmed;
+	const plusIndex = stripped.indexOf("+");
+	const withoutBuild = plusIndex === -1 ? stripped : stripped.slice(0, plusIndex);
+	const dashIndex = withoutBuild.indexOf("-");
+	if (dashIndex === -1) {
+		return { core: withoutBuild.split("."), prerelease: null };
+	}
+	return {
+		core: withoutBuild.slice(0, dashIndex).split("."),
+		prerelease: withoutBuild.slice(dashIndex + 1).split("."),
+	};
+}
+
+/** Compare dot-separated numeric segments; missing/malformed segments count as 0. */
+function compareNumericParts(a: string[], b: string[]): number {
+	const length = Math.max(a.length, b.length);
+	for (let i = 0; i < length; i++) {
+		// Missing or malformed segments compare as 0.
+		const sa = a[i];
+		const sb = b[i];
+		const result = compareDigits(
+			sa !== undefined && DIGITS.test(sa) ? sa : "0",
+			sb !== undefined && DIGITS.test(sb) ? sb : "0",
+		);
+		if (result !== 0) return result;
+	}
+	return 0;
+}
+
+/** Exact integer comparison of digit strings, avoiding float overflow. */
+function compareDigits(a: string, b: string): number {
+	const na = a.replace(/^0+/, "") || "0";
+	const nb = b.replace(/^0+/, "") || "0";
+	if (na.length !== nb.length) return na.length < nb.length ? -1 : 1;
+	if (na < nb) return -1;
+	if (na > nb) return 1;
+	return 0;
+}
+
+/** SemVer-2.0 prerelease ordering; null means a plain release, which wins. */
+function comparePrerelease(a: string[] | null, b: string[] | null): number {
+	if (a === null || b === null) {
+		return a === b ? 0 : a === null ? 1 : -1;
+	}
+	const length = Math.max(a.length, b.length);
+	for (let i = 0; i < length; i++) {
+		const ia = a[i];
+		const ib = b[i];
+		if (ia === undefined) return -1;
+		if (ib === undefined) return 1;
+		const aNumeric = DIGITS.test(ia);
+		const bNumeric = DIGITS.test(ib);
+		if (aNumeric && bNumeric) {
+			const result = compareDigits(ia, ib);
+			if (result !== 0) return result;
+		} else if (aNumeric !== bNumeric) {
+			return aNumeric ? -1 : 1;
+		} else if (ia !== ib) {
+			return ia < ib ? -1 : 1;
+		}
+	}
+	return 0;
+}

packages/utils/test/version.test.ts

@@ -0,0 +1,72 @@
+import { describe, expect, it } from "bun:test";
+import { compareVersions } from "../src/version";
+
+describe("compareVersions", () => {
+	it("trims whitespace and strips one leading v/V", () => {
+		expect(compareVersions(" 1.2.3 ", "1.2.3")).toBe(0);
+		expect(compareVersions("v1.2.3", "1.2.3")).toBe(0);
+		expect(compareVersions("V1.2.3", "v1.2.3")).toBe(0);
+		expect(compareVersions(" v1.2.3 ", "1.2.3")).toBe(0);
+	});
+
+	it("zero-pads missing trailing segments", () => {
+		expect(compareVersions("1.2", "1.2.0")).toBe(0);
+		expect(compareVersions("1", "1.0.0.0")).toBe(0);
+		expect(compareVersions("1.2", "1.2.1")).toBe(-1);
+		expect(compareVersions("1.2.3", "1.2")).toBe(1);
+	});
+
+	it("supports arbitrary segment counts", () => {
+		expect(compareVersions("1.2.3.4.5", "1.2.3.4.5")).toBe(0);
+		expect(compareVersions("1.2.3.4", "1.2.3.5")).toBe(-1);
+		expect(compareVersions("1.0.0.1", "1.0.0")).toBe(1);
+	});
+
+	it("orders SemVer prereleases before the plain release", () => {
+		expect(compareVersions("1.0.0-beta", "1.0.0")).toBe(-1);
+		expect(compareVersions("1.0.0", "1.0.0-rc.1")).toBe(1);
+		expect(compareVersions("v1.0.0-beta", "1.0.0")).toBe(-1);
+		expect(compareVersions("1.1.0-alpha", "1.0.0-beta")).toBe(1);
+	});
+
+	it("compares prerelease identifiers per SemVer 2.0", () => {
+		expect(compareVersions("1.0.0-alpha", "1.0.0-beta")).toBe(-1);
+		expect(compareVersions("1.0.0-rc.1", "1.0.0-rc.2")).toBe(-1);
+		// numeric, not lexical: rc.10 > rc.9
+		expect(compareVersions("1.0.0-rc.9", "1.0.0-rc.10")).toBe(-1);
+		// a larger set of equal fields has higher precedence
+		expect(compareVersions("1.0.0-alpha", "1.0.0-alpha.1")).toBe(-1);
+		// numeric identifiers sort before alphanumeric ones
+		expect(compareVersions("1.0.0-1", "1.0.0-alpha")).toBe(-1);
+		expect(compareVersions("1.0.0-beta.2", "1.0.0-beta.1")).toBe(1);
+	});
+
+	it("strips SemVer build metadata before comparing", () => {
+		// build metadata does not affect precedence
+		expect(compareVersions("1.0.1+linux", "1.0.0")).toBe(1);
+		expect(compareVersions("1.0.0-rc.1+abc", "1.0.0-rc.1+xyz")).toBe(0);
+		expect(compareVersions("1.0.0+build1", "1.0.0+build2")).toBe(0);
+		expect(compareVersions("1.0.0+linux", "1.0.0+mac")).toBe(0);
+		expect(compareVersions("1.0.0+linux", "1.0.1+linux")).toBe(-1);
+		expect(compareVersions("v1.2.3+meta", "1.2.3")).toBe(0);
+		expect(compareVersions(" 1.0.0+meta ", "1.0.0")).toBe(0);
+	});
+
+	it("compares malformed numeric segments as 0", () => {
+		expect(compareVersions("1.2.x", "1.2.0")).toBe(0);
+		expect(compareVersions("1.x", "1.0")).toBe(0);
+		expect(compareVersions("1.2.x", "1.2.1")).toBe(-1);
+	});
+
+	it("never throws and always returns -1, 0, or 1", () => {
+		expect(compareVersions("not_a_version", "0")).toBe(0);
+		expect(compareVersions("", "")).toBe(0);
+		expect(compareVersions("v", "")).toBe(0);
+		// hyphenated garbage parses as a prerelease suffix and loses to the release
+		expect(compareVersions("not-a-version", "1.0.0")).toBe(-1);
+		expect(compareVersions("3.0.0", "1.0.0")).toBe(1);
+		expect(compareVersions("1.0.0", "3.0.0")).toBe(-1);
+		// exact numeric comparison beyond float precision
+		expect(compareVersions("1.2.99999999999999999999", "1.2.100000000000000000000")).toBe(-1);
+	});
+});

scripts/ci-release-notes.ts

@@ -1,4 +1,5 @@
 #!/usr/bin/env bun
+
 /**
  * Generate aggregated release notes from per-package CHANGELOG.md files.
  *
@@ -28,6 +29,7 @@
  * underneath; this only adds curated context.
  */
 
+import { compareVersions } from "@oh-my-pi/pi-utils";
 import { $, Glob } from "bun";
 
 const changelogGlob = new Glob("packages/*/CHANGELOG.md");
@@ -37,22 +39,6 @@ const REPO = process.env.OMP_REPO ?? process.env.GITHUB_REPOSITORY ?? "can1357/o
 // alphabetically after these.
 const CATEGORY_ORDER = ["Breaking Changes", "Added", "Changed", "Fixed", "Removed"] as const;
 
-/** Compare two `X.Y.Z` (or `vX.Y.Z`) version strings; non-semver returns 0. */
-export function compareVersions(a: string, b: string): number {
-	const am = a
-		.replace(/^v/, "")
-		.trim()
-		.match(/^(\d+)\.(\d+)\.(\d+)$/);
-	const bm = b
-		.replace(/^v/, "")
-		.trim()
-		.match(/^(\d+)\.(\d+)\.(\d+)$/);
-	if (!am || !bm) return 0;
-	if (am[1] !== bm[1]) return Number(am[1]) - Number(bm[1]);
-	if (am[2] !== bm[2]) return Number(am[2]) - Number(bm[2]);
-	return Number(am[3]) - Number(bm[3]);
-}
-
 export interface ChangelogVersionSpan {
 	version: string;
 	/** 0-indexed line of the `## [X.Y.Z]` heading. */

scripts/release.ts

@@ -1,4 +1,5 @@
 #!/usr/bin/env bun
+import { compareVersions } from "@oh-my-pi/pi-utils";
 /**
  * Release script for pi-mono
  *
@@ -186,14 +187,6 @@ function bumpVersion(current: string, bump: "major" | "minor" | "patch"): string
 	}
 }
 
-function compareVersions(a: string, b: string): number {
-	const [aMajor, aMinor, aPatch] = parseVersion(a);
-	const [bMajor, bMinor, bPatch] = parseVersion(b);
-	if (aMajor !== bMajor) return aMajor - bMajor;
-	if (aMinor !== bMinor) return aMinor - bMinor;
-	return aPatch - bPatch;
-}
-
 async function cmdRelease(versionOrBump: string): Promise<void> {
 	console.log("\n=== Release Script ===\n");