fix: review polish (build-number ordering, chip fallback, shared CLI helpers) (#492)

* fix(kit): ordering and robustness polish

- Order build numbers numerically by (cycle, train, build, suffix) so a
  same-version re-release sorts correctly (24B91 before 24B2091) instead
  of lexicographically; used by Release's Comparable. Updates the existing
  ordering test, which had encoded the lexicographic behavior.
- resolvedChipFamilies falls back to the arch suffix (e.g. ARM64_T8132 →
  M4) when the chip label isn't a known display name like "Multiple".
- DeviceRegistry uses uniquingKeysWith instead of uniqueKeysWithValues so
  a duplicate model id in the hand-maintained list can't trap at launch.
- ComponentExtractor's integer-decode strategy picks the numerically
  highest match instead of the lexicographically last one.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Patrick Linnane <patrick@linnane.io>

* fix(cli): numeric build ordering in the index; hoist shared print helpers

- The releases.json index sort tie-broke on build number lexicographically;
  use BuildNumber.less so re-release variants order numerically, matching
  Release's Comparable.
- Hoist the identical printStatus/printInline/printError helpers that were
  duplicated across ScanCommand/ValidateCommand/CleanupCommand into
  Utilities.swift. printError now consistently prefixes "Error: " (so the
  show/compare "not found" diagnostics gain it too).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Patrick Linnane <patrick@linnane.io>

---------

Signed-off-by: Patrick Linnane <patrick@linnane.io>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: p-linnane

Sources/macOSdbKit/Models/DeviceRegistry.swift

@@ -176,6 +176,8 @@ public enum DeviceRegistry {
                        marketingName: "Apple Virtual Machine")
         ]
 
-        return Dictionary(uniqueKeysWithValues: entries.map { ($0.model, $0) })
+        // Keep the first entry if a duplicate model id ever slips into the
+        // hand-maintained list, rather than trapping as uniqueKeysWithValues would.
+        return Dictionary(entries.map { ($0.model, $0) }, uniquingKeysWith: { first, _ in first })
     }()
 }

Sources/macOSdbKit/Models/KernelInfo.swift

@@ -57,7 +57,9 @@ public struct KernelInfo: Codable, Identifiable, Hashable, Sendable {
         ChipFamily.from(chipName: chip)
     }
 
-    /// Uses per-device resolution when available, falls back to the kernel-level chip.
+    /// Uses per-device resolution when available, falls back to the kernel-level
+    /// chip, then to the arch suffix (so an unmapped label like "Multiple" on older
+    /// data still resolves via e.g. "ARM64_T8132" → M4).
     public var resolvedChipFamilies: [ChipFamily] {
         if let deviceChips, !deviceChips.isEmpty {
             var seen = Set<ChipFamily>()
@@ -67,11 +69,15 @@ public struct KernelInfo: Codable, Identifiable, Hashable, Sendable {
                     result.append(family)
                 }
             }
-            return result
+            if !result.isEmpty { return result }
         }
         if let family = chipFamily {
             return [family]
         }
+        if let suffix = arch.split(separator: "_").last.map(String.init),
+           suffix != arch, let family = ChipFamily.from(archSuffix: suffix) {
+            return [family]
+        }
         return []
     }
 }

Sources/macOSdbKit/Models/Release.swift

@@ -299,7 +299,7 @@ extension Release: Comparable {
         if lhs.prereleaseRank != rhs.prereleaseRank {
             return lhs.prereleaseRank < rhs.prereleaseRank
         }
-        return lhs.buildNumber < rhs.buildNumber
+        return BuildNumber.less(lhs.buildNumber, rhs.buildNumber)
     }
 
     private var prereleaseRank: (Int, Int) {
@@ -309,13 +309,39 @@ extension Release: Comparable {
     }
 }
 
-enum BuildNumber {
+public enum BuildNumber {
     /// Apple beta build numbers end with a lowercase letter (e.g. `25E5207k`),
     /// while release builds end with a digit (e.g. `24G90`).
     static func isBeta(_ buildNumber: String) -> Bool {
         guard let last = buildNumber.last else { return false }
         return last.isLetter && last.isLowercase
     }
+
+    /// Orders Apple build numbers by (cycle, train, build, suffix) so re-release
+    /// variants compare numerically (24B83 < 24B2083) rather than lexicographically
+    /// (where "24B2083" < "24B83" because '2' < '8').
+    public static func less(_ lhs: String, _ rhs: String) -> Bool {
+        let lhsParts = parse(lhs), rhsParts = parse(rhs)
+        if lhsParts.cycle != rhsParts.cycle { return lhsParts.cycle < rhsParts.cycle }
+        if lhsParts.train != rhsParts.train { return lhsParts.train < rhsParts.train }
+        if lhsParts.build != rhsParts.build { return lhsParts.build < rhsParts.build }
+        return lhsParts.suffix < rhsParts.suffix
+    }
+
+    /// Splits e.g. "24B2083" → (24, "B", 2083, "") and "24A5331b" → (24, "A", 5331, "b").
+    private static func parse(_ build: String) -> (cycle: Int, train: String, build: Int, suffix: String) {
+        let chars = Array(build)
+        var idx = 0
+        func take(_ predicate: (Character) -> Bool) -> String {
+            let start = idx
+            while idx < chars.count, predicate(chars[idx]) { idx += 1 }
+            return String(chars[start..<idx])
+        }
+        let cycle = Int(take { $0.isNumber }) ?? 0
+        let train = take { $0.isLetter }
+        let build = Int(take { $0.isNumber }) ?? 0
+        return (cycle, train, build, String(chars[idx...]))
+    }
 }
 
 enum MacOSRelease {

Sources/macOSdbKit/Scanner/ComponentExtractor.swift

@@ -49,11 +49,10 @@ enum ComponentExtractor {
     ) -> Component? {
         let minLength = definition.minLength ?? BinaryStringScanner.defaultMinLength
         let matches = BinaryStringScanner.findAll(in: data, matching: definition.pattern, minLength: minLength)
-        let uniqueMatches = Array(Set(matches)).sorted()
 
-        // Take the last (highest) match — typically the actual version
-        guard let intString = uniqueMatches.last,
-              let intValue = Int(intString) else {
+        // Take the numerically highest match. A lexicographic sort would rank
+        // different-width integers wrong — e.g. "9" sorts above "21209".
+        guard let intValue = Set(matches).compactMap({ Int($0) }).max() else {
             logger.debug("\(definition.name): no integer version found")
             return nil
         }

Sources/macosdb/CleanupCommand.swift

@@ -188,14 +188,4 @@ struct CleanupCommand: AsyncParsableCommand {
         }
     }
 
-    // MARK: - Helpers
-
-    private func printStatus(_ message: String) {
-        FileHandle.standardError.write(Data((message + "\n").utf8))
-    }
-
-    private func printInline(_ message: String) {
-        let line = message.isEmpty ? "\r\u{1B}[K" : "\r\(message)"
-        FileHandle.standardError.write(Data(line.utf8))
-    }
 }

Sources/macosdb/ScanCommand.swift

@@ -114,7 +114,7 @@ struct ScanCommand: AsyncParsableCommand {
         await configureProgress(scanner)
         if verbose {
             await scanner.setVerbose { message in
-                self.printStatus("[verbose] \(message)")
+                printStatus("[verbose] \(message)")
             }
         }
 
@@ -146,7 +146,7 @@ struct ScanCommand: AsyncParsableCommand {
         await configureProgress(scanner)
         if verbose {
             await scanner.setVerbose { message in
-                self.printStatus("[verbose] \(message)")
+                printStatus("[verbose] \(message)")
             }
         }
 
@@ -172,7 +172,7 @@ struct ScanCommand: AsyncParsableCommand {
         await configureProgress(scanner)
         if verbose {
             await scanner.setVerbose { message in
-                self.printStatus("[verbose] \(message)")
+                printStatus("[verbose] \(message)")
             }
         }
 
@@ -320,7 +320,7 @@ struct ScanCommand: AsyncParsableCommand {
             let lhsRank = lhs.isBeta ? 0 : lhs.isRC ? 1 : 2
             let rhsRank = rhs.isBeta ? 0 : rhs.isRC ? 1 : 2
             if lhsRank != rhsRank { return lhsRank > rhsRank }
-            return lhs.buildNumber > rhs.buildNumber
+            return BuildNumber.less(rhs.buildNumber, lhs.buildNumber)
         }
 
         let encoder = JSONEncoder()
@@ -348,13 +348,6 @@ struct ScanCommand: AsyncParsableCommand {
         }
     }
 
-    private func printStatus(_ message: String) {
-        FileHandle.standardError.write(Data((message + "\n").utf8))
-    }
-
-    private func printError(_ message: String) {
-        FileHandle.standardError.write(Data(("Error: " + message + "\n").utf8))
-    }
 }
 
 extension IPSWScanner {

Sources/macosdb/Utilities.swift

@@ -8,12 +8,23 @@ nonisolated func makeDataProvider(dataURL: String?) -> DataProvider {
     return DataProvider()
 }
 
-/// Writes a diagnostic line to standard error. CLI errors belong on stderr so
+/// Writes an "Error: …" line to standard error. CLI errors belong on stderr so
 /// they don't contaminate the stdout stream consumers parse (e.g. --json output).
 nonisolated func printError(_ message: String) {
+    FileHandle.standardError.write(Data(("Error: " + message + "\n").utf8))
+}
+
+/// Writes a status/progress line to standard error.
+nonisolated func printStatus(_ message: String) {
     FileHandle.standardError.write(Data((message + "\n").utf8))
 }
 
+/// Writes an in-place progress line (carriage return, no newline) to standard error.
+nonisolated func printInline(_ message: String) {
+    let line = message.isEmpty ? "\r\u{1B}[K" : "\r\(message)"
+    FileHandle.standardError.write(Data(line.utf8))
+}
+
 nonisolated func writeJSON<T: Encodable>(_ value: T) throws {
     let encoder = JSONEncoder()
     encoder.outputFormatting = [.prettyPrinted, .sortedKeys, .withoutEscapingSlashes]

Sources/macosdb/ValidateCommand.swift

@@ -183,12 +183,4 @@ struct ValidateCommand: AsyncParsableCommand {
         return urls
     }
 
-    private func printStatus(_ message: String) {
-        FileHandle.standardError.write(Data((message + "\n").utf8))
-    }
-
-    private func printInline(_ message: String) {
-        let line = message.isEmpty ? "\r\u{1B}[K" : "\r\(message)"
-        FileHandle.standardError.write(Data(line.utf8))
-    }
 }

Tests/macOSdbKitTests/ModelOrderingTests.swift

@@ -0,0 +1,58 @@
+import Testing
+
+@testable import macOSdbKit
+
+@Suite("Build number ordering")
+struct BuildNumberOrderingTests {
+
+    @Test("Re-release variant sorts after the original (numeric, not lexicographic)")
+    func reReleaseOrdering() {
+        // Lexicographically "24B2083" < "24B83" (because '2' < '8'); numerically 83 < 2083.
+        #expect(BuildNumber.less("24B83", "24B2083"))
+        #expect(!BuildNumber.less("24B2083", "24B83"))
+    }
+
+    @Test("Orders by cycle, then train, then build, then suffix")
+    func componentOrdering() {
+        #expect(BuildNumber.less("23G93", "24A100"))     // cycle: 23 < 24
+        #expect(BuildNumber.less("24A300", "24B83"))     // train: A < B
+        #expect(BuildNumber.less("25A8364", "25A8365"))  // build: 8364 < 8365
+        #expect(BuildNumber.less("24A5331", "24A5331b")) // suffix: "" < "b"
+    }
+
+    @Test("Equal build numbers are not less than each other")
+    func equalBuilds() {
+        #expect(!BuildNumber.less("24G90", "24G90"))
+    }
+}
+
+@Suite("Chip family resolution")
+struct ChipFamilyResolutionTests {
+
+    @Test("Falls back to the arch suffix when the chip label is unmapped")
+    func archSuffixFallback() {
+        // Older data: chip label "Multiple" maps to no display name and there are no
+        // deviceChips, so resolution must fall back to the arch suffix.
+        let kernel = KernelInfo(
+            file: "kernelcache.release.mac15",
+            darwinVersion: "24.0.0",
+            arch: "ARM64_T8132",
+            chip: "Multiple",
+            devices: ["Mac16,1", "Mac16,2"]
+        )
+        #expect(kernel.resolvedChipFamilies == [.m4])
+    }
+
+    @Test("Per-device chips take priority over the arch fallback")
+    func deviceChipsPreferred() {
+        let kernel = KernelInfo(
+            file: "kernelcache.release.mac15",
+            darwinVersion: "24.0.0",
+            arch: "ARM64_T6041",
+            chip: "Multiple",
+            devices: ["Mac16,5"],
+            deviceChips: [DeviceChip(device: "Mac16,5", chip: "M4 Max")]
+        )
+        #expect(kernel.resolvedChipFamilies == [.m4Max])
+    }
+}

Tests/macOSdbKitTests/ModelTests.swift

@@ -93,15 +93,17 @@ struct ModelTests { // swiftlint:disable:this type_body_length
         #expect(older < newer)
     }
 
-    @Test("Release sorting — same version different builds is deterministic")
+    @Test("Release sorting — same version orders builds numerically, not lexicographically")
     func releaseSortingSameVersion() {
-        let buildA = Release(osVersion: "15.1.1", buildNumber: "24B2091", releaseName: "Sequoia")
-        let buildB = Release(osVersion: "15.1.1", buildNumber: "24B91", releaseName: "Sequoia")
-        #expect(buildA < buildB)
-        #expect(!(buildB < buildA))
-        // Verify sorted order is stable
-        let sorted = [buildB, buildA].sorted()
-        #expect(sorted.map(\.buildNumber) == ["24B2091", "24B91"])
+        // 24B91 (original) predates 24B2091 (the later device-specific re-release).
+        // Numeric ordering must place 91 before 2091; a lexicographic compare would
+        // wrongly rank "24B2091" first because '2' < '8'.
+        let original = Release(osVersion: "15.1.1", buildNumber: "24B91", releaseName: "Sequoia")
+        let reRelease = Release(osVersion: "15.1.1", buildNumber: "24B2091", releaseName: "Sequoia")
+        #expect(original < reRelease)
+        #expect(!(reRelease < original))
+        let sorted = [reRelease, original].sorted()
+        #expect(sorted.map(\.buildNumber) == ["24B91", "24B2091"])
     }
 
     @Test("Release component lookup by name")