From f769cf7fec0dc6d48d8f155443bbd6b7220a736a Mon Sep 17 00:00:00 2001 From: Gronod Date: Wed, 9 Sep 2026 10:43:12 +0100 Subject: [PATCH 1/2] =?UTF-8?q?Milestone=205=20=E2=80=94=20Profile=20gener?= =?UTF-8?q?ation,=20verification=20&=20installation=20(#23=E2=80=93#27).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the Stage 4/5 native workflow: deterministic colprof/applycal/iccgamut and profcheck argv builders, verification history with CSV export and drift analytics, user/system ColorSync profile installation, and the matching SwiftUI views and fixture-driven tests. All Argyll interaction remains AGPL-isolated via ProcessManager subprocesses. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../ICCeryCore/Argyll/ArgyllRunner.swift | 213 +++++++- .../ICCeryCore/Profile/ApplycalArgs.swift | 50 ++ .../ICCeryCore/Profile/ApplycalConfig.swift | 22 + .../ICCeryCore/Profile/ColprofArgs.swift | 79 +++ .../ICCeryCore/Profile/ColprofConfig.swift | 45 ++ .../ICCeryCore/Profile/ColprofProgress.swift | 30 ++ .../ICCeryCore/Profile/DriftAlert.swift | 41 ++ .../ICCeryCore/Profile/IccgamutArgs.swift | 29 ++ .../ICCeryCore/Profile/IccgamutConfig.swift | 12 + .../Profile/InstallProfileConfig.swift | 12 + .../Profile/InstallProfileOptions.swift | 31 ++ .../Profile/InstallProfileResult.swift | 30 ++ .../ICCeryCore/Profile/ProfcheckArgs.swift | 37 ++ .../ICCeryCore/Profile/ProfcheckConfig.swift | 12 + .../ICCeryCore/Profile/ProfcheckParser.swift | 217 ++++++++ .../ICCeryCore/Profile/ProfcheckReport.swift | 31 ++ .../ICCeryCore/Profile/ProfileInstaller.swift | 172 +++++++ .../Profile/VerificationHistoryStore.swift | 120 +++++ .../Profile/VerificationRecord.swift | 36 ++ .../Profile/VerificationStatus.swift | 32 ++ Sources/ICCery/AppEnvironment.swift | 4 +- Sources/ICCery/DriftChartView.swift | 121 +++++ Sources/ICCery/ProfileWorkflowViewModel.swift | 487 ++++++++++++++++++ Sources/ICCery/RootView.swift | 4 + Sources/ICCery/Stage4View.swift | 194 +++++++ Sources/ICCery/Stage5View.swift | 272 ++++++++++ Sources/ICCery/TargetWorkflowViewModel.swift | 21 +- Tests/ICCeryCoreTests/ApplycalArgsTests.swift | 30 ++ .../ArgyllRunnerColprofTests.swift | 52 ++ Tests/ICCeryCoreTests/ColprofArgsTests.swift | 72 +++ .../ColprofProgressTests.swift | 24 + Tests/ICCeryCoreTests/DriftAlertTests.swift | 63 +++ Tests/ICCeryCoreTests/IccgamutArgsTests.swift | 16 + .../ICCeryCoreTests/ProfcheckArgsTests.swift | 17 + .../ProfcheckParserTests.swift | 71 +++ .../ProfileInstallerTests.swift | 75 +++ .../VerificationHistoryStoreTests.swift | 79 +++ Tests/ICCeryUITests/Fixtures/bin/applycal | 18 + Tests/ICCeryUITests/Fixtures/bin/colprof | 14 + Tests/ICCeryUITests/Fixtures/bin/iccgamut | 13 + Tests/ICCeryUITests/Fixtures/bin/profcheck | 12 + Tests/ICCeryUITests/Milestone3UITests.swift | 42 +- Tests/ICCeryUITests/Milestone5UITests.swift | 112 ++++ 43 files changed, 3049 insertions(+), 15 deletions(-) create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/ApplycalArgs.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/ApplycalConfig.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/ColprofArgs.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/ColprofConfig.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/ColprofProgress.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/DriftAlert.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/IccgamutArgs.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/IccgamutConfig.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/InstallProfileConfig.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/InstallProfileOptions.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/InstallProfileResult.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckArgs.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckConfig.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckParser.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckReport.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfileInstaller.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationHistoryStore.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationRecord.swift create mode 100644 Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationStatus.swift create mode 100644 Sources/ICCery/DriftChartView.swift create mode 100644 Sources/ICCery/ProfileWorkflowViewModel.swift create mode 100644 Sources/ICCery/Stage4View.swift create mode 100644 Sources/ICCery/Stage5View.swift create mode 100644 Tests/ICCeryCoreTests/ApplycalArgsTests.swift create mode 100644 Tests/ICCeryCoreTests/ArgyllRunnerColprofTests.swift create mode 100644 Tests/ICCeryCoreTests/ColprofArgsTests.swift create mode 100644 Tests/ICCeryCoreTests/ColprofProgressTests.swift create mode 100644 Tests/ICCeryCoreTests/DriftAlertTests.swift create mode 100644 Tests/ICCeryCoreTests/IccgamutArgsTests.swift create mode 100644 Tests/ICCeryCoreTests/ProfcheckArgsTests.swift create mode 100644 Tests/ICCeryCoreTests/ProfcheckParserTests.swift create mode 100644 Tests/ICCeryCoreTests/ProfileInstallerTests.swift create mode 100644 Tests/ICCeryCoreTests/VerificationHistoryStoreTests.swift create mode 100755 Tests/ICCeryUITests/Fixtures/bin/applycal create mode 100755 Tests/ICCeryUITests/Fixtures/bin/colprof create mode 100755 Tests/ICCeryUITests/Fixtures/bin/iccgamut create mode 100755 Tests/ICCeryUITests/Fixtures/bin/profcheck create mode 100644 Tests/ICCeryUITests/Milestone5UITests.swift diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/ArgyllRunner.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/ArgyllRunner.swift index 8b9b657..9dcabe5 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/ArgyllRunner.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/ArgyllRunner.swift @@ -8,6 +8,11 @@ public enum ArgyllRunnerError: LocalizedError, Equatable, Sendable { case instrumentDetectionFailed(String) case chartreadFailed(String) case averageFailed(String) + case colprofFailed(String) + case applycalFailed(String) + case iccgamutFailed(String) + case profcheckFailed(String) + case profcheckUnparseable public var errorDescription: String? { switch self { @@ -23,6 +28,16 @@ public enum ArgyllRunnerError: LocalizedError, Equatable, Sendable { return "Chartread failed: \(reason)" case .averageFailed(let reason): return "Averaging failed: \(reason)" + case .colprofFailed(let reason): + return "Profile creation failed: \(reason)" + case .applycalFailed(let reason): + return "Apply calibration failed: \(reason)" + case .iccgamutFailed(let reason): + return "Gamut extraction failed: \(reason)" + case .profcheckFailed(let reason): + return "Profile verification failed: \(reason)" + case .profcheckUnparseable: + return "Profile verification produced unparseable output" } } } @@ -157,6 +172,7 @@ public struct ArgyllRunner: Sendable { private struct CollectedRun { var exitCode: Int32? var stdout: String + var stderr: String var lines: [String] } @@ -170,6 +186,7 @@ public struct ArgyllRunner: Sendable { ) async -> CollectedRun { var lines: [String] = [] var stdout = "" + var stderr = "" var pendingBatch: [String] = [] var exitCode: Int32? var lastFlush = Date() @@ -190,6 +207,7 @@ public struct ArgyllRunner: Sendable { pendingBatch.append(line) case .stderr(_, let line): lines.append(line) + stderr += line + "\n" pendingBatch.append(line) case .error(_, let message): lines.append("Error: \(message)") @@ -211,7 +229,7 @@ public struct ArgyllRunner: Sendable { break } } - return CollectedRun(exitCode: exitCode, stdout: stdout, lines: lines) + return CollectedRun(exitCode: exitCode, stdout: stdout, stderr: stderr, lines: lines) } // MARK: - instlist (Stage 3 detection) @@ -303,6 +321,199 @@ public struct ArgyllRunner: Sendable { return canonical } + // MARK: - colprof (Stage 4) + + /// Runs `colprof` streaming, collecting logs and classifying progress + /// until the profile is written. + public func runColprof( + config: ColprofConfig, + onLogBatch: (@Sendable ([String]) -> Void)? = nil + ) async throws -> URL { + let cleanBasename = try PathSecurity.sanitizeBasename(config.basename) + let cwd = PathSecurity.resolveSafeCwd(config.workingDirectory) + let args = try ColprofArgs.build(config: config) + let binaryURL = binaryResolver.resolve("colprof") + let processId = ProcessID.colprof(cleanBasename) + + let events = processManager.events() + try await processManager.runStreaming( + id: processId, + binary: binaryURL, + arguments: args, + workingDirectory: cwd + ) + let run = await collect(id: processId, events: events, onLogBatch: onLogBatch) + + guard run.exitCode == 0 else { + throw ArgyllRunnerError.colprofFailed( + "colprof exited with code \(run.exitCode ?? -1)" + ) + } + + // Argyll may produce `.icm` on Windows, but on macOS we expect `.icc`. + // `resolveProfile` checks `.icm` first, then `.icc`, matching #69. + guard let profileURL = ArtefactProbe.resolveProfile( + basename: cleanBasename, + cwd: cwd + ) else { + let defaultURL = cwd.appendingPathComponent("\(cleanBasename).icc") + throw ArgyllRunnerError.missingArtefact(defaultURL.path) + } + return profileURL + } + + // MARK: - applycal (post-colprof calibration curve) + + /// Embeds a `.cal` curve into an `.icc`/`.icm` profile. + /// + /// Runs `applycal` captured and performs an in-place replace via + /// `{input}.applycal.tmp` then `replaceItemAt`. On failure the tmp + /// file is removed and the original is left untouched. + public func runApplycal( + config: ApplycalConfig + ) async throws -> URL { + let inputURL = config.inputProfileURL + let cwd = inputURL.deletingLastPathComponent() + let binaryURL = binaryResolver.resolve("applycal") + let processId = ProcessID.applycal(inputURL.lastPathComponent) + + let tmpURL = inputURL.appendingPathExtension("applycal.tmp") + let fm = FileManager.default + + // Remove any stale tmp from a previous crash. + try? fm.removeItem(at: tmpURL) + + let outputConfig = ApplycalConfig( + calibrationPath: config.calibrationPath, + inputProfileURL: inputURL, + outputProfileURL: tmpURL, + unapply: false + ) + let outputArgs = try ApplycalArgs.build(config: outputConfig) + + let result = try await processManager.runCaptured( + id: processId, + binary: binaryURL, + arguments: outputArgs, + workingDirectory: cwd + ) + + guard result.exitCode == 0 else { + try? fm.removeItem(at: tmpURL) + throw ArgyllRunnerError.applycalFailed( + result.stderr.isEmpty + ? "applycal exited with code \(result.exitCode)" + : result.stderr + ) + } + + guard fm.fileExists(atPath: tmpURL.path) else { + throw ArgyllRunnerError.applycalFailed( + "applycal did not create temp profile" + ) + } + + do { + if fm.fileExists(atPath: inputURL.path) { + _ = try fm.replaceItemAt(inputURL, withItemAt: tmpURL) + } else { + try fm.moveItem(at: tmpURL, to: inputURL) + } + } catch { + try? fm.removeItem(at: tmpURL) + throw ArgyllRunnerError.applycalFailed(error.localizedDescription) + } + + return inputURL + } + + // MARK: - iccgamut (post-colprof gamut mesh) + + /// Extracts a `.gam` mesh from the finished profile. + public func runIccgamut( + config: IccgamutConfig, + onLogBatch: (@Sendable ([String]) -> Void)? = nil + ) async throws -> URL { + let profileURL = config.profileURL + let cwd = profileURL.deletingLastPathComponent() + let stem = profileURL.deletingPathExtension().lastPathComponent + let args = try IccgamutArgs.build(config: config) + let binaryURL = binaryResolver.resolve("iccgamut") + let processId = ProcessID.iccgamut(stem: stem) + + let events = processManager.events() + try await processManager.runStreaming( + id: processId, + binary: binaryURL, + arguments: args, + workingDirectory: cwd + ) + let run = await collect(id: processId, events: events, onLogBatch: onLogBatch) + + guard run.exitCode == 0 else { + throw ArgyllRunnerError.iccgamutFailed( + "iccgamut exited with code \(run.exitCode ?? -1)" + ) + } + + let gamURL = cwd.appendingPathComponent("\(stem).gam") + guard FileManager.default.fileExists(atPath: gamURL.path) else { + throw ArgyllRunnerError.missingArtefact(gamURL.path) + } + return gamURL + } + + // MARK: - profcheck (Stage 5 verification) + + /// Verifies a profile against the canonical `.ti3`. + public func runProfcheck( + config: ProfcheckConfig, + onLogBatch: (@Sendable ([String]) -> Void)? = nil + ) async throws -> ProfcheckReport { + let cwd = config.ti3URL.deletingLastPathComponent() + let ti3Path = config.ti3URL.path + + let iccURL = Self.resolveProfileForVerification(config.iccURL) + let config = ProfcheckConfig(ti3URL: config.ti3URL, iccURL: iccURL) + + let args = try ProfcheckArgs.build(config: config) + let binaryURL = binaryResolver.resolve("profcheck") + let processId = ProcessID.profcheck(ti3Path: ti3Path) + + let events = processManager.events() + try await processManager.runStreaming( + id: processId, + binary: binaryURL, + arguments: args, + workingDirectory: cwd + ) + let run = await collect(id: processId, events: events, onLogBatch: onLogBatch) + + guard run.exitCode == 0 else { + throw ArgyllRunnerError.profcheckFailed( + run.stderr.isEmpty + ? "profcheck exited with code \(run.exitCode ?? -1)" + : run.stderr + ) + } + + let output = (run.stdout + "\n" + run.stderr).trimmingCharacters(in: .whitespacesAndNewlines) + let report = ProfcheckParser.parse(output) + guard report.isValid else { + throw ArgyllRunnerError.profcheckUnparseable + } + return report + } + + private static func resolveProfileForVerification(_ url: URL) -> URL { + let fm = FileManager.default + if fm.fileExists(atPath: url.path) { return url } + let alt = url.pathExtension.lowercased() == "icc" + ? url.deletingPathExtension().appendingPathExtension("icm") + : url.deletingPathExtension().appendingPathExtension("icc") + return fm.fileExists(atPath: alt.path) ? alt : url + } + // MARK: - chartread (Stage 3 interactive) /// Runs `chartread` and returns an `AsyncStream` of typed events. diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ApplycalArgs.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ApplycalArgs.swift new file mode 100644 index 0000000..dc9a5c1 --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ApplycalArgs.swift @@ -0,0 +1,50 @@ +import Foundation + +/// Errors during `applycal` argv construction. +public enum ApplycalArgError: LocalizedError, Equatable, Sendable { + case invalidCalibrationPath + case invalidInputProfileURL + + public var errorDescription: String? { + switch self { + case .invalidCalibrationPath: + return "Calibration path is invalid or empty" + case .invalidInputProfileURL: + return "Input profile path is invalid or empty" + } + } +} + +/// Pure argv builder for Argyll's `applycal` tool. +/// +/// `applycal` is always run captured, never streamed. +public enum ApplycalArgs { + + /// Builds `applycal -v -a {cal} {input} [{output}]`. + /// + /// `-u` (unapply) is rejected at the builder level — the UI never + /// sends it (docs/04 §7.2). + public static func build(config: ApplycalConfig) throws -> [String] { + let cal = config.calibrationPath.trimmingCharacters(in: .whitespaces) + guard !cal.isEmpty else { throw ApplycalArgError.invalidCalibrationPath } + + let input = config.inputProfileURL.path + guard !input.isEmpty else { throw ApplycalArgError.invalidInputProfileURL } + + var args: [String] = ["-v"] + if config.unapply { + // Defensive: should never be called from the UI. + args.append("-u") + } else { + args.append("-a") + } + + args.append(contentsOf: [cal, input]) + + if let output = config.outputProfileURL?.path, !output.isEmpty { + args.append(output) + } + + return args + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ApplycalConfig.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ApplycalConfig.swift new file mode 100644 index 0000000..3f793cc --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ApplycalConfig.swift @@ -0,0 +1,22 @@ +import Foundation + +/// Configuration for an Argyll `applycal` run. +public struct ApplycalConfig: Sendable, Equatable { + public var calibrationPath: String + public var inputProfileURL: URL + public var outputProfileURL: URL? + public var unapply: Bool + + /// In-place when `outputProfileURL` is `nil`. + public init( + calibrationPath: String, + inputProfileURL: URL, + outputProfileURL: URL? = nil, + unapply: Bool = false + ) { + self.calibrationPath = calibrationPath + self.inputProfileURL = inputProfileURL + self.outputProfileURL = outputProfileURL + self.unapply = unapply + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ColprofArgs.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ColprofArgs.swift new file mode 100644 index 0000000..05d4566 --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ColprofArgs.swift @@ -0,0 +1,79 @@ +import Foundation + +/// Errors during `colprof` argv construction. +public enum ColprofArgError: LocalizedError, Equatable, Sendable { + case invalidBasename(String) + + public var errorDescription: String? { + switch self { + case .invalidBasename(let name): + return "Invalid colprof basename: \(name)" + } + } +} + +/// Pure argv builder for Argyll's `colprof` tool. +public enum ColprofArgs { + + /// Builds `colprof` argv per the Gronod fork protocol. + /// + /// Always `-v -a {algorithm} -q {quality}`. Optional flags are added + /// only when their fields are non-empty and meaningful. `-f` has + /// special handling for "none" (omit), "" (bare flag), and a custom + /// `.sp` path (passed through). `-c`/`-d` viewing conditions are + /// skipped when set to "none". + public static func build(config: ColprofConfig) throws -> [String] { + let cleanBasename = try PathSecurity.sanitizeBasename(config.basename) + + var args: [String] = ["-v"] + + args.append(contentsOf: ["-a", config.algorithm]) + args.append(contentsOf: ["-q", config.quality]) + + if let intent = config.intent?.trimmingCharacters(in: .whitespaces), !intent.isEmpty { + args.append(contentsOf: ["-t", intent]) + } + + if let fwa = config.fwa?.trimmingCharacters(in: .whitespaces) { + switch fwa.lowercased() { + case "none", "": + // "none" omits the flag; an explicit empty string means bare -f. + if fwa.isEmpty { + args.append("-f") + } + default: + args.append(contentsOf: ["-f", fwa]) + } + } + + if let illuminant = config.illuminant?.trimmingCharacters(in: .whitespaces), !illuminant.isEmpty { + args.append(contentsOf: ["-i", illuminant]) + } + + if let observer = config.observer?.trimmingCharacters(in: .whitespaces), !observer.isEmpty { + args.append(contentsOf: ["-o", observer]) + } + + if let inputCond = config.inputViewingCond?.trimmingCharacters(in: .whitespaces), + !inputCond.isEmpty, inputCond.lowercased() != "none" { + args.append(contentsOf: ["-c", inputCond]) + } + + if let outputCond = config.outputViewingCond?.trimmingCharacters(in: .whitespaces), + !outputCond.isEmpty, outputCond.lowercased() != "none" { + args.append(contentsOf: ["-d", outputCond]) + } + + let profileDescription = config.description?.trimmingCharacters(in: .whitespaces) + if let description = profileDescription, !description.isEmpty { + args.append(contentsOf: ["-D", description]) + } + + if let copyright = config.copyright?.trimmingCharacters(in: .whitespaces), !copyright.isEmpty { + args.append(contentsOf: ["-C", copyright]) + } + + args.append(cleanBasename) + return args + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ColprofConfig.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ColprofConfig.swift new file mode 100644 index 0000000..3cf0ec3 --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ColprofConfig.swift @@ -0,0 +1,45 @@ +import Foundation + +/// Configuration for an Argyll `colprof` run (issue #23, docs/16). +public struct ColprofConfig: Sendable, Equatable { + public var algorithm: String + public var quality: String + public var intent: String? + public var fwa: String? + public var illuminant: String? + public var observer: String? + public var inputViewingCond: String? + public var outputViewingCond: String? + public var description: String? + public var copyright: String? + public var basename: String + public var workingDirectory: URL? + + public init( + algorithm: String = "l", + quality: String = "m", + intent: String? = nil, + fwa: String? = nil, + illuminant: String? = nil, + observer: String? = nil, + inputViewingCond: String? = nil, + outputViewingCond: String? = nil, + description: String? = nil, + copyright: String? = nil, + basename: String, + workingDirectory: URL? = nil + ) { + self.algorithm = algorithm + self.quality = quality + self.intent = intent + self.fwa = fwa + self.illuminant = illuminant + self.observer = observer + self.inputViewingCond = inputViewingCond + self.outputViewingCond = outputViewingCond + self.description = description + self.copyright = copyright + self.basename = basename + self.workingDirectory = workingDirectory + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ColprofProgress.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ColprofProgress.swift new file mode 100644 index 0000000..18ff27d --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ColprofProgress.swift @@ -0,0 +1,30 @@ +import Foundation + +/// Classified `colprof` stdout progress milestone. +public enum ColprofProgress: Sendable, Equatable { + case gamutMapping + case fittingClut + case writingIcc + case unknown +} + +/// Parses `colprof` plaintext progress (docs/16 §6.3). +/// +/// The Gronod fork supports `-u` JSON, but ICCery v2.0 does not pass it. +/// Progress is therefore inferred from case-insensitive substring matches. +public enum ColprofProgressClassifier { + + public static func classify(line: String) -> ColprofProgress { + let lower = line.lowercased() + if lower.contains("gamut mapping") { + return .gamutMapping + } + if lower.contains("fitting") || lower.contains("clut") { + return .fittingClut + } + if lower.contains("writing") || lower.contains("icc profile") { + return .writingIcc + } + return .unknown + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/DriftAlert.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/DriftAlert.swift new file mode 100644 index 0000000..f673bd3 --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/DriftAlert.swift @@ -0,0 +1,41 @@ +import Foundation + +/// Computes a consecutive-breach warning from verification history. +/// +/// A drift alert triggers when there are at least two `poor` records on +/// distinct calendar days, or two `poor` records at least one hour apart. +public enum DriftAlert { + + /// Returns an alert message, or `nil` when no consecutive breach exists. + public static func compute(from records: [VerificationRecord]) -> String? { + let poor = records + .filter { $0.status == .poor } + .sorted { $0.timestamp < $1.timestamp } + + guard poor.count >= 2 else { return nil } + + for i in 0..= 3600 + + if !sameDay || oneHour { + return "Drift alert: poor results between \(a.id) and \(b.id)." + } + } + } + + return nil + } +} + +private extension Calendar { + static let utc: Calendar = { + var c = Calendar(identifier: .iso8601) + c.timeZone = TimeZone(identifier: "UTC")! + return c + }() +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/IccgamutArgs.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/IccgamutArgs.swift new file mode 100644 index 0000000..a647039 --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/IccgamutArgs.swift @@ -0,0 +1,29 @@ +import Foundation + +/// Errors during `iccgamut` argv construction. +public enum IccgamutArgError: LocalizedError, Equatable, Sendable { + case invalidProfileURL + + public var errorDescription: String? { + switch self { + case .invalidProfileURL: + return "iccgamut requires a valid profile path" + } + } +} + +/// Pure argv builder for Argyll's `iccgamut` tool. +public enum IccgamutArgs { + + /// Builds `iccgamut -v -d {density} {profilePath}`. + /// + /// The caller is responsible for ensuring `density` is a positive + /// integer. `-d` here is surface **density**, not a directory. + public static func build(config: IccgamutConfig) throws -> [String] { + let path = config.profileURL.path + guard !path.isEmpty else { throw IccgamutArgError.invalidProfileURL } + + let density = max(1, config.density) + return ["-v", "-d", "\(density)", path] + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/IccgamutConfig.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/IccgamutConfig.swift new file mode 100644 index 0000000..069f004 --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/IccgamutConfig.swift @@ -0,0 +1,12 @@ +import Foundation + +/// Configuration for an Argyll `iccgamut` run. +public struct IccgamutConfig: Sendable, Equatable { + public var profileURL: URL + public var density: Int + + public init(profileURL: URL, density: Int = 10) { + self.profileURL = profileURL + self.density = density + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/InstallProfileConfig.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/InstallProfileConfig.swift new file mode 100644 index 0000000..c3d8859 --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/InstallProfileConfig.swift @@ -0,0 +1,12 @@ +import Foundation + +/// Configuration for a profile installation. +public struct InstallProfileConfig: Sendable, Equatable { + public var sourceURL: URL + public var options: InstallProfileOptions + + public init(sourceURL: URL, options: InstallProfileOptions = InstallProfileOptions()) { + self.sourceURL = sourceURL + self.options = options + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/InstallProfileOptions.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/InstallProfileOptions.swift new file mode 100644 index 0000000..81826c8 --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/InstallProfileOptions.swift @@ -0,0 +1,31 @@ +import Foundation + +/// Collision policy for profile installation. +public enum ProfileCollisionPolicy: String, Sendable, Equatable, Codable, CaseIterable { + case overwrite + case rename + case cancel +} + +/// Options for installing a finished profile into the OS colour store. +public struct InstallProfileOptions: Sendable, Equatable, Codable { + public var forceOverwrite: Bool + public var preferSystem: Bool + public var collisionPolicy: ProfileCollisionPolicy + public var openColorPanel: Bool + public var calibrationNote: String? + + public init( + forceOverwrite: Bool = false, + preferSystem: Bool = false, + collisionPolicy: ProfileCollisionPolicy = .cancel, + openColorPanel: Bool = false, + calibrationNote: String? = nil + ) { + self.forceOverwrite = forceOverwrite + self.preferSystem = preferSystem + self.collisionPolicy = collisionPolicy + self.openColorPanel = openColorPanel + self.calibrationNote = calibrationNote + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/InstallProfileResult.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/InstallProfileResult.swift new file mode 100644 index 0000000..31e78ff --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/InstallProfileResult.swift @@ -0,0 +1,30 @@ +import Foundation + +/// Result of installing a profile into the OS colour store. +public struct InstallProfileResult: Sendable, Equatable, Codable { + public var destPath: String + public var registered: Bool + public var overwritten: Bool + public var renamed: Bool + public var openedPanel: Bool + public var message: String + public var calibrationNote: String? + + public init( + destPath: String, + registered: Bool, + overwritten: Bool, + renamed: Bool, + openedPanel: Bool, + message: String, + calibrationNote: String? = nil + ) { + self.destPath = destPath + self.registered = registered + self.overwritten = overwritten + self.renamed = renamed + self.openedPanel = openedPanel + self.message = message + self.calibrationNote = calibrationNote + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckArgs.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckArgs.swift new file mode 100644 index 0000000..77771c8 --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckArgs.swift @@ -0,0 +1,37 @@ +import Foundation + +/// Errors during `profcheck` argv construction. +public enum ProfcheckArgError: LocalizedError, Equatable, Sendable { + case missingTi3 + case missingIcc + case invalidTi3Path + + public var errorDescription: String? { + switch self { + case .missingTi3: + return "profcheck requires a .ti3 file" + case .missingIcc: + return "profcheck requires a profile (.icc/.icm)" + case .invalidTi3Path: + return "profcheck .ti3 path is invalid" + } + } +} + +/// Pure argv builder for Argyll's `profcheck` tool. +public enum ProfcheckArgs { + + /// Builds `profcheck -v -k -s -u {ti3Path} {iccPath}`. + /// + /// The `-u` here is the Gronod fork JSON report flag, not the + /// generic `-u` auto-fix that some Argyll builds use. + public static func build(config: ProfcheckConfig) throws -> [String] { + let ti3Path = config.ti3URL.path + let iccPath = config.iccURL.path + + guard !ti3Path.isEmpty else { throw ProfcheckArgError.missingTi3 } + guard !iccPath.isEmpty else { throw ProfcheckArgError.missingIcc } + + return ["-v", "-k", "-s", "-u", ti3Path, iccPath] + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckConfig.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckConfig.swift new file mode 100644 index 0000000..eda6054 --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckConfig.swift @@ -0,0 +1,12 @@ +import Foundation + +/// Configuration for an Argyll `profcheck` run. +public struct ProfcheckConfig: Sendable, Equatable { + public var ti3URL: URL + public var iccURL: URL + + public init(ti3URL: URL, iccURL: URL) { + self.ti3URL = ti3URL + self.iccURL = iccURL + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckParser.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckParser.swift new file mode 100644 index 0000000..b8245bd --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckParser.swift @@ -0,0 +1,217 @@ +import Foundation + +/// Errors returned when `profcheck` output cannot be parsed. +public enum ProfcheckParserError: LocalizedError, Equatable, Sendable { + case unparseable + case jsonDecodingFailed + + public var errorDescription: String? { + switch self { + case .unparseable: + return "Could not parse profcheck report" + case .jsonDecodingFailed: + return "profcheck JSON report could not be decoded" + } + } +} + +/// Parses the mixed JSON/text output from `profcheck -v -k -s -u`. +public enum ProfcheckParser { + + /// Parsing order (issue #25): + /// 1. Patch count from `No of test patches = N`. + /// 2. JSON objects; prefer one with `event == "report"` or `*_de2000` keys. + /// 3. Legacy text: `Profile check complete, errors(CIEDE2000): max. = X, avg. = Y, RMS = Z`. + /// 4. Broad regex fallback. + /// 5. If no metrics found, return a report whose `warning` is set. + public static func parse(_ output: String) -> ProfcheckReport { + var report = ProfcheckReport() + + // 1. Patch count. + let patchRegex = try? NSRegularExpression( + pattern: #"No of test patches\s*=\s*(\d+)"#, + options: [.caseInsensitive] + ) + if let match = patchRegex?.firstMatch( + in: output, + options: [], + range: NSRange(output.startIndex..., in: output) + ), let range = Range(match.range(at: 1), in: output) { + let count = Int(output[range]) + report.patchCount = count + } + + // 2. JSON objects. + let jsonObjects = extractJSONObjects(from: output) + for object in jsonObjects { + if let event = object["event"] as? String, event == "report" { + if let parsed = metrics(from: object) { + apply(metrics: parsed, to: &report) + return report + } + } + if hasMetricKeys(object) { + if let parsed = metrics(from: object) { + apply(metrics: parsed, to: &report) + return report + } + } + } + + // 3. Legacy text. + let textRegex = try? NSRegularExpression( + pattern: #"Profile check complete, errors\(CIEDE2000\): max\.\s*=\s*([0-9.]+),\s*avg\.\s*=\s*([0-9.]+),\s*RMS\s*=\s*([0-9.]+)"#, + options: [.caseInsensitive] + ) + if let match = textRegex?.firstMatch( + in: output, + options: [], + range: NSRange(output.startIndex..., in: output) + ) { + let numbers = (1...3).compactMap { i -> Double? in + guard let range = Range(match.range(at: i), in: output) else { return nil } + return Double(output[range]) + } + if numbers.count == 3 { + report.maxDE = numbers[0] + report.avgDE = numbers[1] + report.rmsDE = numbers[2] + report.status = report.avgDE.map { VerificationStatus.from(avgDE: $0) } + return report + } + } + + // 4. Broad regex fallback. + if let fallback = parseRegexFallback(output) { + var merged = fallback + merged.patchCount = report.patchCount + return merged + } + + // 5. Unparseable. + report.warning = "profcheck output did not contain a recognisable report." + return report + } + + // MARK: - JSON extraction + + private static func extractJSONObjects(from output: String) -> [[String: Any]] { + var objects: [[String: Any]] = [] + var start: String.Index? + var depth = 0 + + for index in output.indices { + let char = output[index] + if char == "{" { + if depth == 0 { + start = index + } + depth += 1 + } else if char == "}" { + if depth > 0 { + depth -= 1 + if depth == 0, let start = start { + let jsonString = String(output[start...index]) + if let data = jsonString.data(using: .utf8), + let object = try? JSONSerialization.jsonObject(with: data) as? [String: Any] { + objects.append(object) + } + } + } + } + } + + return objects + } + + private static func hasMetricKeys(_ object: [String: Any]) -> Bool { + let keys = [ + "avg_de", "avg_de2000", + "peak_de", "peak_de2000", "max_de", + "rms", "rms_de" + ] + return keys.contains { object[$0] != nil } + } + + private struct Metrics { + var avg: Double? + var max: Double? + var rms: Double? + } + + private static func metrics(from object: [String: Any]) -> Metrics? { + var m = Metrics() + m.avg = doubleValue(for: "avg_de2000", in: object) + ?? doubleValue(for: "avg_de", in: object) + m.max = doubleValue(for: "peak_de2000", in: object) + ?? doubleValue(for: "peak_de", in: object) + ?? doubleValue(for: "max_de", in: object) + ?? doubleValue(for: "max_de2000", in: object) + m.rms = doubleValue(for: "rms", in: object) + ?? doubleValue(for: "rms_de", in: object) + ?? doubleValue(for: "rms_de2000", in: object) + + guard m.avg != nil || m.max != nil || m.rms != nil else { return nil } + return m + } + + private static func doubleValue(for key: String, in object: [String: Any]) -> Double? { + if let number = object[key] as? Double { return number } + if let number = object[key] as? NSNumber { return number.doubleValue } + if let string = object[key] as? String { return Double(string) } + return nil + } + + private static func apply(metrics: Metrics, to report: inout ProfcheckReport) { + report.avgDE = metrics.avg + report.maxDE = metrics.max + report.rmsDE = metrics.rms + if let avg = metrics.avg { + report.status = VerificationStatus.from(avgDE: avg) + } + } + + // MARK: - Regex fallback + + private static func parseRegexFallback(_ output: String) -> ProfcheckReport? { + var report = ProfcheckReport() + + let avgRegex = try? NSRegularExpression( + pattern: #"(?:avg\.?|average)\s*(?:=|:)\s*([0-9.]+)"#, + options: [.caseInsensitive] + ) + let maxRegex = try? NSRegularExpression( + pattern: #"(?:max\.?|peak|maximum)\s*(?:=|:)\s*([0-9.]+)"#, + options: [.caseInsensitive] + ) + let rmsRegex = try? NSRegularExpression( + pattern: #"(?:rms)\s*(?:=|:)\s*([0-9.]+)"#, + options: [.caseInsensitive] + ) + + report.avgDE = firstDouble(from: output, regex: avgRegex) + report.maxDE = firstDouble(from: output, regex: maxRegex) + report.rmsDE = firstDouble(from: output, regex: rmsRegex) + + guard report.avgDE != nil || report.maxDE != nil || report.rmsDE != nil else { + return nil + } + + if let avg = report.avgDE { + report.status = VerificationStatus.from(avgDE: avg) + } + + return report + } + + private static func firstDouble(from output: String, regex: NSRegularExpression?) -> Double? { + guard let regex = regex, + let match = regex.firstMatch( + in: output, + options: [], + range: NSRange(output.startIndex..., in: output) + ), + let range = Range(match.range(at: 1), in: output) else { return nil } + return Double(output[range]) + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckReport.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckReport.swift new file mode 100644 index 0000000..7e14370 --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfcheckReport.swift @@ -0,0 +1,31 @@ +import Foundation + +/// Parsed result from a `profcheck -u` run. +public struct ProfcheckReport: Sendable, Equatable, Codable { + public var patchCount: Int? + public var avgDE: Double? + public var maxDE: Double? + public var rmsDE: Double? + public var status: VerificationStatus? + public var warning: String? + + public var isValid: Bool { + avgDE != nil && maxDE != nil && rmsDE != nil + } + + public init( + patchCount: Int? = nil, + avgDE: Double? = nil, + maxDE: Double? = nil, + rmsDE: Double? = nil, + status: VerificationStatus? = nil, + warning: String? = nil + ) { + self.patchCount = patchCount + self.avgDE = avgDE + self.maxDE = maxDE + self.rmsDE = rmsDE + self.status = status + self.warning = warning + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfileInstaller.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfileInstaller.swift new file mode 100644 index 0000000..a81dda8 --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfileInstaller.swift @@ -0,0 +1,172 @@ +import Foundation + +/// Errors thrown by `ProfileInstaller`. +public enum ProfileInstallError: LocalizedError, Equatable, Sendable { + case unsafeStem(String) + case sourceMissing + case sourceNotProfile + case sourceTooSmall + case systemRequiresAdminRights + case copyFailed(String) + case cancelled + + public var errorDescription: String? { + switch self { + case .unsafeStem(let stem): + return "Profile name contains unsafe characters: \(stem)" + case .sourceMissing: + return "Source profile does not exist" + case .sourceNotProfile: + return "Source must be a .icc or .icm file" + case .sourceTooSmall: + return "Source file is too small to be a valid profile" + case .systemRequiresAdminRights: + return "Installing to /Library/ColorSync/Profiles requires administrator rights" + case .copyFailed(let reason): + return "Could not install profile: \(reason)" + case .cancelled: + return "Install cancelled" + } + } +} + +/// Installs an ICC/ICM profile into the OS colour store. +public enum ProfileInstaller { + + /// Installs `sourceURL` into `~/Library/ColorSync/Profiles` or + /// `/Library/ColorSync/Profiles`. Always copies, never moves. + public static func install(config: InstallProfileConfig) throws -> InstallProfileResult { + let fm = FileManager.default + + // Source validation. + let sourceURL = config.sourceURL + guard fm.fileExists(atPath: sourceURL.path) else { + throw ProfileInstallError.sourceMissing + } + + let ext = sourceURL.pathExtension.lowercased() + guard ext == "icc" || ext == "icm" else { + throw ProfileInstallError.sourceNotProfile + } + + let attrs = try? fm.attributesOfItem(atPath: sourceURL.path) + let size = attrs?[.size] as? UInt64 ?? 0 + guard size >= 128 else { + throw ProfileInstallError.sourceTooSmall + } + + // Stem security. + let stem = sourceURL.deletingPathExtension().lastPathComponent + guard !stem.contains("..") && !stem.contains("/") && !stem.contains("\\") else { + throw ProfileInstallError.unsafeStem(stem) + } + + // Destination directory. + let destDir: URL + if config.options.preferSystem { + destDir = URL(fileURLWithPath: "/Library/ColorSync/Profiles") + } else { + let home = fm.homeDirectoryForCurrentUser + destDir = home.appendingPathComponent("Library/ColorSync/Profiles") + } + + // Ensure parent exists. + try? fm.createDirectory(at: destDir, withIntermediateDirectories: true) + + let destURL = destDir.appendingPathComponent("\(stem).icc") + + // Collision resolution. + let destExists = fm.fileExists(atPath: destURL.path) + if destExists { + if config.options.forceOverwrite { + // Continue to overwrite path. + } else if config.options.collisionPolicy == .rename { + let epoch = Int(Date().timeIntervalSince1970) + let renamedURL = destDir.appendingPathComponent("\(stem)-\(epoch).icc") + return try performInstall( + from: sourceURL, + to: renamedURL, + options: config.options, + overwritten: false, + renamed: true + ) + } else if config.options.collisionPolicy == .cancel { + throw ProfileInstallError.cancelled + } else { + // Default with askBeforeOverwrite — the app must decide. + throw ProfileInstallError.copyFailed("destination already exists") + } + } + + return try performInstall( + from: sourceURL, + to: destURL, + options: config.options, + overwritten: destExists, + renamed: false + ) + } + + private static func performInstall( + from sourceURL: URL, + to destURL: URL, + options: InstallProfileOptions, + overwritten: Bool, + renamed: Bool + ) throws -> InstallProfileResult { + let fm = FileManager.default + let tmpURL = destURL.appendingPathExtension("iccery-install.tmp") + + // Remove stale tmp. + try? fm.removeItem(at: tmpURL) + + do { + try fm.copyItem(at: sourceURL, to: tmpURL) + + if fm.fileExists(atPath: destURL.path) { + _ = try fm.replaceItemAt(destURL, withItemAt: tmpURL) + } else { + try fm.moveItem(at: tmpURL, to: destURL) + } + } catch { + try? fm.removeItem(at: tmpURL) + + // Surface a clear admin-rights hint when writing to system. + if destURL.path.hasPrefix("/Library/") && !fm.fileExists(atPath: destURL.path) { + throw ProfileInstallError.systemRequiresAdminRights + } + throw ProfileInstallError.copyFailed(error.localizedDescription) + } + + let registered = fm.fileExists(atPath: destURL.path) + + var openedPanel = false + if options.openColorPanel { + openedPanel = openColorSyncUtility() + } + + return InstallProfileResult( + destPath: destURL.path, + registered: registered, + overwritten: overwritten, + renamed: renamed, + openedPanel: openedPanel, + message: "Profile installed to \(destURL.path)", + calibrationNote: options.calibrationNote + ) + } + + private static func openColorSyncUtility() -> Bool { + let task = Process() + task.launchPath = "/usr/bin/open" + task.arguments = ["-a", "ColorSync Utility"] + task.environment = ["ARGYLL_NOT_INTERACTIVE": "1"] + do { + try task.run() + task.waitUntilExit() + } catch { + return false + } + return true + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationHistoryStore.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationHistoryStore.swift new file mode 100644 index 0000000..6a06d97 --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationHistoryStore.swift @@ -0,0 +1,120 @@ +import Foundation + +/// Persistence for `VerificationRecord` entries. +public actor VerificationHistoryStore { + + /// Default cap. + public static let defaultCapacity = 1000 + + /// Path to the JSON store. + public let url: URL + + /// In-memory cache, kept in sync with disk. + private var records: [VerificationRecord] = [] + + private let capacity: Int + private let encoder: JSONEncoder + private let decoder: JSONDecoder + + public init( + url: URL = AppPaths.appDataDir.appendingPathComponent("verification_history.json"), + capacity: Int = defaultCapacity + ) { + self.url = url + self.capacity = capacity + + self.encoder = JSONEncoder() + self.encoder.dateEncodingStrategy = .iso8601 + self.encoder.outputFormatting = [.prettyPrinted, .sortedKeys] + + self.decoder = JSONDecoder() + self.decoder.dateDecodingStrategy = .iso8601 + } + + /// Loads records from disk. Returns the existing cache if already loaded. + /// + /// Throws when the file exists but cannot be parsed; the existing file + /// is never overwritten in that case. + public func load() throws -> [VerificationRecord] { + guard records.isEmpty else { return records } + let fm = FileManager.default + guard fm.fileExists(atPath: url.path), + let data = try? Data(contentsOf: url) else { return [] } + records = try decoder.decode([VerificationRecord].self, from: data) + return records + } + + /// Returns all records. + public func all() -> [VerificationRecord] { + records + } + + /// Records matching the optional printer filter. + public func filtered(by printer: String?) -> [VerificationRecord] { + guard let printer = printer, !printer.isEmpty else { return records } + return records.filter { $0.printerName == printer } + } + + /// Appends a record, trims to capacity, and writes atomically. + /// + /// Returns the trimmed list, or `nil` if a write error occurs so the + /// caller can surface the failure without replacing the in-memory list. + @discardableResult + public func append(_ record: VerificationRecord) throws -> [VerificationRecord] { + var updated = records + updated.append(record) + if updated.count > capacity { + updated.sort { $0.timestamp < $1.timestamp } + updated = Array(updated.suffix(capacity)) + } + + try write(updated) + records = updated + return updated + } + + /// Removes all history and updates disk. + public func clear() throws { + try write([]) + records = [] + } + + /// RFC-4180 CSV export. + public func exportCSV() -> String { + var lines: [String] = [ + csvRow(["id", "profile_name", "printer_name", "avg_de", "max_de", "rms_de", "patch_count", "status", "timestamp"]) + ] + + for record in records { + lines.append(csvRow([ + record.id, + record.profileName, + record.printerName, + String(record.avgDE), + String(record.maxDE), + String(record.rmsDE), + String(record.patchCount), + record.status.rawValue, + ISO8601DateFormatter().string(from: record.timestamp) + ])) + } + + return lines.joined(separator: "\n") + "\n" + } + + /// Writes `records` through a temp file and rename. + private func write(_ records: [VerificationRecord]) throws { + let data = try encoder.encode(records) + try AtomicFileWriter.write(data, to: url) + } + + private func csvRow(_ fields: [String]) -> String { + fields.map { field in + let escaped = field.replacingOccurrences(of: "\"", with: "\"\"") + if field.contains(",") || field.contains("\"") || field.contains("\n") || field.contains("\r") { + return "\"\(escaped)\"" + } + return escaped + }.joined(separator: ",") + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationRecord.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationRecord.swift new file mode 100644 index 0000000..238991d --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationRecord.swift @@ -0,0 +1,36 @@ +import Foundation + +/// A single entry in the verification history store. +public struct VerificationRecord: Sendable, Equatable, Codable, Identifiable { + public var id: String + public var profileName: String + public var printerName: String + public var avgDE: Double + public var maxDE: Double + public var rmsDE: Double + public var patchCount: Int + public var status: VerificationStatus + public var timestamp: Date + + public init( + id: String, + profileName: String, + printerName: String, + avgDE: Double, + maxDE: Double, + rmsDE: Double, + patchCount: Int, + status: VerificationStatus, + timestamp: Date + ) { + self.id = id + self.profileName = profileName + self.printerName = printerName + self.avgDE = avgDE + self.maxDE = maxDE + self.rmsDE = rmsDE + self.patchCount = patchCount + self.status = status + self.timestamp = timestamp + } +} diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationStatus.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationStatus.swift new file mode 100644 index 0000000..7791e46 --- /dev/null +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationStatus.swift @@ -0,0 +1,32 @@ +import Foundation + +/// ICCery quality band for a verification run. +/// +/// Bands are on the **average** ΔE₀₀: +/// - < 1.0 → Excellent +/// - < 2.0 → Good +/// - < 3.5 → Acceptable +/// - ≥ 3.5 → Warning +public enum VerificationStatus: String, Sendable, Equatable, Codable, CaseIterable { + case excellent + case good + case acceptable + case poor + + public var displayName: String { + switch self { + case .excellent: return "Excellent" + case .good: return "Good" + case .acceptable: return "Acceptable" + case .poor: return "Warning" + } + } + + /// Returns the quality band for the given average ΔE₀₀. + public static func from(avgDE: Double) -> VerificationStatus { + if avgDE < 1.0 { return .excellent } + if avgDE < 2.0 { return .good } + if avgDE < 3.5 { return .acceptable } + return .poor + } +} diff --git a/Sources/ICCery/AppEnvironment.swift b/Sources/ICCery/AppEnvironment.swift index 5d7c0e9..aeb404c 100644 --- a/Sources/ICCery/AppEnvironment.swift +++ b/Sources/ICCery/AppEnvironment.swift @@ -12,6 +12,7 @@ struct AppEnvironment: Sendable { let presetStore: PresetStore let runner: ArgyllRunner let cupsService: CupsService + let historyStore: VerificationHistoryStore static func live( environment: [String: String] = ProcessInfo.processInfo.environment @@ -38,7 +39,8 @@ struct AppEnvironment: Sendable { ), cupsService: CupsService( processManager: .shared, - binaryDir: cupsDir) + binaryDir: cupsDir), + historyStore: VerificationHistoryStore() ) } } diff --git a/Sources/ICCery/DriftChartView.swift b/Sources/ICCery/DriftChartView.swift new file mode 100644 index 0000000..e2e8fa9 --- /dev/null +++ b/Sources/ICCery/DriftChartView.swift @@ -0,0 +1,121 @@ +import SwiftUI +import ICCeryCore + +/// Minimal line chart for drift history without depending on the +/// `Charts` framework link. Renders avg/max series with shaded quality +/// bands. +struct DriftChartView: View { + let records: [VerificationRecord] + + var body: some View { + GeometryReader { geometry in + let width = geometry.size.width + let height = geometry.size.height + + ZStack(alignment: .topLeading) { + if let (_, _, _, maxV) = scales(in: height) { + // Quality bands — bottom (red, > 3.5) drawn first, then + // orange, yellow, green so the upper-most bands overlay. + band(from: 3.5, to: maxV, color: .red.opacity(0.12), height: height, maxValue: maxV) + band(from: 2.0, to: 3.5, color: .orange.opacity(0.12), height: height, maxValue: maxV) + band(from: 1.0, to: 2.0, color: .yellow.opacity(0.12), height: height, maxValue: maxV) + band(from: 0.0, to: 1.0, color: .green.opacity(0.12), height: height, maxValue: maxV) + } + + if !records.isEmpty, let (minT, maxT, minV, maxV) = scales(in: height) { + // Average ΔE series + Path { path in + for (index, record) in records.enumerated() { + let pt = point( + for: record, + minTime: minT, + maxTime: maxT, + minValue: minV, + maxValue: maxV, + width: width, + height: height, + keyPath: \.avgDE + ) + if index == 0 { + path.move(to: pt) + } else { + path.addLine(to: pt) + } + } + } + .stroke(Color.blue, lineWidth: 2) + .accessibilityIdentifier("driftAvgSeries") + + // Max ΔE series + Path { path in + for (index, record) in records.enumerated() { + let pt = point( + for: record, + minTime: minT, + maxTime: maxT, + minValue: minV, + maxValue: maxV, + width: width, + height: height, + keyPath: \.maxDE + ) + if index == 0 { + path.move(to: pt) + } else { + path.addLine(to: pt) + } + } + } + .stroke(Color.orange, lineWidth: 2) + .accessibilityIdentifier("driftMaxSeries") + } else { + Text("No data") + .font(.caption) + .foregroundStyle(.secondary) + .frame(maxWidth: .infinity, maxHeight: .infinity) + } + } + } + } + + private func band( + from lower: Double, + to upper: Double, + color: Color, + height: CGFloat, + maxValue: Double + ) -> some View { + let yTop = valueY(lower, minValue: 0, maxValue: maxValue, height: height) + let yBottom = valueY(upper, minValue: 0, maxValue: maxValue, height: height) + return color + .frame(height: yBottom - yTop) + .offset(y: yTop) + } + + private func scales(in height: CGFloat) -> (Date, Date, Double, Double)? { + guard let minT = records.first?.timestamp, let maxT = records.last?.timestamp else { return nil } + let maxV = max(records.map { max($0.avgDE, $0.maxDE) }.max() ?? 5.0, 5.0) + return (minT, maxT, 0.0, maxV) + } + + private func point( + for record: VerificationRecord, + minTime: Date, + maxTime: Date, + minValue: Double, + maxValue: Double, + width: CGFloat, + height: CGFloat, + keyPath: KeyPath + ) -> CGPoint { + let timeSpan = max(1, maxTime.timeIntervalSince(minTime)) + let x = width * CGFloat(record.timestamp.timeIntervalSince(minTime) / timeSpan) + let y = valueY(record[keyPath: keyPath], minValue: minValue, maxValue: maxValue, height: height) + return CGPoint(x: x, y: y) + } + + private func valueY(_ value: Double, minValue: Double, maxValue: Double, height: CGFloat) -> CGFloat { + let valueSpan = max(1, maxValue - minValue) + return height - height * CGFloat((value - minValue) / valueSpan) + } +} diff --git a/Sources/ICCery/ProfileWorkflowViewModel.swift b/Sources/ICCery/ProfileWorkflowViewModel.swift new file mode 100644 index 0000000..d1f0093 --- /dev/null +++ b/Sources/ICCery/ProfileWorkflowViewModel.swift @@ -0,0 +1,487 @@ +import Foundation +import Observation +import SwiftUI +import ICCeryCore + +/// User-facing FWA selection for the Stage 4 form. +enum ColprofFwaSelection: String, CaseIterable, Sendable, Equatable { + case none = "none" + case empty = "" + case D50 = "D50" + case D65 = "D65" + case custom = "custom" + + var displayName: String { + switch self { + case .none: return "None" + case .empty: return "Bare (-f)" + case .D50: return "D50" + case .D65: return "D65" + case .custom: return "Custom .sp" + } + } +} + +/// Stage 4/5 workflow: build a profile, verify it, track drift, and install. +@MainActor +@Observable +final class ProfileWorkflowViewModel { + + let wizard: WizardViewModel + let environment: AppEnvironment + private let fileDialogs = FileDialogService.shared + + // MARK: - Stage 4 form + + var algorithm: String = "l" // l | x | X | m + var quality: String = "m" // l | m | h | u + var intent: String = "" // usually empty at Stage 4 + var fwaSelection: ColprofFwaSelection = .none + var fwaCustomPath: String = "" + var illuminant: String = "" + var observer: String = "" + var inputViewingCond: String = "" + var outputViewingCond: String = "" + var profileDescription: String = "" + var copyright: String = "" + + // MARK: - Run state + + var isColprofRunning = false + var colprofLog: [String] = [] + var colprofProgress: String? + var lastError: String? + var createdProfileURL: URL? + + // MARK: - Stage 4/5 calibration (issue #24) + + var applyCalibration = false + var calibrationFile: String = "" + + // MARK: - Stage 5 verification (issue #25) + + var profcheckReport: ProfcheckReport? + var profcheckWarning: String? + var isProfcheckRunning = false + + // MARK: - History / drift (issue #26) + + var verificationHistory: [VerificationRecord] = [] + var driftPrinterFilter: String? = nil + var driftAlert: String? + var isHistoryStoreError: String? + + // MARK: - Install (issue #27) + + var installResult: InstallProfileResult? + var showingInstallCollision = false + var installCollisionMessage: String = "" + var pendingInstallOptions: InstallProfileOptions? + + init(wizard: WizardViewModel, environment: AppEnvironment) { + self.wizard = wizard + self.environment = environment + } + + // MARK: - Derived + + var canCreateProfile: Bool { + !wizard.basename.isEmpty && wizard.effectiveWorkingDirectory != nil && !isColprofRunning + } + + var canVerify: Bool { + createdProfileURL != nil && !isProfcheckRunning + } + + var fwaValue: String? { + switch fwaSelection { + case .none: return nil + case .empty: return "" + case .D50: return "D50" + case .D65: return "D65" + case .custom: return fwaCustomPath + } + } + + // MARK: - Preset application + + func applyPreset(_ preset: ProfilingPreset?) { + guard let preset else { return } + algorithm = preset.colprofAlgorithm ?? "l" + quality = preset.colprofQuality ?? "m" + intent = preset.colprofIntent ?? "" + + if let fwa = preset.colprofFwa { + switch fwa.lowercased() { + case "none": fwaSelection = .none + case "": fwaSelection = .empty + case "d50": fwaSelection = .D50 + case "d65": fwaSelection = .D65 + default: + fwaSelection = .custom + fwaCustomPath = fwa + } + } + + illuminant = preset.colprofIlluminant ?? "" + observer = preset.colprofObserver ?? "" + inputViewingCond = preset.colprofInputViewingCond ?? "" + outputViewingCond = preset.colprofOutputViewingCond ?? "" + } + + /// Stage 4 form values for saving into a custom preset. + func presetSnapshot() -> ( + algorithm: String, + quality: String, + intent: String?, + fwa: String?, + illuminant: String?, + observer: String?, + inputViewingCond: String?, + outputViewingCond: String? + ) { + ( + algorithm: algorithm, + quality: quality, + intent: intent.isEmpty ? nil : intent, + fwa: fwaValue, + illuminant: illuminant.isEmpty ? nil : illuminant, + observer: observer.isEmpty ? nil : observer, + inputViewingCond: inputViewingCond.isEmpty ? nil : inputViewingCond, + outputViewingCond: outputViewingCond.isEmpty ? nil : outputViewingCond + ) + } + + // MARK: - Stage 4: build profile + + func buildColprofConfig() -> ColprofConfig { + let description = profileDescription.isEmpty ? wizard.basename : profileDescription + return ColprofConfig( + algorithm: algorithm, + quality: quality, + intent: intent.isEmpty ? nil : intent, + fwa: fwaValue, + illuminant: illuminant.isEmpty ? nil : illuminant, + observer: observer.isEmpty ? nil : observer, + inputViewingCond: inputViewingCond.isEmpty ? nil : inputViewingCond, + outputViewingCond: outputViewingCond.isEmpty ? nil : outputViewingCond, + description: description, + copyright: copyright.isEmpty ? nil : copyright, + basename: wizard.basename, + workingDirectory: wizard.effectiveWorkingDirectory + ) + } + + func createProfile() { + guard canCreateProfile, let _ = wizard.effectiveWorkingDirectory else { return } + let config = buildColprofConfig() + + isColprofRunning = true + colprofLog = [] + colprofProgress = nil + lastError = nil + createdProfileURL = nil + + let runner = environment.runner + Task { @MainActor [weak self] in + guard let self else { return } + defer { self.isColprofRunning = false } + + do { + let url = try await runner.runColprof(config: config) { [weak self] batch in + Task { @MainActor [weak self] in + guard let self else { return } + self.colprofLog.append(contentsOf: batch) + if let last = batch.last { + let progress = ColprofProgressClassifier.classify(line: last) + self.updateProgress(progress) + } + } + } + + var finalProfileURL = url + + if self.applyCalibration, !self.calibrationFile.isEmpty { + let applyConfig = ApplycalConfig( + calibrationPath: self.calibrationFile, + inputProfileURL: url + ) + finalProfileURL = try await runner.runApplycal(config: applyConfig) + self.colprofLog.append("Calibration embedded: \(self.calibrationFile)") + } + + // Gamut extraction is best-effort for Stage 5 / M6 viewer. + do { + let gamConfig = IccgamutConfig(profileURL: finalProfileURL) + _ = try await runner.runIccgamut(config: gamConfig) { [weak self] batch in + Task { @MainActor [weak self] in + self?.colprofLog.append(contentsOf: batch) + } + } + self.colprofLog.append("Gamut mesh extracted.") + } catch { + self.wizard.showNotice( + "Gamut extraction skipped: \(error.localizedDescription)", + kind: .info + ) + } + + self.createdProfileURL = finalProfileURL + self.wizard.refreshGating() + self.wizard.showNotice("Profile created: \(finalProfileURL.lastPathComponent)") + self.wizard.go(to: .verifyInstall) + } catch { + self.lastError = error.localizedDescription + self.wizard.showNotice( + "Profile creation failed: \(error.localizedDescription)", + kind: .error + ) + } + } + } + + private func updateProgress(_ progress: ColprofProgress) { + switch progress { + case .gamutMapping: + colprofProgress = "Gamut mapping calculation…" + case .fittingClut: + colprofProgress = "Fitting cLUT grid points…" + case .writingIcc: + colprofProgress = "Writing ICC profile…" + case .unknown: + break + } + } + + // MARK: - Stage 5: verify profile + + var knownPrinters: [String] { + Array(Set(verificationHistory.map { $0.printerName })).sorted() + } + + func loadHistory() { + Task { @MainActor [weak self] in + guard let self else { return } + do { + self.verificationHistory = try await self.environment.historyStore.load() + self.driftAlert = DriftAlert.compute(from: self.filteredHistory) + } catch { + self.isHistoryStoreError = error.localizedDescription + self.wizard.showNotice( + "Could not load verification history: \(error.localizedDescription)", + kind: .error + ) + } + } + } + + var filteredHistory: [VerificationRecord] { + guard let filter = driftPrinterFilter, !filter.isEmpty else { + return verificationHistory + } + return verificationHistory.filter { $0.printerName == filter } + } + + func verifyProfile() { + guard canVerify, + let cwd = wizard.effectiveWorkingDirectory, + let profileURL = createdProfileURL else { return } + + let ti3URL = ArtefactProbe.artefact(wizard.basename, "ti3", cwd) + let config = ProfcheckConfig(ti3URL: ti3URL, iccURL: profileURL) + + isProfcheckRunning = true + profcheckReport = nil + profcheckWarning = nil + + let runner = environment.runner + Task { @MainActor [weak self] in + guard let self else { return } + defer { self.isProfcheckRunning = false } + + do { + let report = try await runner.runProfcheck(config: config) { [weak self] batch in + Task { @MainActor [weak self] in + self?.colprofLog.append(contentsOf: batch) + } + } + self.profcheckReport = report + if let record = self.makeVerificationRecord(from: report) { + let updated = try await self.environment.historyStore.append(record) + self.verificationHistory = updated + self.driftAlert = DriftAlert.compute(from: self.filteredHistory) + } + } catch let error as ArgyllRunnerError where error == .profcheckUnparseable { + self.profcheckWarning = "profcheck output could not be parsed." + self.profcheckReport = ProfcheckReport(warning: self.profcheckWarning) + } catch { + self.profcheckWarning = error.localizedDescription + self.profcheckReport = ProfcheckReport(warning: self.profcheckWarning) + self.wizard.showNotice( + "Verification failed: \(error.localizedDescription)", + kind: .error + ) + } + } + } + + func makeVerificationRecord(from report: ProfcheckReport) -> VerificationRecord? { + guard let avg = report.avgDE, + let max = report.maxDE, + let rms = report.rmsDE, + let status = report.status else { return nil } + + let timestamp = Date() + let id = "vr-\(Int(timestamp.timeIntervalSince1970))-\(Self.nextSeq())" + return VerificationRecord( + id: id, + profileName: createdProfileURL?.lastPathComponent ?? wizard.basename, + printerName: wizard.printerName ?? "", + avgDE: avg, + maxDE: max, + rmsDE: rms, + patchCount: report.patchCount ?? 0, + status: status, + timestamp: timestamp + ) + } + + private static func nextSeq() -> Int { + Int.random(in: 0..<1_000_000) + } + + func clearHistory() { + Task { @MainActor [weak self] in + guard let self else { return } + do { + try await self.environment.historyStore.clear() + self.verificationHistory = [] + self.driftAlert = nil + } catch { + self.wizard.showNotice( + "Could not clear history: \(error.localizedDescription)", + kind: .error + ) + } + } + } + + // MARK: - Profile install + + func beginInstallProfile() { + guard let sourceURL = createdProfileURL, + let _ = wizard.effectiveWorkingDirectory else { return } + + let settings = environment.settingsStore.load() + let preferSystem = settings.defaultInstallLocation == .system + let options = InstallProfileOptions( + forceOverwrite: !settings.askBeforeOverwriteProfile, + preferSystem: preferSystem, + collisionPolicy: .overwrite, + openColorPanel: settings.openColorPanelAfterInstall + ) + + let destURL = installDestination(for: sourceURL, options: options) + let collision = FileManager.default.fileExists(atPath: destURL.path) + + if collision && settings.askBeforeOverwriteProfile { + pendingInstallOptions = options + installCollisionMessage = "A profile named \(destURL.lastPathComponent) already exists." + showingInstallCollision = true + return + } + + runInstall(sourceURL: sourceURL, options: options) + } + + func resolveInstallCollision(policy: ProfileCollisionPolicy) { + showingInstallCollision = false + guard let sourceURL = createdProfileURL, + var options = pendingInstallOptions else { return } + options.collisionPolicy = policy + if policy == .cancel { + installResult = InstallProfileResult( + destPath: "", + registered: false, + overwritten: false, + renamed: false, + openedPanel: false, + message: "Install cancelled." + ) + return + } + runInstall(sourceURL: sourceURL, options: options) + } + + private func installDestination(for sourceURL: URL, options: InstallProfileOptions) -> URL { + let stem = sourceURL.deletingPathExtension().lastPathComponent + let fm = FileManager.default + let destDir: URL + if options.preferSystem { + destDir = URL(fileURLWithPath: "/Library/ColorSync/Profiles") + } else { + destDir = fm.homeDirectoryForCurrentUser + .appendingPathComponent("Library/ColorSync/Profiles") + } + return destDir.appendingPathComponent("\(stem).icc") + } + + private func runInstall(sourceURL: URL, options: InstallProfileOptions) { + let config = InstallProfileConfig(sourceURL: sourceURL, options: options) + Task(priority: .userInitiated) { [weak self] in + do { + let result = try ProfileInstaller.install(config: config) + await MainActor.run { [weak self] in + self?.installResult = result + self?.wizard.showNotice(result.message) + } + } catch { + await MainActor.run { [weak self] in + self?.wizard.showNotice( + "Install failed: \(error.localizedDescription)", + kind: .error + ) + } + } + } + } + + func exportHistory() { + guard let url = fileDialogs.selectCsvSavePath() else { return } + Task { @MainActor [weak self] in + guard let self else { return } + let csv = await self.environment.historyStore.exportCSV() + do { + try csv.write(to: url, atomically: true, encoding: .utf8) + self.wizard.showNotice("History exported: \(url.lastPathComponent)") + } catch { + self.wizard.showNotice( + "Export failed: \(error.localizedDescription)", + kind: .error + ) + } + } + } + + // MARK: - File pickers + + func browseForSpectrumFile() { + let start = wizard.effectiveWorkingDirectory + let url = UITestHooks.isEnabled + ? nil + : fileDialogs.selectSpectrumFile(startingAt: start) + if let url { + fwaSelection = .custom + fwaCustomPath = url.path + } + } + + func browseForCalibrationFile() { + let start = wizard.effectiveWorkingDirectory + let url = fileDialogs.selectCalFile(startingAt: start) + if let url { + calibrationFile = url.path + applyCalibration = true + } + } +} diff --git a/Sources/ICCery/RootView.swift b/Sources/ICCery/RootView.swift index b486356..48d1ff2 100644 --- a/Sources/ICCery/RootView.swift +++ b/Sources/ICCery/RootView.swift @@ -64,6 +64,10 @@ struct RootView: View { Stage2View(workflow: workflow) case .measure: Stage3View(model: workflow.measurement) + case .buildProfile: + Stage4View(model: workflow.profile) + case .verifyInstall: + Stage5View(model: workflow.profile) default: StagePlaceholderView(stage: model.stage) } diff --git a/Sources/ICCery/Stage4View.swift b/Sources/ICCery/Stage4View.swift new file mode 100644 index 0000000..3fe4a08 --- /dev/null +++ b/Sources/ICCery/Stage4View.swift @@ -0,0 +1,194 @@ +import SwiftUI +import ICCeryCore + +/// Stage 4 — build an ICC/ICM profile from the canonical `.ti3`. +struct Stage4View: View { + @Bindable var model: ProfileWorkflowViewModel + + var body: some View { + VStack(spacing: 0) { + header + ScrollView { + VStack(alignment: .leading, spacing: 16) { + formSection + runSection + } + .padding(20) + } + } + .frame(maxWidth: .infinity, maxHeight: .infinity) + .background(Theme.background) + } + + // MARK: - Header + + @ViewBuilder + private var header: some View { + HStack(alignment: .firstTextBaseline) { + VStack(alignment: .leading, spacing: 4) { + Text(model.wizard.basename) + .font(.title3) + .foregroundStyle(Theme.text) + .accessibilityIdentifier("stage4TargetBasename") + Text("Build the ICC profile from the measured .ti3.") + .font(.callout) + .foregroundStyle(.secondary) + .accessibilityIdentifier("stage4TargetMeta") + } + + Spacer() + + if let progress = model.colprofProgress, model.isColprofRunning { + Text(progress) + .font(.caption) + .padding(.horizontal, 8) + .padding(.vertical, 4) + .background(Theme.border) + .cornerRadius(4) + .foregroundStyle(Theme.text) + .accessibilityIdentifier("colprofProgress") + } + } + .padding(16) + .background(Theme.panel) + } + + // MARK: - Form + + @ViewBuilder + private var formSection: some View { + VStack(alignment: .leading, spacing: 12) { + Text("Profile settings") + .font(.headline) + .foregroundStyle(Theme.text) + + HStack(spacing: 16) { + Picker("Algorithm", selection: $model.algorithm) { + Text("Lab cLUT").tag("l") + Text("XYZ cLUT").tag("x") + Text("Display XYZ+matrix").tag("X") + Text("Matrix").tag("m") + } + .accessibilityIdentifier("colprofAlgorithm") + + Picker("Quality", selection: $model.quality) { + Text("Low").tag("l") + Text("Medium").tag("m") + Text("High").tag("h") + Text("Ultra").tag("u") + } + .accessibilityIdentifier("colprofQuality") + } + + Picker("FWA / OBA compensation", selection: $model.fwaSelection) { + ForEach(ColprofFwaSelection.allCases, id: \.self) { selection in + Text(selection.displayName).tag(selection) + } + } + .accessibilityIdentifier("colprofFwa") + + if model.fwaSelection == .custom { + HStack { + TextField("Custom .sp spectrum path", text: $model.fwaCustomPath) + .textFieldStyle(.roundedBorder) + .accessibilityIdentifier("colprofFwaCustomPath") + Button("Browse…") { model.browseForSpectrumFile() } + .accessibilityIdentifier("btnBrowseFwaSp") + } + } + + HStack(spacing: 16) { + TextField("Illuminant", text: $model.illuminant) + .textFieldStyle(.roundedBorder) + .accessibilityIdentifier("colprofIlluminant") + + TextField("Observer", text: $model.observer) + .textFieldStyle(.roundedBorder) + .accessibilityIdentifier("colprofObserver") + } + + HStack(spacing: 16) { + TextField("Input viewing condition", text: $model.inputViewingCond) + .textFieldStyle(.roundedBorder) + .accessibilityIdentifier("colprofInputViewCond") + + TextField("Output viewing condition", text: $model.outputViewingCond) + .textFieldStyle(.roundedBorder) + .accessibilityIdentifier("colprofOutputViewCond") + } + + Text("Use 'none' to skip a viewing condition.") + .font(.caption) + .foregroundStyle(.secondary) + + TextField("Description", text: $model.profileDescription) + .textFieldStyle(.roundedBorder) + .accessibilityIdentifier("colprofDescription") + + TextField("Copyright", text: $model.copyright) + .textFieldStyle(.roundedBorder) + .accessibilityIdentifier("colprofCopyright") + + Toggle("Apply calibration curve", isOn: $model.applyCalibration) + .accessibilityIdentifier("colprofApplyCalibration") + + if model.applyCalibration { + HStack { + TextField("Calibration .cal file", text: $model.calibrationFile) + .textFieldStyle(.roundedBorder) + .accessibilityIdentifier("colprofCalibrationFile") + Button("Browse…") { model.browseForCalibrationFile() } + .accessibilityIdentifier("btnBrowseCalibrationFile") + } + } + } + .padding(16) + .background(Theme.panel) + } + + // MARK: - Run controls + + @ViewBuilder + private var runSection: some View { + VStack(alignment: .leading, spacing: 12) { + HStack(spacing: 12) { + Button("Create Profile") { + model.createProfile() + } + .disabled(!model.canCreateProfile) + .accessibilityIdentifier("btnCreateProfile") + + if model.isColprofRunning { + ProgressView() + .scaleEffect(0.8) + .accessibilityIdentifier("colprofProgressIndicator") + } + + Spacer() + + if let lastError = model.lastError { + Text(lastError) + .font(.caption) + .foregroundStyle(.red) + .accessibilityIdentifier("colprofLastError") + } + } + + if !model.colprofLog.isEmpty { + DisclosureGroup("Log") { + VStack(alignment: .leading) { + ForEach(model.colprofLog, id: \.self) { line in + Text(line) + .font(.system(.caption, design: .monospaced)) + .foregroundStyle(.secondary) + } + } + } + .foregroundStyle(Theme.text) + .accessibilityIdentifier("colprofLogContainer") + } + } + .padding(16) + .background(Theme.panel) + } +} diff --git a/Sources/ICCery/Stage5View.swift b/Sources/ICCery/Stage5View.swift new file mode 100644 index 0000000..af7b760 --- /dev/null +++ b/Sources/ICCery/Stage5View.swift @@ -0,0 +1,272 @@ +import SwiftUI +import ICCeryCore + +/// Stage 5 — verify the generated profile, track drift, and install. +struct Stage5View: View { + @Bindable var model: ProfileWorkflowViewModel + + var body: some View { + VStack(spacing: 0) { + header + ScrollView { + VStack(alignment: .leading, spacing: 16) { + verifySection + if let report = model.profcheckReport { + resultSection(report: report) + } + historySection + } + .padding(20) + } + } + .frame(maxWidth: .infinity, maxHeight: .infinity) + .background(Theme.background) + .onAppear { model.loadHistory() } + .alert("Install profile", isPresented: $model.showingInstallCollision) { + Button("Overwrite", role: .destructive) { + model.resolveInstallCollision(policy: .overwrite) + } + .accessibilityIdentifier("profileOverwriteBtn") + Button("Rename") { + model.resolveInstallCollision(policy: .rename) + } + .accessibilityIdentifier("profileRenameBtn") + Button("Cancel", role: .cancel) { + model.resolveInstallCollision(policy: .cancel) + } + .accessibilityIdentifier("profileCancelCollisionBtn") + } message: { + Text(model.installCollisionMessage) + .accessibilityIdentifier("profileInstallCollisionMessage") + } + } + + // MARK: - Header + + @ViewBuilder + private var header: some View { + HStack(alignment: .firstTextBaseline) { + VStack(alignment: .leading, spacing: 4) { + Text(model.wizard.basename) + .font(.title3) + .foregroundStyle(Theme.text) + .accessibilityIdentifier("stage5TargetBasename") + Text("Verify the profile and compare against historical results.") + .font(.callout) + .foregroundStyle(.secondary) + .accessibilityIdentifier("stage5TargetMeta") + } + + Spacer() + + if let alert = model.driftAlert { + Text(alert) + .font(.caption) + .padding(.horizontal, 8) + .padding(.vertical, 4) + .background(Color.red.opacity(0.2)) + .cornerRadius(4) + .foregroundStyle(.red) + .accessibilityIdentifier("driftAlert") + } + + if let warning = model.profcheckWarning, !warning.isEmpty, model.driftAlert == nil { + Text("⚠ \(warning)") + .font(.caption) + .padding(.horizontal, 8) + .padding(.vertical, 4) + .background(Color.red.opacity(0.2)) + .cornerRadius(4) + .foregroundStyle(.red) + .accessibilityIdentifier("profcheckWarningBanner") + } + } + .padding(16) + .background(Theme.panel) + } + + // MARK: - Verify + + @ViewBuilder + private var verifySection: some View { + VStack(alignment: .leading, spacing: 12) { + HStack(spacing: 12) { + Button("Verify Profile") { + model.verifyProfile() + } + .disabled(!model.canVerify) + .accessibilityIdentifier("btnVerifyProfile") + + if model.isProfcheckRunning { + ProgressView() + .scaleEffect(0.8) + .accessibilityIdentifier("profcheckProgressIndicator") + } + + Spacer() + + if let profileURL = model.createdProfileURL { + Text(profileURL.lastPathComponent) + .font(.caption) + .foregroundStyle(.secondary) + .accessibilityIdentifier("stage5ProfilePath") + } + } + + if !model.colprofLog.isEmpty { + DisclosureGroup("Log") { + VStack(alignment: .leading) { + ForEach(model.colprofLog, id: \.self) { line in + Text(line) + .font(.system(.caption, design: .monospaced)) + .foregroundStyle(.secondary) + } + } + } + .foregroundStyle(Theme.text) + .accessibilityIdentifier("profcheckLogContainer") + } + } + .padding(16) + .background(Theme.panel) + } + + // MARK: - Result cards + + @ViewBuilder + private func resultSection(report: ProfcheckReport) -> some View { + VStack(alignment: .leading, spacing: 12) { + HStack { + Text("Verification result") + .font(.headline) + .foregroundStyle(Theme.text) + + Spacer() + + Button("Install Profile") { model.beginInstallProfile() } + .disabled(model.createdProfileURL == nil) + .accessibilityIdentifier("btnInstallProfile") + } + + HStack(spacing: 16) { + metricCard(title: "Avg ΔE", value: report.avgDE) + metricCard(title: "Max ΔE", value: report.maxDE) + metricCard(title: "RMS", value: report.rmsDE) + metricCard(title: "Patches", value: report.patchCount.map(Double.init)) + } + + if let status = report.status { + HStack { + Text("Status") + Spacer() + Text(status.displayName) + .fontWeight(.semibold) + .foregroundStyle(statusColor(status)) + .accessibilityIdentifier("profcheckStatus") + } + } + } + .padding(16) + .background(Theme.panel) + } + + @ViewBuilder + private func metricCard(title: String, value: Double?) -> some View { + VStack(alignment: .leading, spacing: 4) { + Text(title) + .font(.caption) + .foregroundStyle(.secondary) + Text(value.map { String(format: "%.2f", $0) } ?? "—") + .font(.title3) + .foregroundStyle(Theme.text) + } + .frame(maxWidth: .infinity, alignment: .leading) + } + + // MARK: - History + + @ViewBuilder + private var historySection: some View { + VStack(alignment: .leading, spacing: 12) { + Text("History & drift") + .font(.headline) + .foregroundStyle(Theme.text) + + HStack { + Picker("Printer", selection: Binding( + get: { model.driftPrinterFilter ?? "" }, + set: { model.driftPrinterFilter = $0.isEmpty ? nil : $0 } + )) { + Text("All").tag("") + ForEach(model.knownPrinters, id: \.self) { printer in + Text(printer.isEmpty ? "Unknown" : printer).tag(printer) + } + } + .accessibilityIdentifier("driftPrinterFilter") + .frame(width: 200) + + Spacer() + + Button("Export CSV") { model.exportHistory() } + .accessibilityIdentifier("btnExportHistory") + + Button("Clear") { model.clearHistory() } + .accessibilityIdentifier("btnClearHistory") + } + + driftChart + + if !model.filteredHistory.isEmpty { + Table(of: VerificationRecord.self) { + TableColumn("Date") { record in + Text(record.timestamp.formatted(date: .numeric, time: .shortened)) + } + TableColumn("Profile") { record in + Text(record.profileName) + } + TableColumn("Avg") { record in + Text(String(format: "%.2f", record.avgDE)) + } + TableColumn("Max") { record in + Text(String(format: "%.2f", record.maxDE)) + } + TableColumn("RMS") { record in + Text(String(format: "%.2f", record.rmsDE)) + } + TableColumn("Status") { record in + Text(record.status.displayName) + .foregroundStyle(statusColor(record.status)) + } + } rows: { + ForEach(model.filteredHistory) { record in + TableRow(record) + } + } + .frame(minHeight: 120) + .accessibilityIdentifier("verificationHistoryTable") + } else { + Text("No verification records yet.") + .font(.callout) + .foregroundStyle(.secondary) + } + } + .padding(16) + .background(Theme.panel) + } + + @ViewBuilder + private var driftChart: some View { + let records = model.filteredHistory.sorted { $0.timestamp < $1.timestamp } + DriftChartView(records: records) + .frame(height: 160) + .accessibilityIdentifier("driftChart") + } + + private func statusColor(_ status: VerificationStatus) -> Color { + switch status { + case .excellent, .good: return .green + case .acceptable: return .yellow + case .poor: return .red + } + } +} diff --git a/Sources/ICCery/TargetWorkflowViewModel.swift b/Sources/ICCery/TargetWorkflowViewModel.swift index 3676e6d..be5e55c 100644 --- a/Sources/ICCery/TargetWorkflowViewModel.swift +++ b/Sources/ICCery/TargetWorkflowViewModel.swift @@ -122,6 +122,9 @@ final class TargetWorkflowViewModel { /// Stage 3 measurement workflow, owned at the app level so it persists /// across stage switches and can observe settings changes. var measurement: MeasurementWorkflowViewModel + /// Stage 4/5 profile workflow, owned at the app level so it persists + /// across stage switches and can apply preset values. + var profile: ProfileWorkflowViewModel init(environment: AppEnvironment = .live()) { self.environment = environment @@ -130,6 +133,10 @@ final class TargetWorkflowViewModel { wizard: wizard, environment: environment ) + self.profile = ProfileWorkflowViewModel( + wizard: wizard, + environment: environment + ) reloadPresets() } @@ -535,6 +542,8 @@ final class TargetWorkflowViewModel { } customSeed = preset.randomSeed ?? 1 + profile.applyPreset(preset) + selectedPresetID = preset.id } @@ -571,7 +580,17 @@ final class TargetWorkflowViewModel { bitDepth: bitDepth.rawValue, dpi: tiffDpi, randomSeed: layoutOrder == .deterministic ? 1 : customSeed, - noRandomize: layoutOrder == .raster + noRandomize: layoutOrder == .raster, + calibrationFile: profile.calibrationFile.isEmpty ? nil : profile.calibrationFile, + applyCalibration: profile.applyCalibration ? true : nil, + colprofAlgorithm: profile.algorithm, + colprofQuality: profile.quality, + colprofIntent: profile.intent.isEmpty ? nil : profile.intent, + colprofFwa: profile.fwaValue, + colprofIlluminant: profile.illuminant.isEmpty ? nil : profile.illuminant, + colprofObserver: profile.observer.isEmpty ? nil : profile.observer, + colprofInputViewingCond: profile.inputViewingCond.isEmpty ? nil : profile.inputViewingCond, + colprofOutputViewingCond: profile.outputViewingCond.isEmpty ? nil : profile.outputViewingCond ) do { try environment.presetStore.saveCustom(preset) diff --git a/Tests/ICCeryCoreTests/ApplycalArgsTests.swift b/Tests/ICCeryCoreTests/ApplycalArgsTests.swift new file mode 100644 index 0000000..9075509 --- /dev/null +++ b/Tests/ICCeryCoreTests/ApplycalArgsTests.swift @@ -0,0 +1,30 @@ +import Foundation +import Testing +@testable import ICCeryCore + +@Suite("ApplycalArgs") +struct ApplycalArgsTests { + + @Test("Apply argv") + func applyArgv() throws { + let config = ApplycalConfig( + calibrationPath: "/tmp/cal.cal", + inputProfileURL: URL(fileURLWithPath: "/tmp/profile.icc") + ) + let args = try ApplycalArgs.build(config: config) + #expect(args == ["-v", "-a", "/tmp/cal.cal", "/tmp/profile.icc"]) + } + + @Test("Unapply is never sent from build") + func unapplyNotEmitted() throws { + let config = ApplycalConfig( + calibrationPath: "/tmp/cal.cal", + inputProfileURL: URL(fileURLWithPath: "/tmp/profile.icc"), + unapply: true + ) + let args = try ApplycalArgs.build(config: config) + // Builder intentionally emits -u because config can set it, but + // the UI layer never passes unapply: true in v2.0. + #expect(args == ["-v", "-u", "/tmp/cal.cal", "/tmp/profile.icc"]) + } +} diff --git a/Tests/ICCeryCoreTests/ArgyllRunnerColprofTests.swift b/Tests/ICCeryCoreTests/ArgyllRunnerColprofTests.swift new file mode 100644 index 0000000..ce92e47 --- /dev/null +++ b/Tests/ICCeryCoreTests/ArgyllRunnerColprofTests.swift @@ -0,0 +1,52 @@ +import Foundation +import Testing +@testable import ICCeryCore + +final class LogHolder: @unchecked Sendable { + private let lock = NSLock() + private var _lines: [String] = [] + + func append(_ batch: [String]) { + lock.lock() + _lines.append(contentsOf: batch) + lock.unlock() + } + + var lines: [String] { + lock.lock() + defer { lock.unlock() } + return _lines + } +} + +@Suite("ArgyllRunner colprof") +struct ArgyllRunnerColprofTests { + + @Test("Mock colprof produces .icc") + func colprofProducesIcc() async throws { + let binDir = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + .appendingPathComponent("ICCeryUITests/Fixtures/bin") + let testRoot = FileManager.default.temporaryDirectory + .appendingPathComponent("colprof-test-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: testRoot, withIntermediateDirectories: true) + + let runner = ArgyllRunner( + processManager: .shared, + binaryResolver: BinaryResolver(overrideDir: binDir) + ) + + let holder = LogHolder() + let config = ColprofConfig(basename: "testrun", workingDirectory: testRoot) + let url = try await runner.runColprof(config: config) { batch in + holder.append(batch) + } + + #expect(url.lastPathComponent == "testrun.icc") + #expect(FileManager.default.fileExists(atPath: url.path)) + #expect(holder.lines.contains { $0.contains("Gamut mapping") }) + + try? FileManager.default.removeItem(at: testRoot) + } +} diff --git a/Tests/ICCeryCoreTests/ColprofArgsTests.swift b/Tests/ICCeryCoreTests/ColprofArgsTests.swift new file mode 100644 index 0000000..9b4f0f7 --- /dev/null +++ b/Tests/ICCeryCoreTests/ColprofArgsTests.swift @@ -0,0 +1,72 @@ +import Foundation +import Testing +@testable import ICCeryCore + +@Suite("ColprofArgs") +struct ColprofArgsTests { + + @Test("Default algorithm and quality") + func defaults() throws { + let config = ColprofConfig(basename: "target") + let args = try ColprofArgs.build(config: config) + #expect(args == ["-v", "-a", "l", "-q", "m", "target"]) + } + + @Test("FWA bare -f when empty string") + func fwaBareFlag() throws { + let config = ColprofConfig(fwa: "", basename: "target") + let args = try ColprofArgs.build(config: config) + #expect(args == ["-v", "-a", "l", "-q", "m", "-f", "target"]) + } + + @Test("FWA D50 and D65 emit -f value") + func fwaD50() throws { + let config = ColprofConfig(fwa: "D50", basename: "target") + let args = try ColprofArgs.build(config: config) + #expect(args.contains("-f")) + #expect(args.contains("D50")) + #expect(args.last == "target") + } + + @Test("FWA none is omitted") + func fwaNoneOmitted() throws { + let config = ColprofConfig(fwa: "none", basename: "target") + let args = try ColprofArgs.build(config: config) + #expect(!args.contains("-f")) + } + + @Test("Viewing conditions skip none") + func viewingCondNoneSkipped() throws { + let config = ColprofConfig( + inputViewingCond: "none", + outputViewingCond: "mt", + basename: "target" + ) + let args = try ColprofArgs.build(config: config) + #expect(!args.contains("-c")) + #expect(args.contains("-d")) + #expect(args.contains("mt")) + } + + @Test("Description falls back to basename when empty") + func descriptionFallback() throws { + let config = ColprofConfig(description: "", basename: "target") + let args = try ColprofArgs.build(config: config) + #expect(!args.contains("-D")) + } + + @Test("Copyright only when non-empty") + func copyright() throws { + let config = ColprofConfig(copyright: "Gronod 2026", basename: "target") + let args = try ColprofArgs.build(config: config) + #expect(args.contains("-C")) + #expect(args.contains("Gronod 2026")) + } + + @Test("No -u passed") + func noProgressJsonFlag() throws { + let config = ColprofConfig(basename: "target") + let args = try ColprofArgs.build(config: config) + #expect(!args.contains("-u")) + } +} diff --git a/Tests/ICCeryCoreTests/ColprofProgressTests.swift b/Tests/ICCeryCoreTests/ColprofProgressTests.swift new file mode 100644 index 0000000..922cf53 --- /dev/null +++ b/Tests/ICCeryCoreTests/ColprofProgressTests.swift @@ -0,0 +1,24 @@ +import Foundation +import Testing +@testable import ICCeryCore + +@Suite("ColprofProgress") +struct ColprofProgressTests { + + @Test("Classifies gamut mapping") + func gamutMapping() { + #expect(ColprofProgressClassifier.classify(line: "Gamut mapping calculation in progress") == .gamutMapping) + } + + @Test("Classifies fitting or clut") + func fitting() { + #expect(ColprofProgressClassifier.classify(line: "Fitting cLUT grid points") == .fittingClut) + #expect(ColprofProgressClassifier.classify(line: "clut table") == .fittingClut) + } + + @Test("Classifies writing") + func writing() { + #expect(ColprofProgressClassifier.classify(line: "Writing ICC profile header") == .writingIcc) + #expect(ColprofProgressClassifier.classify(line: "icc profile written") == .writingIcc) + } +} diff --git a/Tests/ICCeryCoreTests/DriftAlertTests.swift b/Tests/ICCeryCoreTests/DriftAlertTests.swift new file mode 100644 index 0000000..51ab7fa --- /dev/null +++ b/Tests/ICCeryCoreTests/DriftAlertTests.swift @@ -0,0 +1,63 @@ +import Foundation +import Testing +@testable import ICCeryCore + +@Suite("DriftAlert") +struct DriftAlertTests { + + @Test("No alert with fewer than two poor results") + func notEnough() { + let records = [ + record(avg: 4.0, at: 1000) + ] + #expect(DriftAlert.compute(from: records) == nil) + } + + @Test("Alert on two poor results one hour apart") + func oneHourApart() { + let records = [ + record(avg: 4.0, at: 1000), + record(avg: 5.0, at: 4600) + ] + #expect(DriftAlert.compute(from: records) != nil) + } + + @Test("No alert if same day and under one hour") + func sameDayUnderHour() { + let records = [ + record(avg: 4.0, at: 1000), + record(avg: 5.0, at: 2000) + ] + #expect(DriftAlert.compute(from: records) == nil) + } + + @Test("Alert on distinct days") + func distinctDays() { + let day1 = record(avg: 4.0, at: 0) + let day2 = record(avg: 5.0, at: 86400 + 1000) + #expect(DriftAlert.compute(from: [day1, day2]) != nil) + } + + @Test("Non-poor results do not trigger") + func nonPoor() { + let records = [ + record(avg: 1.0, at: 0), + record(avg: 1.5, at: 86400) + ] + #expect(DriftAlert.compute(from: records) == nil) + } + + private func record(avg: Double, at offset: TimeInterval) -> VerificationRecord { + VerificationRecord( + id: "vr-\(Int(offset))", + profileName: "p", + printerName: "", + avgDE: avg, + maxDE: avg, + rmsDE: avg, + patchCount: 1, + status: VerificationStatus.from(avgDE: avg), + timestamp: Date(timeIntervalSince1970: offset) + ) + } +} diff --git a/Tests/ICCeryCoreTests/IccgamutArgsTests.swift b/Tests/ICCeryCoreTests/IccgamutArgsTests.swift new file mode 100644 index 0000000..2ef2ba9 --- /dev/null +++ b/Tests/ICCeryCoreTests/IccgamutArgsTests.swift @@ -0,0 +1,16 @@ +import Foundation +import Testing +@testable import ICCeryCore + +@Suite("IccgamutArgs") +struct IccgamutArgsTests { + + @Test("Density is 10 and not a directory") + func densityNotDirectory() throws { + let config = IccgamutConfig( + profileURL: URL(fileURLWithPath: "/tmp/MyProfile.icc") + ) + let args = try IccgamutArgs.build(config: config) + #expect(args == ["-v", "-d", "10", "/tmp/MyProfile.icc"]) + } +} diff --git a/Tests/ICCeryCoreTests/ProfcheckArgsTests.swift b/Tests/ICCeryCoreTests/ProfcheckArgsTests.swift new file mode 100644 index 0000000..4ac887d --- /dev/null +++ b/Tests/ICCeryCoreTests/ProfcheckArgsTests.swift @@ -0,0 +1,17 @@ +import Foundation +import Testing +@testable import ICCeryCore + +@Suite("ProfcheckArgs") +struct ProfcheckArgsTests { + + @Test("Hard-coded argv") + func argv() throws { + let config = ProfcheckConfig( + ti3URL: URL(fileURLWithPath: "/tmp/target.ti3"), + iccURL: URL(fileURLWithPath: "/tmp/target.icc") + ) + let args = try ProfcheckArgs.build(config: config) + #expect(args == ["-v", "-k", "-s", "-u", "/tmp/target.ti3", "/tmp/target.icc"]) + } +} diff --git a/Tests/ICCeryCoreTests/ProfcheckParserTests.swift b/Tests/ICCeryCoreTests/ProfcheckParserTests.swift new file mode 100644 index 0000000..30ea74f --- /dev/null +++ b/Tests/ICCeryCoreTests/ProfcheckParserTests.swift @@ -0,0 +1,71 @@ +import Foundation +import Testing +@testable import ICCeryCore + +@Suite("ProfcheckParser") +struct ProfcheckParserTests { + + @Test("Prefers JSON report with de2000 keys") + func jsonReport() { + let output = """ + No of test patches = 52 + {"event": "report", "peak_de2000": 2.41, "avg_de2000": 0.85, "rms": 1.02} + Profile check complete, errors(CIEDE2000): max. = 9.99, avg. = 9.99, RMS = 9.99 + """ + let report = ProfcheckParser.parse(output) + #expect(report.isValid == true) + #expect(report.patchCount == 52) + #expect(report.avgDE == 0.85) + #expect(report.maxDE == 2.41) + #expect(report.rmsDE == 1.02) + #expect(report.status == .excellent) + } + + @Test("Falls back to legacy text") + func legacyText() { + let output = """ + No of test patches = 120 + Profile check complete, errors(CIEDE2000): max. = 3.50, avg. = 1.80, RMS = 0.95 + """ + let report = ProfcheckParser.parse(output) + #expect(report.isValid == true) + #expect(report.patchCount == 120) + #expect(report.avgDE == 1.80) + #expect(report.maxDE == 3.50) + #expect(report.rmsDE == 0.95) + #expect(report.status == .good) + } + + @Test("Broad regex fallback") + func regexFallback() { + let output = """ + No of test patches = 10 + avg = 4.25 + max = 6.10 + rms = 2.30 + """ + let report = ProfcheckParser.parse(output) + #expect(report.isValid == true) + #expect(report.avgDE == 4.25) + #expect(report.maxDE == 6.10) + #expect(report.rmsDE == 2.30) + #expect(report.status == .poor) + } + + @Test("Unparseable output warns, not zeros") + func unparseable() { + let output = "some random text without metrics" + let report = ProfcheckParser.parse(output) + #expect(report.isValid == false) + #expect(report.warning != nil) + #expect(report.avgDE == nil) + } + + @Test("Status bands") + func statusBands() { + #expect(VerificationStatus.from(avgDE: 0.5) == .excellent) + #expect(VerificationStatus.from(avgDE: 1.5) == .good) + #expect(VerificationStatus.from(avgDE: 2.5) == .acceptable) + #expect(VerificationStatus.from(avgDE: 4.0) == .poor) + } +} diff --git a/Tests/ICCeryCoreTests/ProfileInstallerTests.swift b/Tests/ICCeryCoreTests/ProfileInstallerTests.swift new file mode 100644 index 0000000..640a6c5 --- /dev/null +++ b/Tests/ICCeryCoreTests/ProfileInstallerTests.swift @@ -0,0 +1,75 @@ +import Foundation +import Testing +@testable import ICCeryCore + +@Suite("ProfileInstaller") +struct ProfileInstallerTests { + + @Test("Copies .icc to user ColorSync folder") + func userInstall() throws { + let fm = FileManager.default + let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try fm.createDirectory(at: tmp, withIntermediateDirectories: true) + + let source = tmp.appendingPathComponent("test.icc") + let iccData = Data(repeating: 0, count: 256) + try iccData.write(to: source) + + let colorsync = tmp.appendingPathComponent("Library/ColorSync/Profiles") + + // Inject a user profile install by replacing the home directory + // is not practical; instead exercise Core validation on a + // temp-only path via the file URL safety checks and the public + // install against a writable system-like path is tested below. + + // For this unit test, validate the stem security and source rules. + let unsafe = tmp.appendingPathComponent("bad..stem.icc") + try Data(repeating: 0, count: 256).write(to: unsafe) + do { + _ = try ProfileInstaller.install(config: InstallProfileConfig(sourceURL: unsafe)) + Issue.record("Expected unsafeStem error") + } catch let error as ProfileInstallError { + if case .unsafeStem = error { } else { Issue.record("Expected unsafeStem, got \(error)") } + } catch { + Issue.record("Unexpected error type: \(error)") + } + + let small = tmp.appendingPathComponent("tiny.icc") + try Data(repeating: 0, count: 64).write(to: small) + do { + _ = try ProfileInstaller.install(config: InstallProfileConfig(sourceURL: small)) + Issue.record("Expected sourceTooSmall error") + } catch let error as ProfileInstallError { + if case .sourceTooSmall = error { } else { Issue.record("Expected sourceTooSmall, got \(error)") } + } catch { + Issue.record("Unexpected error type: \(error)") + } + } + + @Test("Installs into a temp user folder and preserves source") + func tempInstallPreservesSource() throws { + let fm = FileManager.default + let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try fm.createDirectory(at: tmp, withIntermediateDirectories: true) + + let source = tmp.appendingPathComponent("m5_profile.icc") + try Data(repeating: 0, count: 256).write(to: source) + + let destDir = tmp.appendingPathComponent("ColorSync/Profiles") + try fm.createDirectory(at: destDir, withIntermediateDirectories: true) + + // There is no public API to override the home directory, so + // test the copy mechanism directly via file operations. + let dest = destDir.appendingPathComponent("m5_profile.icc") + let tmpDest = dest.appendingPathExtension("iccery-install.tmp") + try fm.copyItem(at: source, to: tmpDest) + if fm.fileExists(atPath: dest.path) { + _ = try fm.replaceItemAt(dest, withItemAt: tmpDest) + } else { + try fm.moveItem(at: tmpDest, to: dest) + } + + #expect(fm.fileExists(atPath: source.path)) + #expect(fm.fileExists(atPath: dest.path)) + } +} diff --git a/Tests/ICCeryCoreTests/VerificationHistoryStoreTests.swift b/Tests/ICCeryCoreTests/VerificationHistoryStoreTests.swift new file mode 100644 index 0000000..c478f5f --- /dev/null +++ b/Tests/ICCeryCoreTests/VerificationHistoryStoreTests.swift @@ -0,0 +1,79 @@ +import Foundation +import Testing +@testable import ICCeryCore + +@Suite("VerificationHistoryStore") +struct VerificationHistoryStoreTests { + + @Test("Append and cap") + func appendAndCap() async throws { + let fm = FileManager.default + let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try fm.createDirectory(at: tmp, withIntermediateDirectories: true) + let url = tmp.appendingPathComponent("verification_history.json") + + let store = VerificationHistoryStore(url: url, capacity: 3) + for i in 0..<5 { + let record = VerificationRecord( + id: "vr-\(i)", + profileName: "p", + printerName: "", + avgDE: Double(i), + maxDE: Double(i), + rmsDE: Double(i), + patchCount: i, + status: .good, + timestamp: Date(timeIntervalSince1970: TimeInterval(i)) + ) + _ = try await store.append(record) + } + + let all = await store.all() + #expect(all.count == 3) + #expect(all.first?.avgDE == 2.0) + } + + @Test("Parse failure preserves file") + func parseFailurePreservesFile() async { + let fm = FileManager.default + let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try? fm.createDirectory(at: tmp, withIntermediateDirectories: true) + let url = tmp.appendingPathComponent("verification_history.json") + + try? "not json".write(to: url, atomically: true, encoding: .utf8) + + let store = VerificationHistoryStore(url: url) + do { + _ = try await store.load() + Issue.record("load() should throw on invalid JSON") + } catch { + #expect(fm.fileExists(atPath: url.path)) + } + } + + @Test("CSV export quoting") + func csvQuoting() async throws { + let fm = FileManager.default + let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try fm.createDirectory(at: tmp, withIntermediateDirectories: true) + let url = tmp.appendingPathComponent("verification_history.json") + + let store = VerificationHistoryStore(url: url) + let record = VerificationRecord( + id: "a,b", + profileName: "\"quoted\"", + printerName: "", + avgDE: 1.0, + maxDE: 2.0, + rmsDE: 3.0, + patchCount: 1, + status: .good, + timestamp: Date(timeIntervalSince1970: 0) + ) + _ = try await store.append(record) + + let csv = await store.exportCSV() + #expect(csv.contains("\"a,b\"")) + #expect(csv.contains("\"\"quoted\"\"")) + } +} diff --git a/Tests/ICCeryUITests/Fixtures/bin/applycal b/Tests/ICCeryUITests/Fixtures/bin/applycal new file mode 100755 index 0000000..747cbeb --- /dev/null +++ b/Tests/ICCeryUITests/Fixtures/bin/applycal @@ -0,0 +1,18 @@ +#!/bin/sh +# Mock applycal for Milestone 5 UI tests. +# Copies the input profile to the optional output path. +while [ "$#" -gt 0 ]; do + case "$1" in + -v|-a|-u) shift ;; + *) break ;; + esac +done +cal="$1" +input="$2" +output="$3" +if [ -n "$output" ]; then + cp "$input" "$output" +else + cp "$cal" "$input.cal.ctl" +fi +exit 0 diff --git a/Tests/ICCeryUITests/Fixtures/bin/colprof b/Tests/ICCeryUITests/Fixtures/bin/colprof new file mode 100755 index 0000000..cdcd453 --- /dev/null +++ b/Tests/ICCeryUITests/Fixtures/bin/colprof @@ -0,0 +1,14 @@ +#!/bin/sh +# Mock colprof for Milestone 5 UI tests. +# Writes {basename}.icc next to the last argument and emits progress. +last="" +for arg in "$@"; do last="$arg"; done +if [ "${ICCERY_MOCK_COLPROF_EXIT:-0}" -ne 0 ]; then + echo "mock colprof failure" >&2 + exit "$ICCERY_MOCK_COLPROF_EXIT" +fi +echo "Gamut mapping calculation..." +echo "Fitting cLUT grid points..." +echo "Writing ICC profile..." +touch "$last.icc" +exit 0 diff --git a/Tests/ICCeryUITests/Fixtures/bin/iccgamut b/Tests/ICCeryUITests/Fixtures/bin/iccgamut new file mode 100755 index 0000000..9c0b74e --- /dev/null +++ b/Tests/ICCeryUITests/Fixtures/bin/iccgamut @@ -0,0 +1,13 @@ +#!/bin/sh +# Mock iccgamut for Milestone 5 UI tests. +# Writes {stem}.gam next to the profile path. +last="" +for arg in "$@"; do last="$arg"; done +if [ "${ICCERY_MOCK_ICCGAMUT_EXIT:-0}" -ne 0 ]; then + echo "mock iccgamut failure" >&2 + exit "$ICCERY_MOCK_ICCGAMUT_EXIT" +fi +stem=$(basename "$last" | sed 's/\.icc$//; s/\.icm$//') +dir=$(dirname "$last") +touch "$dir/$stem.gam" +exit 0 diff --git a/Tests/ICCeryUITests/Fixtures/bin/profcheck b/Tests/ICCeryUITests/Fixtures/bin/profcheck new file mode 100755 index 0000000..df49df8 --- /dev/null +++ b/Tests/ICCeryUITests/Fixtures/bin/profcheck @@ -0,0 +1,12 @@ +#!/bin/sh +# Mock profcheck for Milestone 5 UI tests. +# Emits a JSON report and legacy text summary. +if [ "${ICCERY_MOCK_PROFCHECK_EXIT:-0}" -ne 0 ]; then + echo "mock profcheck failure" >&2 + exit "$ICCERY_MOCK_PROFCHECK_EXIT" +fi +echo "No of test patches = 52" +sleep 0.1 +printf '{"event": "report", "peak_de2000": 2.41, "avg_de2000": 0.85, "rms": 1.02}\n' +echo "Profile check complete, errors(CIEDE2000): max. = 2.41, avg. = 0.85, RMS = 1.02" +exit 0 diff --git a/Tests/ICCeryUITests/Milestone3UITests.swift b/Tests/ICCeryUITests/Milestone3UITests.swift index a674288..b821f96 100644 --- a/Tests/ICCeryUITests/Milestone3UITests.swift +++ b/Tests/ICCeryUITests/Milestone3UITests.swift @@ -49,6 +49,23 @@ final class Milestone3UITests: XCTestCase { testRoot = nil } + private func waitForFileContent( + _ url: URL, + containing needle: String, + timeout: TimeInterval = 10 + ) -> String? { + let deadline = Date().addingTimeInterval(timeout) + while Date() < deadline { + if let data = try? Data(contentsOf: url), + let text = String(data: data, encoding: .utf8), + text.contains(needle) { + return text + } + RunLoop.current.run(until: Date().addingTimeInterval(0.1)) + } + return (try? String(contentsOf: url, encoding: .utf8)) ?? "" + } + private func launchApp() { app.launch() app.activate() @@ -171,6 +188,14 @@ final class Milestone3UITests: XCTestCase { reachPrintPanel() _ = waitFor("printerStatusBadge") + // Wait for the async printer enumeration to select a queue; once + // `btnPrintAll` is enabled, `btnPrintPage-0` is too. + let deadline = Date().addingTimeInterval(15) + while Date() < deadline, !app.buttons["btnPrintAll"].isEnabled { + RunLoop.current.run(until: Date().addingTimeInterval(0.1)) + } + XCTAssertTrue(app.buttons["btnPrintAll"].isEnabled) + app.buttons["btnPrintPage-0"].click() let argv = waitForLpLine() XCTAssertTrue(argv.contains("AP_ColorMatchingMode"), argv) @@ -202,18 +227,11 @@ final class Milestone3UITests: XCTestCase { let stateURL = testRoot .appendingPathComponent("AppData") .appendingPathComponent("wizard_state.json") - XCTAssertTrue(waitForFile(stateURL)) - let data = try Data(contentsOf: stateURL) - let state = String(data: data, encoding: .utf8) ?? "" - XCTAssertTrue(state.contains("Mock_Epson_7450"), state) + let state = waitForFileContent( + stateURL, containing: "Mock_Epson_7450", timeout: 15) + XCTAssertNotNil(state) + XCTAssertTrue((state ?? "").contains("Mock_Epson_7450")) } - private func waitForFile(_ url: URL, timeout: TimeInterval = 10) -> Bool { - let deadline = Date().addingTimeInterval(timeout) - while Date() < deadline { - if FileManager.default.fileExists(atPath: url.path) { return true } - RunLoop.current.run(until: Date().addingTimeInterval(0.1)) - } - return false - } + } diff --git a/Tests/ICCeryUITests/Milestone5UITests.swift b/Tests/ICCeryUITests/Milestone5UITests.swift new file mode 100644 index 0000000..bea974c --- /dev/null +++ b/Tests/ICCeryUITests/Milestone5UITests.swift @@ -0,0 +1,112 @@ +import Foundation +import XCTest + +/// Milestone 5 UI tests — issues #23–#27. +@MainActor +final class Milestone5UITests: XCTestCase { + + private var app: XCUIApplication! + private var testRoot: URL! + private var binDir: URL! + private var workDir: URL! + private var appDataDir: URL! + + override func setUp() async throws { + continueAfterFailure = false + testRoot = FileManager.default.temporaryDirectory + .appendingPathComponent("iccery-ui-m5-\(UUID().uuidString)") + binDir = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() + .appendingPathComponent("Fixtures/bin") + workDir = testRoot.appendingPathComponent("work") + appDataDir = testRoot.appendingPathComponent("AppData") + + try FileManager.default.createDirectory( + at: workDir, withIntermediateDirectories: true) + try FileManager.default.createDirectory( + at: appDataDir, withIntermediateDirectories: true) + + // Pre-stage a measured .ti3 so the wizard is already on Stage 4. + FileManager.default.createFile( + atPath: workDir.appendingPathComponent("mytarget.ti3").path, + contents: Data("MOCK_TI3".utf8), + attributes: nil) + + let state: [String: Any] = [ + "currentStage": 4, + "basename": "mytarget", + "cwd": workDir.path, + "printerName": "MockPrinter", + "sessionMode": "profile" + ] + let stateData = try JSONSerialization.data(withJSONObject: state, options: []) + try stateData.write(to: appDataDir.appendingPathComponent("wizard_state.json")) + + app = XCUIApplication() + app.launchEnvironment = [ + "ICCERY_UI_TESTING": "1", + "ICCERY_TEST_ROOT": testRoot.path, + "ICCERY_ARGYLL_BINARY_DIR": binDir.path, + "ICCERY_TEST_WORKDIR": workDir.path, + ] + } + + override func tearDown() async throws { + app?.terminate() + app = nil + if let testRoot { + try? FileManager.default.removeItem(at: testRoot) + } + testRoot = nil + } + + private func launchApp() { + app.launch() + if !app.wait(for: .runningForeground, timeout: 10) { + app.activate() + } + } + + private func element(_ id: String) -> XCUIElement { + app.descendants(matching: .any)[id] + } + + private func waitFor(_ id: String, timeout: TimeInterval = 20) -> XCUIElement { + let deadline = Date().addingTimeInterval(timeout) + while Date() < deadline { + let el = element(id) + if el.exists { return el } + RunLoop.current.run(until: Date().addingTimeInterval(0.1)) + } + let el = element(id) + XCTAssertTrue(el.exists, "Expected element \(id)") + return el + } + + /// Mock colprof produces a profile, unlocks Stage 5, and the + /// mock profcheck reports Good. + func testBuildProfileAndVerify() throws { + launchApp() + + let create = waitFor("btnCreateProfile") + XCTAssertTrue(create.isEnabled) + create.click() + + _ = waitFor("btnVerifyProfile", timeout: 30) + + // The mock iccgamut should have written a .gam next to the profile. + let gam = workDir.appendingPathComponent("mytarget.gam") + let icc = workDir.appendingPathComponent("mytarget.icc") + XCTAssertTrue(FileManager.default.fileExists(atPath: icc.path)) + XCTAssertTrue(FileManager.default.fileExists(atPath: gam.path)) + + app.buttons["btnVerifyProfile"].click() + _ = waitFor("profcheckStatus", timeout: 30) + + let statusValue = app.staticTexts["profcheckStatus"].firstMatch.value as? String ?? "" + XCTAssertTrue( + statusValue.contains("Good") || statusValue.contains("Excellent"), + "Expected verification status, got '\(statusValue)'" + ) + } +} -- 2.39.5 From ef56cdd7d476632a8abe7cc7d82cf4622aafcf26 Mon Sep 17 00:00:00 2001 From: Gronod Date: Wed, 9 Sep 2026 13:25:39 +0100 Subject: [PATCH 2/2] Fix M5 regressions and Stage 3 chartread prompt delivery (#50, #52). Hardens ProcessManager finalisation, ensures stdout/stderr pipe write-ends close after spawn, sets termination handlers before run(), and fixes the runCaptured continuation hand-off for fast exits. Resolves the Stage 3 prompt stream being dropped and the handheld fixture UI not advancing, unskips and repairs the handheld UI test, and fixes averaging panel accessibility. Includes the selected #52 profile, verification, and artefact-gating fixes. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../ICCeryCore/Argyll/ArgyllRunner.swift | 124 +++++++-- .../Measurement/ChartreadClassifier.swift | 85 ++++-- .../Process/ProcessLineDecoder.swift | 12 + .../ICCeryCore/Process/ProcessManager.swift | 257 +++++++++++++++--- .../ICCeryCore/Profile/DriftAlert.swift | 44 +-- .../ICCeryCore/Profile/ProfileInstaller.swift | 112 ++++++-- .../Profile/VerificationHistoryStore.swift | 6 +- .../ICCery/MeasurementWorkflowViewModel.swift | 4 + Sources/ICCery/ProfileWorkflowViewModel.swift | 67 +++-- Sources/ICCery/Stage3View.swift | 17 +- Sources/ICCery/Stage4View.swift | 1 + Sources/ICCery/Stage5View.swift | 7 +- Tests/ICCeryCoreTests/ApplycalArgsTests.swift | 8 +- Tests/ICCeryCoreTests/DriftAlertTests.swift | 44 ++- .../ProfileInstallerTests.swift | 184 ++++++++++--- .../VerificationHistoryStoreTests.swift | 80 ++++++ Tests/ICCeryUITests/Fixtures/bin/chartread | 2 +- Tests/ICCeryUITests/Milestone4UITests.swift | 11 +- 18 files changed, 839 insertions(+), 226 deletions(-) diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/ArgyllRunner.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/ArgyllRunner.swift index 9dcabe5..7e4d499 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/ArgyllRunner.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Argyll/ArgyllRunner.swift @@ -87,6 +87,7 @@ public struct ArgyllRunner: Sendable { let binaryURL = binaryResolver.resolve("targen") let processId = ProcessID.targen(cleanBasename) + await ensureNotRunning(id: processId) let events = processManager.events() try await processManager.runStreaming( id: processId, @@ -121,6 +122,7 @@ public struct ArgyllRunner: Sendable { let binaryURL = binaryResolver.resolve("printtarg") let processId = ProcessID.printtarg(cleanBasename) + await ensureNotRunning(id: processId) let events = processManager.events() try await processManager.runStreaming( id: processId, @@ -169,6 +171,19 @@ public struct ArgyllRunner: Sendable { // MARK: - Shared collection + /// Cancels any previous child with the same id and waits for it to + /// finalize, so `runStreaming` / `runCaptured` never sees a + /// `duplicateID` from a leftover process (#50, #52). + private func ensureNotRunning(id: String) async { + guard await processManager.isRunning(id) else { return } + await processManager.kill(id: id) + var attempts = 0 + while await processManager.isRunning(id), attempts < 30 { + try? await Task.sleep(for: .milliseconds(100)) + attempts += 1 + } + } + private struct CollectedRun { var exitCode: Int32? var stdout: String @@ -179,10 +194,15 @@ public struct ArgyllRunner: Sendable { /// Drains the event stream until this child's `exit` event. /// stdout is accumulated both per-line (logs) and verbatim (for /// the manifest parse — the pretty JSON needs its newlines). + /// + /// When `flushPartialLines` is `true`, a background `Task` flushes + /// unterminated output every 500 ms so tools like `colprof` that + /// print dots without newlines still produce log batches. private func collect( id processId: String, events: AsyncStream, - onLogBatch: (@Sendable ([String]) -> Void)? + onLogBatch: (@Sendable ([String]) -> Void)?, + flushPartialLines: Bool = false ) async -> CollectedRun { var lines: [String] = [] var stdout = "" @@ -198,6 +218,17 @@ public struct ArgyllRunner: Sendable { onLogBatch?(out) } + var dotFlushTask: Task? + if flushPartialLines { + dotFlushTask = Task { [processManager] in + while !Task.isCancelled { + try? await Task.sleep(for: .milliseconds(500)) + if Task.isCancelled { break } + await processManager.flushPartialLine(id: processId) + } + } + } + for await event in events { guard event.id == processId else { continue } switch event { @@ -229,6 +260,12 @@ public struct ArgyllRunner: Sendable { break } } + + dotFlushTask?.cancel() + if let dotFlushTask { + _ = await dotFlushTask.value + } + return CollectedRun(exitCode: exitCode, stdout: stdout, stderr: stderr, lines: lines) } @@ -242,6 +279,7 @@ public struct ArgyllRunner: Sendable { let binaryURL = binaryResolver.resolve("instlist") let processId = ProcessID.instlist + await ensureNotRunning(id: processId) let events = processManager.events() try await processManager.runStreaming( id: processId, @@ -301,6 +339,7 @@ public struct ArgyllRunner: Sendable { let binaryURL = binaryResolver.resolve("average") let processId = ProcessID.average(config.basename) + await ensureNotRunning(id: processId) let events = processManager.events() try await processManager.runStreaming( id: processId, @@ -335,6 +374,7 @@ public struct ArgyllRunner: Sendable { let binaryURL = binaryResolver.resolve("colprof") let processId = ProcessID.colprof(cleanBasename) + await ensureNotRunning(id: processId) let events = processManager.events() try await processManager.runStreaming( id: processId, @@ -342,7 +382,12 @@ public struct ArgyllRunner: Sendable { arguments: args, workingDirectory: cwd ) - let run = await collect(id: processId, events: events, onLogBatch: onLogBatch) + let run = await collect( + id: processId, + events: events, + onLogBatch: onLogBatch, + flushPartialLines: true + ) guard run.exitCode == 0 else { throw ArgyllRunnerError.colprofFailed( @@ -368,10 +413,13 @@ public struct ArgyllRunner: Sendable { /// /// Runs `applycal` captured and performs an in-place replace via /// `{input}.applycal.tmp` then `replaceItemAt`. On failure the tmp - /// file is removed and the original is left untouched. + /// file is removed and the original is left untouched. The UI must + /// never request `unapply` (#52). public func runApplycal( config: ApplycalConfig ) async throws -> URL { + assert(!config.unapply, "runApplycal does not support unapply") + let inputURL = config.inputProfileURL let cwd = inputURL.deletingLastPathComponent() let binaryURL = binaryResolver.resolve("applycal") @@ -383,6 +431,8 @@ public struct ArgyllRunner: Sendable { // Remove any stale tmp from a previous crash. try? fm.removeItem(at: tmpURL) + await ensureNotRunning(id: processId) + let outputConfig = ApplycalConfig( calibrationPath: config.calibrationPath, inputProfileURL: inputURL, @@ -398,8 +448,11 @@ public struct ArgyllRunner: Sendable { workingDirectory: cwd ) - guard result.exitCode == 0 else { + guard result.exitCode == 0, !Task.isCancelled else { try? fm.removeItem(at: tmpURL) + if Task.isCancelled { + throw CancellationError() + } throw ArgyllRunnerError.applycalFailed( result.stderr.isEmpty ? "applycal exited with code \(result.exitCode)" @@ -413,6 +466,15 @@ public struct ArgyllRunner: Sendable { ) } + let attrs = try? fm.attributesOfItem(atPath: tmpURL.path) + let size = attrs?[.size] as? UInt64 ?? 0 + guard size >= 128 else { + try? fm.removeItem(at: tmpURL) + throw ArgyllRunnerError.applycalFailed( + "calibrated profile is too small (\(size) bytes)" + ) + } + do { if fm.fileExists(atPath: inputURL.path) { _ = try fm.replaceItemAt(inputURL, withItemAt: tmpURL) @@ -441,6 +503,7 @@ public struct ArgyllRunner: Sendable { let binaryURL = binaryResolver.resolve("iccgamut") let processId = ProcessID.iccgamut(stem: stem) + await ensureNotRunning(id: processId) let events = processManager.events() try await processManager.runStreaming( id: processId, @@ -480,6 +543,7 @@ public struct ArgyllRunner: Sendable { let binaryURL = binaryResolver.resolve("profcheck") let processId = ProcessID.profcheck(ti3Path: ti3Path) + await ensureNotRunning(id: processId) let events = processManager.events() try await processManager.runStreaming( id: processId, @@ -547,11 +611,21 @@ public struct ArgyllRunner: Sendable { let binaryURL = binaryResolver.resolve("chartread") let processId = ProcessID.chartread(cleanBasename) let processManager = self.processManager + let isXY = config.isXY return AsyncStream { continuation in let task = Task { + await ensureNotRunning(id: processId) let events = processManager.events() + // Register the XY parking hook before spawning. + await processManager.setPreKillHook(id: processId) { [processManager] in + if isXY { + try? await processManager.sendStdin(id: processId, bytes: ChartreadInput.quit.bytes) + try? await Task.sleep(for: .milliseconds(500)) + } + } + do { try await processManager.runStreaming( id: processId, @@ -582,20 +656,22 @@ public struct ArgyllRunner: Sendable { switch event { case .stdout(_, let line): - let classified = ChartreadClassifier.classify(line: line, previousState: state) + let previous = state + let classified = ChartreadClassifier.classify(line: line, previousState: previous) state = classified.state + if classified.isRemoveSheetNotice { continuation.yield(.removeSheetNotice) } - if classified.sheetNumber != nil || classified.alignmentPatch != nil { - continuation.yield(.prompt(classified)) - } else if state != previousOrContinuationState(state, classified) { - // Only emit prompt when the state meaningfully changes. - continuation.yield(.prompt(classified)) - } else if state == .tablePlaceSheet || state == .tableAlign { - // Continuation lines in table states are still prompts. - continuation.yield(.prompt(classified)) - } else if classified.requestedWarningKey != nil { + + let shouldPrompt = + classified.sheetNumber != nil + || classified.alignmentPatch != nil + || classified.requestedWarningKey != nil + || classified.state != previous + || classified.isTableContinuation + + if shouldPrompt { continuation.yield(.prompt(classified)) } @@ -632,6 +708,11 @@ public struct ArgyllRunner: Sendable { } } + if Task.isCancelled { + continuation.finish() + return + } + let canonical = cwd.appendingPathComponent("\(cleanBasename).ti3") if let code = exitCode, code == 0 { if FileManager.default.fileExists(atPath: canonical.path) { @@ -647,15 +728,13 @@ public struct ArgyllRunner: Sendable { continuation.onTermination = { _ in task.cancel() + Task { + await processManager.kill(id: processId) + } } } } - private func previousOrContinuationState(_ state: ChartreadState, _ classified: ChartreadClassifyResult) -> ChartreadState { - if classified.isTableContinuation { return .promptContinue } - return state - } - /// Send an exact input sequence to the running `chartread` child. public func sendChartreadInput(basename: String, input: ChartreadInput) async throws { let cleanBasename = try PathSecurity.sanitizeBasename(basename) @@ -665,17 +744,14 @@ public struct ArgyllRunner: Sendable { /// Terminate a running `chartread` child. /// - /// For XY tables, sends `q\n` first and waits ~500 ms so the head parks. + /// The actual XY parking is handled by the pre-kill hook registered in + /// `runChartread`. public func cancelChartread(basename: String, isXY: Bool = false) { let cleanBasename = try? PathSecurity.sanitizeBasename(basename) guard let cleanBasename else { return } let processId = ProcessID.chartread(cleanBasename) Task { - if isXY { - try? await processManager.sendStdin(id: processId, bytes: ChartreadInput.quit.bytes) - try? await Task.sleep(for: .milliseconds(500)) - } await processManager.kill(id: processId) } } diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Measurement/ChartreadClassifier.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Measurement/ChartreadClassifier.swift index af4e90d..097c8d7 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Measurement/ChartreadClassifier.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Measurement/ChartreadClassifier.swift @@ -153,7 +153,7 @@ public enum ChartreadClassifier { let phrases = [ "'d' if/when done", "d to finish/save", "all strips/patches read", "all strips read", "all patches read", "done reading", - "'d' to save", "press d to", "hit 'd'" + "'d' to save", "press d to", "hit 'd'", "d to finish", "d to save" ] if phrases.contains(where: { text.contains($0) }) { return ChartreadClassifyResult(state: .allStripsRead) @@ -163,20 +163,28 @@ public enum ChartreadClassifier { // 7. Warnings / prompts needing a key. private static func warning(text: String, previous: ChartreadState) -> ChartreadClassifyResult? { + let lower = text let warningSignals = [ - "(warning)", "use it anyway", "seem to have read strip pass", - "unexpected response", "seem to have read", "misread", - "try again", "do you want to" + "(warning)", "use it anyway", "seem to have read strip", + "unexpected response", "try again", "do you want to", + "abort ? - are you sure", "are you sure" ] - guard warningSignals.contains(where: { text.contains($0) }) else { return nil } + + let isWarningPrompt = + warningSignals.contains(where: { lower.contains($0) }) + || lower.contains("(y/n)") + || lower.contains("'y' or 'n'") + || lower.contains("?") + + guard isWarningPrompt else { return nil } var key: String? - if text.contains("(y/n)") || text.contains("'y' or 'n'") { + if lower.contains("(y/n)") || lower.contains("'y' or 'n'") { // Default to asking the user; no automatic key. key = nil - } else if text.contains("'y'") || text.contains("press y") || text.contains("hit 'y'") { + } else if lower.contains("'y'") || lower.contains("press y") || lower.contains("hit 'y'") { key = "y" - } else if text.contains("'n'") || text.contains("press n") || text.contains("hit 'n'") { + } else if lower.contains("'n'") || lower.contains("press n") || lower.contains("hit 'n'") { key = "n" } @@ -195,12 +203,14 @@ public enum ChartreadClassifier { !hasLocate else { return nil } - if lowercased.contains("hit any key to continue") - || lowercased.contains("hit space to continue") - || lowercased.contains("calibration") - || lowercased.contains("calibrate") + if lowercased.contains("calibrat") + || lowercased.contains("white reference") || lowercased.contains("white tile") - || lowercased.contains("standard tile") { + || lowercased.contains("standard tile") + || lowercased.contains("reference") + || lowercased.contains("tile") + || lowercased.contains("hit any key to continue") + || lowercased.contains("hit space to continue") { return ChartreadClassifyResult(state: .calibrating) } return nil @@ -209,11 +219,29 @@ public enum ChartreadClassifier { // 9. Awaiting strip. private static func awaitingStrip(text: String, previous: ChartreadState) -> ChartreadClassifyResult? { let lowercased = text.lowercased() + + // These are explicit, multi-word prompts; we deliberately do NOT + // match bare "read strip" so that error lines like + // "failed to read strip" or "error reading strip" fall through to + // the error matcher. let phrases = [ - "hit ... read ... strip", "ready to read", "read ... strip ... key", - "hit any key to read", "ready to read strip", "hit a key to read", - "press any key to read", "read strip" + "ready to read", + "hit any key to read", + "hit a key to read", + "hit space to read", + "hit [space] to read", + "press any key to read", + "press space to read", + "trigger instrument", + "start reading", + "read next strip" ] + + // Also permit "hit X to read strip Y" or "ready to read strip Z". + if lowercased.range(of: #"(hit|press).+to\s+read\s+strip"#, options: .regularExpression) != nil { + return ChartreadClassifyResult(state: .awaitingStrip) + } + guard phrases.contains(where: { lowercased.contains($0) }) else { return nil } return ChartreadClassifyResult(state: .awaitingStrip) } @@ -228,16 +256,27 @@ public enum ChartreadClassifier { // 11. Error. private static func error(text: String, previous: ChartreadState) -> ChartreadClassifyResult? { - let phrases = ["error", "too fast", "too slow", "misread", "failed to read", "failed"] - // Avoid false positives inside harmless words by matching full words where possible. - let lower = text - guard phrases.contains(where: { phrase in - lower.contains(phrase) && !lower.contains("no error") - }) else { return nil } + let lower = text.lowercased() - if lower.contains("misread") || lower.contains("failed to read") || lower.contains("error") { + // Avoid false positives from confirmation prompts and "no error" status. + guard !lower.contains("no error") else { return nil } + guard !lower.contains("(y/n)") + && !lower.contains("'y' or 'n'") + && !lower.contains("?") + else { return nil } + + let phraseMatches = ["failed to read", "error reading", "too fast", "too slow", "misread"] + for phrase in phraseMatches { + if lower.contains(phrase) { + return ChartreadClassifyResult(state: .error) + } + } + + // Whole-word "error" only — bare "failed" alone is not enough. + if lower.range(of: #"\berror\b"#, options: .regularExpression) != nil { return ChartreadClassifyResult(state: .error) } + return nil } } diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Process/ProcessLineDecoder.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Process/ProcessLineDecoder.swift index ab79ff6..25f9c79 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Process/ProcessLineDecoder.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Process/ProcessLineDecoder.swift @@ -35,6 +35,18 @@ public struct ProcessLineDecoder: Sendable { return rest.isEmpty ? nil : Self.decode(rest) } + /// Emits the current unterminated tail as a single line and clears it. + /// Used by `ProcessManager.flushPartialLine` for tools that emit + /// progress dots without newlines. + public mutating func flushPartial() -> String? { + guard !pending.isEmpty else { return nil } + var rest = pending + pending.removeAll(keepingCapacity: false) + if rest.last == 0x0D { rest = rest.dropLast() } + let text = Self.decode(rest) + return text.isEmpty ? nil : text + } + private static func decode(_ bytes: Data.SubSequence) -> String { String(decoding: bytes, as: UTF8.self) } diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Process/ProcessManager.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Process/ProcessManager.swift index 4d692fc..88c7f2d 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Process/ProcessManager.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Process/ProcessManager.swift @@ -20,8 +20,11 @@ public struct CapturedResult: Sendable, Equatable { /// with the prefix stripped; all other stdout is `stdout` events. /// - `exit` is emitted exactly once per child, and only after both /// output pipes reach EOF — so no buffered output is lost on fast -/// exits or kills. -/// - `kill` drops the stdin handle so writers fail fast. +/// exits or kills. If EOFs never arrive, a watchdog finalizes. +/// - `kill` runs a pre-kill hook (e.g. XY `q\n` + 500 ms park) before +/// terminating. Hooks are removed once the child finalizes. +/// - `killAll` on `NSApplication.willTerminate` and last-window close +/// runs all hooks and terminates every child (#147, #149). public actor ProcessManager { public static let rowColorsPrefix = "ROW_COLORS_JSON: " @@ -92,12 +95,18 @@ public actor ProcessManager { /// pipes have also reached EOF. var pendingExitCode: Int32? var finalized = false + /// Watchdog that forces finalization if EOFs never arrive. + var finalizeTask: Task? } private var children: [String: RunningChild] = [:] /// Processes owned by `runCaptured` (dup detection + kill support). private var captured: [String: Process] = [:] + /// Hooks run by `kill` before terminating the child. + /// Used by `chartread` to park an XY head with `q\n`. + private var preKillHooks: [String: @Sendable () async -> Void] = [:] + /// Ids of currently-running children. public var runningIDs: [String] { Array(children.keys) + captured.keys } @@ -105,6 +114,14 @@ public actor ProcessManager { children[id] != nil || captured[id] != nil } + // MARK: - Pre-kill hooks + + /// Register a hook to run before `kill(id:)` terminates the child. + /// The hook is removed once the child finalizes. + public func setPreKillHook(id: String, hook: @escaping @Sendable () async -> Void) { + preKillHooks[id] = hook + } + // MARK: - Spawn (streaming) /// Spawns a streaming child. Returns after spawn; callers wait for @@ -142,14 +159,6 @@ public actor ProcessManager { stderrDecoder: ProcessLineDecoder() ) - do { - try process.run() - } catch { - children.removeValue(forKey: id) - emit(.error(id: id, message: error.localizedDescription)) - throw ProcessError.spawnFailed("\(binary.path): \(error.localizedDescription)") - } - let stdoutHandle = stdoutPipe.fileHandleForReading let stderrHandle = stderrPipe.fileHandleForReading stdoutHandle.readabilityHandler = { [weak self] handle in @@ -167,6 +176,15 @@ public actor ProcessManager { guard let self else { return } Task { await self.didTerminate(id: id, code: proc.terminationStatus) } } + + do { + try process.run() + } catch { + preKillHooks.removeValue(forKey: id) + children.removeValue(forKey: id) + emit(.error(id: id, message: error.localizedDescription)) + throw ProcessError.spawnFailed("\(binary.path): \(error.localizedDescription)") + } } // MARK: - Spawn (captured) @@ -198,41 +216,114 @@ public actor ProcessManager { "spawn(captured) \(id): \(binary.path) \(LogSanitizer.sanitizeArgs(arguments))" ) - // Register before run() so a concurrent duplicate spawn fails. + // Register and set up the termination hand-off before run() so + // a very fast exit is never missed (#50, #52). captured[id] = process + let capturedProcess = process + + // Box is local and synchronised with an NSLock; the @unchecked + // Sendable annotation is safe because all access is under the lock. + final class Box: @unchecked Sendable { + private let lock = NSLock() + private var status: Int32? + private var continuation: CheckedContinuation? + + /// Try to resume an already-stored continuation with the exit + /// status. Returns true if a continuation was resumed. + func resume(with status: Int32) -> Bool { + lock.lock() + if let cont = continuation { + continuation = nil + lock.unlock() + cont.resume(returning: status) + return true + } + self.status = status + lock.unlock() + return false + } + + /// Store a continuation, returning any status that arrived + /// before it. The caller must resume with the returned status. + func store(_ continuation: CheckedContinuation) -> Int32? { + lock.lock() + if let status = status { + self.status = nil + self.continuation = nil + lock.unlock() + return status + } + self.continuation = continuation + // A fast exit may have raced past the first nil-check. + if let status = status { + self.status = nil + self.continuation = nil + lock.unlock() + return status + } + lock.unlock() + return nil + } + } + let box = Box() + capturedProcess.terminationHandler = { proc in + _ = box.resume(with: proc.terminationStatus) + } + do { try process.run() } catch { + _ = box.resume(with: -1) captured.removeValue(forKey: id) + preKillHooks.removeValue(forKey: id) emit(.error(id: id, message: error.localizedDescription)) throw ProcessError.spawnFailed("\(binary.path): \(error.localizedDescription)") } - async let outData = Task.detached { - stdoutPipe.fileHandleForReading.readDataToEndOfFile() - }.value - async let errData = Task.detached { - stderrPipe.fileHandleForReading.readDataToEndOfFile() - }.value + // Close the parent write ends so readDataToEndOfFile() gets EOF + // as soon as the child exits; the child still has its own copies. + try? stdoutPipe.fileHandleForWriting.close() + try? stderrPipe.fileHandleForWriting.close() - let code = await withCheckedContinuation { continuation in - process.terminationHandler = { proc in - continuation.resume(returning: proc.terminationStatus) + return await withTaskCancellationHandler { + async let outData = Task.detached { + stdoutPipe.fileHandleForReading.readDataToEndOfFile() + }.value + async let errData = Task.detached { + stderrPipe.fileHandleForReading.readDataToEndOfFile() + }.value + + let code = await withCheckedContinuation { continuation in + if let status = box.store(continuation) { + continuation.resume(returning: status) + } + } + + let (out, err) = await (outData, errData) + + // Emit the real exit code once, regardless of whether kill() + // already removed the id from `captured`. + _ = captured.removeValue(forKey: id) + preKillHooks.removeValue(forKey: id) + emit(.exit(id: id, code: code)) + + return CapturedResult( + stdout: String(decoding: out, as: UTF8.self), + stderr: String(decoding: err, as: UTF8.self), + exitCode: code + ) + } onCancel: { [weak self] in + // If the awaiting Task is cancelled, terminate the child so + // callers like runApplycal never replace a good profile with + // a truncated tmp. + if capturedProcess.isRunning { + capturedProcess.terminate() + } + Task { [weak self] in + await self?.kill(id: id) } } - - let (out, err) = await (outData, errData) - // If kill() already reaped this child, its exit event went out. - if captured.removeValue(forKey: id) != nil { - emit(.exit(id: id, code: code)) - } - - return CapturedResult( - stdout: String(decoding: out, as: UTF8.self), - stderr: String(decoding: err, as: UTF8.self), - exitCode: code - ) } // MARK: - stdin @@ -255,39 +346,89 @@ public actor ProcessManager { try sendStdin(id: id, bytes: Data(text.utf8)) } + // MARK: - Partial-line flush + + /// Emits the current unterminated tail of a streaming child's stdout + /// and stderr as ordinary lines. Callers (e.g. `colprof`) use this + /// to flush progress dots without waiting for a newline. + public func flushPartialLine(id: String) { + guard var child = children[id], !child.finalized else { return } + + if let tail = child.stdoutDecoder.flushPartial() { + if tail.hasPrefix(Self.rowColorsPrefix) { + let payload = Data(tail.dropFirst(Self.rowColorsPrefix.count).utf8) + emit(.jsonRow(id: id, payload: payload)) + } else { + emit(.stdout(id: id, line: tail)) + } + } + if let tail = child.stderrDecoder.flushPartial() { + emit(.stderr(id: id, line: tail)) + } + + children[id] = child + } + // MARK: - Kill - /// Terminates a child. The `exit` event still fires exactly once. - /// stdin is dropped immediately so writers fail fast (docs/03 rule 7). - public func kill(id: String) { + /// Terminates a child. First runs any registered pre-kill hook, then + /// drops stdin and signals the process. For streaming children the + /// `exit` event is emitted once both stdout and stderr EOFs have been + /// seen (or the watchdog finalizes). For captured children the real + /// exit code is emitted by `runCaptured` itself. + public func kill(id: String) async { + if let hook = preKillHooks.removeValue(forKey: id) { + await hook() + } + if var child = children[id] { try? child.stdin?.close() child.stdin = nil children[id] = child + if child.process.isRunning { child.process.terminate() - } else { - Task { await self.didTerminate(id: id, code: child.process.terminationStatus) } + } else if child.pendingExitCode == nil { + // The process already exited but `didTerminate` has not + // run; synthesize it so `maybeFinalize` can fire. + didTerminate(id: id, code: child.process.terminationStatus) } return } + if let process = captured[id] { if process.isRunning { process.terminate() } - if captured.removeValue(forKey: id) != nil { - emit(.exit(id: id, code: process.terminationStatus)) - } + // Do not emit `.exit` here; `runCaptured` emits the real code + // after the process reaps. + return } } /// Terminates every running child; returns how many were signaled /// (`kill_all_processes`, docs/03). Mandatory on app exit (#147/#149). @discardableResult - public func killAll() -> Int { - let ids = Array(children.keys) + Array(captured.keys) - for id in ids { kill(id: id) } + public func killAll() async -> Int { + let ids = runningIDs + for id in ids { await kill(id: id) } return ids.count } + // MARK: - Force kill (SIGKILL fallback) + + /// Sends `SIGKILL` to a streaming child if it is still running. + /// Used by the finalization watchdog when a graceful `terminate()` + /// does not cause the process to exit. + public func forceKill(id: String) { + guard let child = children[id], + !child.finalized, + child.process.isRunning + else { return } + + let pid = child.process.processIdentifier + guard pid > 0 else { return } + _ = Darwin.kill(pid, SIGKILL) + } + // MARK: - Internals private func childEnvironment(extra: [String: String]) -> [String: String] { @@ -339,6 +480,16 @@ public actor ProcessManager { child.pendingExitCode = code try? child.stdin?.close() child.stdin = nil + + // Start a watchdog in case the `readabilityHandler` EOFs never + // arrive after the process exits (e.g. a hung pipe). + child.finalizeTask = Task { [weak self] in + try? await Task.sleep(for: .seconds(2)) + guard let self else { return } + await self.forceKill(id: id) + await self.forceFinalize(id: id) + } + children[id] = child maybeFinalize(id: id) } @@ -351,8 +502,12 @@ public actor ProcessManager { child.stdoutEOF, child.stderrEOF, !child.finalized else { return } + child.finalized = true + child.finalizeTask?.cancel() + child.finalizeTask = nil children.removeValue(forKey: id) + preKillHooks.removeValue(forKey: id) // Flush unterminated tail lines. if var decoder = Optional(child.stdoutDecoder), @@ -369,4 +524,20 @@ public actor ProcessManager { } emit(.exit(id: id, code: code)) } + + /// Forces finalization even when one or both EOFs are missing. + /// Used by the `didTerminate` watchdog. + private func forceFinalize(id: String) { + guard var child = children[id], !child.finalized else { return } + + if child.pendingExitCode == nil { + child.pendingExitCode = -9 + } + child.stdoutEOF = true + child.stderrEOF = true + child.finalizeTask?.cancel() + child.finalizeTask = nil + children[id] = child + maybeFinalize(id: id) + } } diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/DriftAlert.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/DriftAlert.swift index f673bd3..382f053 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/DriftAlert.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/DriftAlert.swift @@ -2,32 +2,40 @@ import Foundation /// Computes a consecutive-breach warning from verification history. /// -/// A drift alert triggers when there are at least two `poor` records on -/// distinct calendar days, or two `poor` records at least one hour apart. +/// A drift alert triggers when the most recent chronologically consecutive +/// poor records form a run of at least two, and the first and last of that +/// run are on distinct UTC days or at least one hour apart. public enum DriftAlert { /// Returns an alert message, or `nil` when no consecutive breach exists. public static func compute(from records: [VerificationRecord]) -> String? { - let poor = records - .filter { $0.status == .poor } - .sorted { $0.timestamp < $1.timestamp } + // Work in chronological order. + let chronological = records.sorted { $0.timestamp < $1.timestamp } - guard poor.count >= 2 else { return nil } - - for i in 0..= 3600 - - if !sameDay || oneHour { - return "Drift alert: poor results between \(a.id) and \(b.id)." - } + // Build the longest suffix of consecutive `.poor` records. + // Non-poor records break the run, so we stop at the first non-poor + // encountered from the end. + var run: [VerificationRecord] = [] + for record in chronological.reversed() { + if record.status == .poor { + run.insert(record, at: 0) + } else { + break } } + guard run.count >= 2 else { return nil } + + let first = run.first! + let last = run.last! + + let sameDay = Calendar.utc.isDate(first.timestamp, inSameDayAs: last.timestamp) + let oneHour = last.timestamp.timeIntervalSince(first.timestamp) >= 3600 + + if !sameDay || oneHour { + return "Drift alert: poor results between \(first.id) and \(last.id)." + } + return nil } } diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfileInstaller.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfileInstaller.swift index a81dda8..7cd22bc 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfileInstaller.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/ProfileInstaller.swift @@ -33,10 +33,32 @@ public enum ProfileInstallError: LocalizedError, Equatable, Sendable { /// Installs an ICC/ICM profile into the OS colour store. public enum ProfileInstaller { + /// Resolves the destination URL that `install` would write to for the + /// given source and options, without copying anything. Useful for + /// collision previews in the UI. + public static func resolveDestinationURL( + for config: InstallProfileConfig, + fileManager: FileManager = .default + ) throws -> URL { + let sourceURL = config.sourceURL + let ext = sourceURL.pathExtension.lowercased() + guard ext == "icc" || ext == "icm" else { + throw ProfileInstallError.sourceNotProfile + } + + try validateSourceURL(sourceURL) + + let destDir = destinationDirectory(for: config.options, fileManager: fileManager) + return destDir.appendingPathComponent(sourceURL.lastPathComponent) + } + /// Installs `sourceURL` into `~/Library/ColorSync/Profiles` or /// `/Library/ColorSync/Profiles`. Always copies, never moves. - public static func install(config: InstallProfileConfig) throws -> InstallProfileResult { - let fm = FileManager.default + public static func install( + config: InstallProfileConfig, + fileManager: FileManager = .default + ) throws -> InstallProfileResult { + let fm = fileManager // Source validation. let sourceURL = config.sourceURL @@ -55,38 +77,37 @@ public enum ProfileInstaller { throw ProfileInstallError.sourceTooSmall } - // Stem security. - let stem = sourceURL.deletingPathExtension().lastPathComponent - guard !stem.contains("..") && !stem.contains("/") && !stem.contains("\\") else { - throw ProfileInstallError.unsafeStem(stem) - } + try validateSourceURL(sourceURL) // Destination directory. - let destDir: URL - if config.options.preferSystem { - destDir = URL(fileURLWithPath: "/Library/ColorSync/Profiles") - } else { - let home = fm.homeDirectoryForCurrentUser - destDir = home.appendingPathComponent("Library/ColorSync/Profiles") - } - - // Ensure parent exists. - try? fm.createDirectory(at: destDir, withIntermediateDirectories: true) - - let destURL = destDir.appendingPathComponent("\(stem).icc") + let destURL = try resolveDestinationURL(for: config, fileManager: fm) + try? fm.createDirectory( + at: destURL.deletingLastPathComponent(), + withIntermediateDirectories: true + ) // Collision resolution. let destExists = fm.fileExists(atPath: destURL.path) if destExists { if config.options.forceOverwrite { - // Continue to overwrite path. + return try performInstall( + from: sourceURL, + to: destURL, + options: config.options, + fileManager: fm, + overwritten: true, + renamed: false + ) } else if config.options.collisionPolicy == .rename { let epoch = Int(Date().timeIntervalSince1970) - let renamedURL = destDir.appendingPathComponent("\(stem)-\(epoch).icc") + let stem = sourceURL.deletingPathExtension().lastPathComponent + let renamedURL = destURL.deletingLastPathComponent() + .appendingPathComponent("\(stem)-\(epoch).\(ext)") return try performInstall( from: sourceURL, to: renamedURL, options: config.options, + fileManager: fm, overwritten: false, renamed: true ) @@ -102,19 +123,53 @@ public enum ProfileInstaller { from: sourceURL, to: destURL, options: config.options, - overwritten: destExists, + fileManager: fm, + overwritten: false, renamed: false ) } + // MARK: - Private helpers + + private static func validateSourceURL(_ sourceURL: URL) throws { + let path = sourceURL.path + let stem = sourceURL.deletingPathExtension().lastPathComponent + + // Reject backslashes anywhere in the path. + guard !path.contains("\\") else { + throw ProfileInstallError.unsafeStem(stem) + } + + // Reject any path component that is literally "." or "..". + // This allows names like "foo..bar" while blocking real traversal. + for component in sourceURL.pathComponents { + if component == "." || component == ".." { + throw ProfileInstallError.unsafeStem(stem) + } + } + } + + private static func destinationDirectory( + for options: InstallProfileOptions, + fileManager: FileManager + ) -> URL { + if options.preferSystem { + return URL(fileURLWithPath: "/Library/ColorSync/Profiles") + } else { + return fileManager.homeDirectoryForCurrentUser + .appendingPathComponent("Library/ColorSync/Profiles") + } + } + private static func performInstall( from sourceURL: URL, to destURL: URL, options: InstallProfileOptions, + fileManager: FileManager, overwritten: Bool, renamed: Bool ) throws -> InstallProfileResult { - let fm = FileManager.default + let fm = fileManager let tmpURL = destURL.appendingPathExtension("iccery-install.tmp") // Remove stale tmp. @@ -123,6 +178,13 @@ public enum ProfileInstaller { do { try fm.copyItem(at: sourceURL, to: tmpURL) + let attrs = try? fm.attributesOfItem(atPath: tmpURL.path) + let tmpSize = attrs?[.size] as? UInt64 ?? 0 + guard tmpSize >= 128 else { + try? fm.removeItem(at: tmpURL) + throw ProfileInstallError.sourceTooSmall + } + if fm.fileExists(atPath: destURL.path) { _ = try fm.replaceItemAt(destURL, withItemAt: tmpURL) } else { @@ -135,6 +197,10 @@ public enum ProfileInstaller { if destURL.path.hasPrefix("/Library/") && !fm.fileExists(atPath: destURL.path) { throw ProfileInstallError.systemRequiresAdminRights } + + if let installError = error as? ProfileInstallError { + throw installError + } throw ProfileInstallError.copyFailed(error.localizedDescription) } diff --git a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationHistoryStore.swift b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationHistoryStore.swift index 6a06d97..2910aca 100644 --- a/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationHistoryStore.swift +++ b/Packages/ICCeryCore/Sources/ICCeryCore/Profile/VerificationHistoryStore.swift @@ -57,10 +57,12 @@ public actor VerificationHistoryStore { /// Appends a record, trims to capacity, and writes atomically. /// - /// Returns the trimmed list, or `nil` if a write error occurs so the - /// caller can surface the failure without replacing the in-memory list. + /// Loads the existing history first and propagates any load error so an + /// unparseable file is never overwritten. @discardableResult public func append(_ record: VerificationRecord) throws -> [VerificationRecord] { + try load() + var updated = records updated.append(record) if updated.count > capacity { diff --git a/Sources/ICCery/MeasurementWorkflowViewModel.swift b/Sources/ICCery/MeasurementWorkflowViewModel.swift index 66d3103..541608d 100644 --- a/Sources/ICCery/MeasurementWorkflowViewModel.swift +++ b/Sources/ICCery/MeasurementWorkflowViewModel.swift @@ -305,6 +305,10 @@ final class MeasurementWorkflowViewModel { environment.runner.cancelChartread(basename: basename, isXY: selectedInstrument.isXY) chartreadTask?.cancel() isChartreadRunning = false + chartreadState = .idle + currentPrompt = nil + requestedWarningKey = nil + showRemoveSheetNotice = false } func sendWarningKey(_ key: String) { diff --git a/Sources/ICCery/ProfileWorkflowViewModel.swift b/Sources/ICCery/ProfileWorkflowViewModel.swift index d1f0093..320c774 100644 --- a/Sources/ICCery/ProfileWorkflowViewModel.swift +++ b/Sources/ICCery/ProfileWorkflowViewModel.swift @@ -81,6 +81,15 @@ final class ProfileWorkflowViewModel { init(wizard: WizardViewModel, environment: AppEnvironment) { self.wizard = wizard self.environment = environment + restoreCreatedProfileURL() + } + + /// Restores `createdProfileURL` from the wizard artefacts or by probing + /// the working directory for an existing `.icc`/`.icm` (#52). + func restoreCreatedProfileURL() { + let cwd = wizard.effectiveWorkingDirectory ?? PathSecurity.resolveSafeCwd(nil) + createdProfileURL = wizard.artefacts.profilePath + ?? ArtefactProbe.resolveProfile(basename: wizard.basename, cwd: cwd) } // MARK: - Derived @@ -206,6 +215,7 @@ final class ProfileWorkflowViewModel { calibrationPath: self.calibrationFile, inputProfileURL: url ) + assert(!applyConfig.unapply, "applycal unapply is not supported in v2.0") finalProfileURL = try await runner.runApplycal(config: applyConfig) self.colprofLog.append("Calibration embedded: \(self.calibrationFile)") } @@ -256,7 +266,15 @@ final class ProfileWorkflowViewModel { // MARK: - Stage 5: verify profile var knownPrinters: [String] { - Array(Set(verificationHistory.map { $0.printerName })).sorted() + var names = Set() + for record in verificationHistory { + if record.printerName.isEmpty { + names.insert("Unknown") + } else { + names.insert(record.printerName) + } + } + return Array(names).sorted() } func loadHistory() { @@ -333,10 +351,11 @@ final class ProfileWorkflowViewModel { let timestamp = Date() let id = "vr-\(Int(timestamp.timeIntervalSince1970))-\(Self.nextSeq())" + let printerName = wizard.printerName?.isEmpty == false ? wizard.printerName! : "Unknown" return VerificationRecord( id: id, profileName: createdProfileURL?.lastPathComponent ?? wizard.basename, - printerName: wizard.printerName ?? "", + printerName: printerName, avgDE: avg, maxDE: max, rmsDE: rms, @@ -381,24 +400,32 @@ final class ProfileWorkflowViewModel { openColorPanel: settings.openColorPanelAfterInstall ) - let destURL = installDestination(for: sourceURL, options: options) - let collision = FileManager.default.fileExists(atPath: destURL.path) + do { + let config = InstallProfileConfig(sourceURL: sourceURL, options: options) + let destURL = try ProfileInstaller.resolveDestinationURL(for: config) + let collision = FileManager.default.fileExists(atPath: destURL.path) - if collision && settings.askBeforeOverwriteProfile { - pendingInstallOptions = options - installCollisionMessage = "A profile named \(destURL.lastPathComponent) already exists." - showingInstallCollision = true - return + if collision && settings.askBeforeOverwriteProfile { + pendingInstallOptions = options + installCollisionMessage = "A profile named \(destURL.lastPathComponent) already exists." + showingInstallCollision = true + return + } + + runInstall(sourceURL: sourceURL, options: options) + } catch { + wizard.showNotice( + "Install failed: \(error.localizedDescription)", + kind: .error + ) } - - runInstall(sourceURL: sourceURL, options: options) } func resolveInstallCollision(policy: ProfileCollisionPolicy) { showingInstallCollision = false guard let sourceURL = createdProfileURL, var options = pendingInstallOptions else { return } - options.collisionPolicy = policy + if policy == .cancel { installResult = InstallProfileResult( destPath: "", @@ -410,20 +437,12 @@ final class ProfileWorkflowViewModel { ) return } - runInstall(sourceURL: sourceURL, options: options) - } - private func installDestination(for sourceURL: URL, options: InstallProfileOptions) -> URL { - let stem = sourceURL.deletingPathExtension().lastPathComponent - let fm = FileManager.default - let destDir: URL - if options.preferSystem { - destDir = URL(fileURLWithPath: "/Library/ColorSync/Profiles") - } else { - destDir = fm.homeDirectoryForCurrentUser - .appendingPathComponent("Library/ColorSync/Profiles") + options.collisionPolicy = policy + if policy == .overwrite { + options.forceOverwrite = true } - return destDir.appendingPathComponent("\(stem).icc") + runInstall(sourceURL: sourceURL, options: options) } private func runInstall(sourceURL: URL, options: InstallProfileOptions) { diff --git a/Sources/ICCery/Stage3View.swift b/Sources/ICCery/Stage3View.swift index 2509fb3..290b2d3 100644 --- a/Sources/ICCery/Stage3View.swift +++ b/Sources/ICCery/Stage3View.swift @@ -210,7 +210,9 @@ struct Stage3View: View { .accessibilityIdentifier("btnCalibrate") case .awaitingStrip: Button("Trigger") { model.calibrate() } - .accessibilityIdentifier("btnCalibrate") + .accessibilityIdentifier("btnTrigger") + Button("Done & Save") { model.doneAndSave() } + .accessibilityIdentifier("btnDoneReadEarly") case .tablePlaceSheet, .tableAlign, .promptContinue, .warning: Button(continueTitle) { model.accept() } .accessibilityIdentifier("btnAccept") @@ -224,16 +226,6 @@ struct Stage3View: View { EmptyView() } - if model.chartreadState == .awaitingStrip || model.chartreadState == .allStripsRead { - Button("Done & Save") { model.doneAndSave() } - .accessibilityIdentifier("btnDoneRead") - } - - if model.chartreadState == .error { - Button("Retry") { model.retry() } - .accessibilityIdentifier("btnRetry") - } - Button("Cancel") { model.cancelRead() } .accessibilityIdentifier("btnCancel") } @@ -374,7 +366,7 @@ struct Stage3View: View { Button("Finish & Average") { model.finishAndAverage() } - .disabled(!model.isFinished || model.isFinishing) + .disabled(!model.canFinish || model.isFinishing) .accessibilityIdentifier("btnFinishAndAverage") } @@ -386,6 +378,7 @@ struct Stage3View: View { } .padding(16) .background(Theme.panel) + .accessibilityElement(children: .contain) .accessibilityIdentifier("chartreadAveragingPanel") } } diff --git a/Sources/ICCery/Stage4View.swift b/Sources/ICCery/Stage4View.swift index 3fe4a08..e879abb 100644 --- a/Sources/ICCery/Stage4View.swift +++ b/Sources/ICCery/Stage4View.swift @@ -18,6 +18,7 @@ struct Stage4View: View { } .frame(maxWidth: .infinity, maxHeight: .infinity) .background(Theme.background) + .onAppear { model.restoreCreatedProfileURL() } } // MARK: - Header diff --git a/Sources/ICCery/Stage5View.swift b/Sources/ICCery/Stage5View.swift index af7b760..a6c18bd 100644 --- a/Sources/ICCery/Stage5View.swift +++ b/Sources/ICCery/Stage5View.swift @@ -21,7 +21,10 @@ struct Stage5View: View { } .frame(maxWidth: .infinity, maxHeight: .infinity) .background(Theme.background) - .onAppear { model.loadHistory() } + .onAppear { + model.restoreCreatedProfileURL() + model.loadHistory() + } .alert("Install profile", isPresented: $model.showingInstallCollision) { Button("Overwrite", role: .destructive) { model.resolveInstallCollision(policy: .overwrite) @@ -70,7 +73,7 @@ struct Stage5View: View { .accessibilityIdentifier("driftAlert") } - if let warning = model.profcheckWarning, !warning.isEmpty, model.driftAlert == nil { + if let warning = model.profcheckWarning, !warning.isEmpty { Text("⚠ \(warning)") .font(.caption) .padding(.horizontal, 8) diff --git a/Tests/ICCeryCoreTests/ApplycalArgsTests.swift b/Tests/ICCeryCoreTests/ApplycalArgsTests.swift index 9075509..fb10001 100644 --- a/Tests/ICCeryCoreTests/ApplycalArgsTests.swift +++ b/Tests/ICCeryCoreTests/ApplycalArgsTests.swift @@ -15,16 +15,16 @@ struct ApplycalArgsTests { #expect(args == ["-v", "-a", "/tmp/cal.cal", "/tmp/profile.icc"]) } - @Test("Unapply is never sent from build") - func unapplyNotEmitted() throws { + @Test("Unapply is emitted when the caller explicitly sets it") + func unapplyEmittedWhenConfigSet() throws { let config = ApplycalConfig( calibrationPath: "/tmp/cal.cal", inputProfileURL: URL(fileURLWithPath: "/tmp/profile.icc"), unapply: true ) let args = try ApplycalArgs.build(config: config) - // Builder intentionally emits -u because config can set it, but - // the UI layer never passes unapply: true in v2.0. + // Builder emits -u only when the caller explicitly sets unapply. + // The UI layer never passes unapply: true in v2.0. #expect(args == ["-v", "-u", "/tmp/cal.cal", "/tmp/profile.icc"]) } } diff --git a/Tests/ICCeryCoreTests/DriftAlertTests.swift b/Tests/ICCeryCoreTests/DriftAlertTests.swift index 51ab7fa..4eb2835 100644 --- a/Tests/ICCeryCoreTests/DriftAlertTests.swift +++ b/Tests/ICCeryCoreTests/DriftAlertTests.swift @@ -38,7 +38,7 @@ struct DriftAlertTests { #expect(DriftAlert.compute(from: [day1, day2]) != nil) } - @Test("Non-poor results do not trigger") + @Test("Non-poor records do not trigger") func nonPoor() { let records = [ record(avg: 1.0, at: 0), @@ -47,6 +47,48 @@ struct DriftAlertTests { #expect(DriftAlert.compute(from: records) == nil) } + @Test("Non-poor records break the consecutive poor run") + func nonPoorBreaksRun() { + let records = [ + record(avg: 4.0, at: 0), // poor + record(avg: 4.5, at: 86400), // poor, far apart + record(avg: 1.0, at: 90000), // good — breaks the run + record(avg: 4.0, at: 92000) // poor, recent but close to previous poor + ] + #expect(DriftAlert.compute(from: records) == nil) + } + + @Test("Only the final consecutive poor run is considered") + func onlySuffixRun() { + let records = [ + record(avg: 4.0, at: 0), // poor + record(avg: 4.5, at: 18000), // poor, > 1h from first + record(avg: 1.0, at: 20000), // good — breaks the run + record(avg: 4.0, at: 25000), // poor + record(avg: 4.5, at: 26000) // poor, < 1h and same day + ] + #expect(DriftAlert.compute(from: records) == nil) + } + + @Test("Final consecutive poor run alerts when far apart") + func suffixRunAlerts() { + let records = [ + record(avg: 1.0, at: 0), // good + record(avg: 4.0, at: 1000), // poor + record(avg: 4.5, at: 4600) // poor, 1h after previous + ] + #expect(DriftAlert.compute(from: records) != nil) + } + + @Test("A single final poor record after good records does not alert") + func singleFinalPoor() { + let records = [ + record(avg: 1.0, at: 0), + record(avg: 4.0, at: 86400) + ] + #expect(DriftAlert.compute(from: records) == nil) + } + private func record(avg: Double, at offset: TimeInterval) -> VerificationRecord { VerificationRecord( id: "vr-\(Int(offset))", diff --git a/Tests/ICCeryCoreTests/ProfileInstallerTests.swift b/Tests/ICCeryCoreTests/ProfileInstallerTests.swift index 640a6c5..7851582 100644 --- a/Tests/ICCeryCoreTests/ProfileInstallerTests.swift +++ b/Tests/ICCeryCoreTests/ProfileInstallerTests.swift @@ -2,42 +2,165 @@ import Foundation import Testing @testable import ICCeryCore +/// A `FileManager` subclass that reports a temporary directory as the +/// user home, so `ProfileInstaller` can be tested without writing to the +/// real `~/Library/ColorSync/Profiles`. +private final class TestFileManager: FileManager { + let tempHome: URL + + init(home: URL) { + self.tempHome = home + super.init() + } + + override var homeDirectoryForCurrentUser: URL { + tempHome + } +} + @Suite("ProfileInstaller") struct ProfileInstallerTests { - @Test("Copies .icc to user ColorSync folder") - func userInstall() throws { + private func makeTempDir() throws -> URL { let fm = FileManager.default let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) try fm.createDirectory(at: tmp, withIntermediateDirectories: true) + return tmp + } - let source = tmp.appendingPathComponent("test.icc") - let iccData = Data(repeating: 0, count: 256) - try iccData.write(to: source) + private func makeSource( + at dir: URL, + name: String, + bytes: [UInt8] = Array(repeating: 0, count: 256) + ) throws -> URL { + let url = dir.appendingPathComponent(name) + let data = Data(bytes) + try data.write(to: url) + return url + } - let colorsync = tmp.appendingPathComponent("Library/ColorSync/Profiles") + @Test("Installs .icc to user ColorSync folder") + func userInstall() throws { + let fm = FileManager.default + let tmp = try makeTempDir() + let testFM = TestFileManager(home: tmp) + let source = try makeSource(at: tmp, name: "test.icc") - // Inject a user profile install by replacing the home directory - // is not practical; instead exercise Core validation on a - // temp-only path via the file URL safety checks and the public - // install against a writable system-like path is tested below. + let result = try ProfileInstaller.install( + config: InstallProfileConfig(sourceURL: source), + fileManager: testFM + ) + + #expect(result.registered) + #expect(!result.overwritten) + #expect(!result.renamed) + #expect(result.destPath.hasSuffix("test.icc")) + #expect(fm.fileExists(atPath: result.destPath)) + } + + @Test("Overwrite succeeds and replaces the existing file") + func overwriteSucceeds() throws { + let fm = FileManager.default + let tmp = try makeTempDir() + let testFM = TestFileManager(home: tmp) + let source = try makeSource(at: tmp, name: "m5_profile.icc", bytes: (0..<256).map { UInt8($0) }) + + // First install. + let first = try ProfileInstaller.install( + config: InstallProfileConfig(sourceURL: source), + fileManager: testFM + ) + #expect(!first.overwritten) + + // Change the source contents. + let newBytes: [UInt8] = (0..<256).map { UInt8(($0 + 100) % 256) } + try Data(newBytes).write(to: source) + + let options = InstallProfileOptions( + forceOverwrite: true, + preferSystem: false, + collisionPolicy: .overwrite, + openColorPanel: false + ) + let second = try ProfileInstaller.install( + config: InstallProfileConfig(sourceURL: source, options: options), + fileManager: testFM + ) + + #expect(second.overwritten) + #expect(!second.renamed) + #expect(fm.fileExists(atPath: second.destPath)) + let installed = try Data(contentsOf: URL(fileURLWithPath: second.destPath)) + #expect(Array(installed) == newBytes) + } + + @Test("Preserves .icm source extension") + func preservesIcmExtension() throws { + let tmp = try makeTempDir() + let testFM = TestFileManager(home: tmp) + let source = try makeSource(at: tmp, name: "m5_profile.icm") + + let result = try ProfileInstaller.install( + config: InstallProfileConfig(sourceURL: source), + fileManager: testFM + ) + + #expect(URL(fileURLWithPath: result.destPath).pathExtension == "icm") + #expect(result.destPath.hasSuffix("m5_profile.icm")) + } + + @Test("Rejects parent traversal in source path") + func rejectsParentTraversal() throws { + let fm = FileManager.default + let tmp = try makeTempDir() + + // Create a real file in the parent of `tmp` with a path that contains + // a literal ".." component. + let parent = tmp.deletingLastPathComponent() + let naughtyName = "naughty-\(UUID().uuidString).icc" + let realFile = parent.appendingPathComponent(naughtyName) + _ = try makeSource(at: parent, name: naughtyName) + defer { try? fm.removeItem(at: realFile) } + + let sourceURL = tmp + .appendingPathComponent("..") + .appendingPathComponent(naughtyName) + #expect(fm.fileExists(atPath: sourceURL.path)) - // For this unit test, validate the stem security and source rules. - let unsafe = tmp.appendingPathComponent("bad..stem.icc") - try Data(repeating: 0, count: 256).write(to: unsafe) do { - _ = try ProfileInstaller.install(config: InstallProfileConfig(sourceURL: unsafe)) + _ = try ProfileInstaller.install(config: InstallProfileConfig(sourceURL: sourceURL)) Issue.record("Expected unsafeStem error") } catch let error as ProfileInstallError { if case .unsafeStem = error { } else { Issue.record("Expected unsafeStem, got \(error)") } } catch { Issue.record("Unexpected error type: \(error)") } + } + + @Test("Allows stems with consecutive dots like foo..bar") + func allowsDoubleDotStem() throws { + let fm = FileManager.default + let tmp = try makeTempDir() + let testFM = TestFileManager(home: tmp) + let source = try makeSource(at: tmp, name: "foo..bar.icc") + + let result = try ProfileInstaller.install( + config: InstallProfileConfig(sourceURL: source), + fileManager: testFM + ) + + #expect(result.destPath.hasSuffix("foo..bar.icc")) + #expect(fm.fileExists(atPath: result.destPath)) + } + + @Test("Rejects source files that are too small") + func rejectsSmallSource() throws { + let tmp = try makeTempDir() + let source = tmp.appendingPathComponent("tiny.icc") + try Data(repeating: 0, count: 64).write(to: source) - let small = tmp.appendingPathComponent("tiny.icc") - try Data(repeating: 0, count: 64).write(to: small) do { - _ = try ProfileInstaller.install(config: InstallProfileConfig(sourceURL: small)) + _ = try ProfileInstaller.install(config: InstallProfileConfig(sourceURL: source)) Issue.record("Expected sourceTooSmall error") } catch let error as ProfileInstallError { if case .sourceTooSmall = error { } else { Issue.record("Expected sourceTooSmall, got \(error)") } @@ -45,31 +168,4 @@ struct ProfileInstallerTests { Issue.record("Unexpected error type: \(error)") } } - - @Test("Installs into a temp user folder and preserves source") - func tempInstallPreservesSource() throws { - let fm = FileManager.default - let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) - try fm.createDirectory(at: tmp, withIntermediateDirectories: true) - - let source = tmp.appendingPathComponent("m5_profile.icc") - try Data(repeating: 0, count: 256).write(to: source) - - let destDir = tmp.appendingPathComponent("ColorSync/Profiles") - try fm.createDirectory(at: destDir, withIntermediateDirectories: true) - - // There is no public API to override the home directory, so - // test the copy mechanism directly via file operations. - let dest = destDir.appendingPathComponent("m5_profile.icc") - let tmpDest = dest.appendingPathExtension("iccery-install.tmp") - try fm.copyItem(at: source, to: tmpDest) - if fm.fileExists(atPath: dest.path) { - _ = try fm.replaceItemAt(dest, withItemAt: tmpDest) - } else { - try fm.moveItem(at: tmpDest, to: dest) - } - - #expect(fm.fileExists(atPath: source.path)) - #expect(fm.fileExists(atPath: dest.path)) - } } diff --git a/Tests/ICCeryCoreTests/VerificationHistoryStoreTests.swift b/Tests/ICCeryCoreTests/VerificationHistoryStoreTests.swift index c478f5f..91a63fc 100644 --- a/Tests/ICCeryCoreTests/VerificationHistoryStoreTests.swift +++ b/Tests/ICCeryCoreTests/VerificationHistoryStoreTests.swift @@ -51,6 +51,86 @@ struct VerificationHistoryStoreTests { } } + @Test("Append loads existing records first") + func appendLoadsExisting() async throws { + let fm = FileManager.default + let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try fm.createDirectory(at: tmp, withIntermediateDirectories: true) + let url = tmp.appendingPathComponent("verification_history.json") + + // Pre-populate the store on disk. + let existing = VerificationRecord( + id: "vr-existing", + profileName: "p", + printerName: "", + avgDE: 1.0, + maxDE: 1.0, + rmsDE: 1.0, + patchCount: 1, + status: .good, + timestamp: Date(timeIntervalSince1970: 0) + ) + let store1 = VerificationHistoryStore(url: url) + _ = try await store1.append(existing) + + // A fresh store appending a new record must keep the existing one. + let store2 = VerificationHistoryStore(url: url) + let new = VerificationRecord( + id: "vr-new", + profileName: "p", + printerName: "", + avgDE: 2.0, + maxDE: 2.0, + rmsDE: 2.0, + patchCount: 2, + status: .good, + timestamp: Date(timeIntervalSince1970: 10) + ) + _ = try await store2.append(new) + + let all = await store2.all() + #expect(all.count == 2) + #expect(all.contains { $0.id == "vr-existing" }) + #expect(all.contains { $0.id == "vr-new" }) + } + + @Test("Append does not overwrite an unparseable file") + func appendPreservesUnparseableFile() async { + let fm = FileManager.default + let tmp = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try? fm.createDirectory(at: tmp, withIntermediateDirectories: true) + let url = tmp.appendingPathComponent("verification_history.json") + + let badJSON = "not json" + try? badJSON.write(to: url, atomically: true, encoding: .utf8) + + let store = VerificationHistoryStore(url: url) + let record = VerificationRecord( + id: "vr-new", + profileName: "p", + printerName: "", + avgDE: 1.0, + maxDE: 1.0, + rmsDE: 1.0, + patchCount: 1, + status: .good, + timestamp: Date(timeIntervalSince1970: 0) + ) + + do { + _ = try await store.append(record) + Issue.record("append() should propagate the load error") + } catch { + #expect(fm.fileExists(atPath: url.path)) + if let data = try? Data(contentsOf: url), + let contents = String(data: data, encoding: .utf8) { + #expect(contents == badJSON) + } else { + Issue.record("Could not read preserved file") + } + } + } + @Test("CSV export quoting") func csvQuoting() async throws { let fm = FileManager.default diff --git a/Tests/ICCeryUITests/Fixtures/bin/chartread b/Tests/ICCeryUITests/Fixtures/bin/chartread index c540cd6..8f38d75 100755 --- a/Tests/ICCeryUITests/Fixtures/bin/chartread +++ b/Tests/ICCeryUITests/Fixtures/bin/chartread @@ -39,7 +39,7 @@ def main(): def read_input(): line = read_line() - if not line: + if line == "": sys.exit(1) return line.strip() diff --git a/Tests/ICCeryUITests/Milestone4UITests.swift b/Tests/ICCeryUITests/Milestone4UITests.swift index 2a50bdc..92759aa 100644 --- a/Tests/ICCeryUITests/Milestone4UITests.swift +++ b/Tests/ICCeryUITests/Milestone4UITests.swift @@ -93,7 +93,6 @@ final class Milestone4UITests: XCTestCase { /// End-to-end handheld chartread with the mock fixture produces a /// canonical .ti3 and unlocks Stage 4. func testHandheldFixtureChartreadAndAverage() throws { - try XCTSkipIf(true, "Full interactive chartread UI requires fixture timing tuning; skipped for CI stability. Core chartread/arteffact tests cover the model.") reachStage3() app.buttons["btnDetectInstruments"].click() @@ -113,19 +112,21 @@ final class Milestone4UITests: XCTestCase { app.buttons["btnCalibrate"].click() // Trigger strip A. - _ = waitFor("btnCalibrate", timeout: 20) - app.buttons["btnCalibrate"].click() + _ = waitFor("btnTrigger", timeout: 20) + app.buttons["btnTrigger"].click() // Trigger strip B. - _ = waitFor("btnCalibrate", timeout: 20) - app.buttons["btnCalibrate"].click() + _ = waitFor("btnTrigger", timeout: 20) + app.buttons["btnTrigger"].click() // All strips read → Done & Save appears. _ = waitFor("btnDoneRead", timeout: 20) app.buttons["btnDoneRead"].firstMatch.click() // Averaging panel appears with one pass snapshot. + _ = waitFor("chartreadAveragingPanel", timeout: 20) _ = waitFor("passCounterBadge", timeout: 20) + XCTAssertTrue(app.buttons["btnFinishAndAverage"].waitForExistence(timeout: 5)) XCTAssertTrue(app.buttons["btnFinishAndAverage"].isEnabled) app.buttons["btnFinishAndAverage"].click() -- 2.39.5