fix(ios): a close-up the phone couldn't save says so, instead of "Hold still" (B-20, on #191)

ios/HouseScan/Contract/ScanContract.swift

@@ -443,6 +443,9 @@ enum CloseUpProblem: Equatable, Sendable {
     case numberTooSmall
     /// No number could be read at all: retake.
     case noNumber
+    /// The photo couldn't be written to the phone, or read back once written. Nothing about the
+    /// shot was wrong, so the words don't blame the homeowner's hands (B-20).
+    case photoNotSaved
 }
 
 /// One reading of the meter number from the close-up, for the homeowner to confirm.

ios/HouseScan/Runtime/Autopilot.swift

@@ -119,7 +119,7 @@ final class Autopilot {
         let state = engine.state
         guard state.phase == .meterCloseUp else { return .left }
         if case .choose(let candidates) = state.meterNumber, let first = candidates.first { return .choose(first) }
-        if case .aiming(_, let problem?) = state.closeUp, problem == .noNumber || problem == .numberTooSmall { return .retake(problem) }
+        if case .aiming(_, let problem?) = state.closeUp, [.noNumber, .numberTooSmall, .photoNotSaved].contains(problem) { return .retake(problem) }
         return nil
     }
 

ios/HouseScan/Runtime/KeyframeStore.swift

@@ -162,26 +162,51 @@ final class KeyframeStore {
         case writeFailed = "write failed"
     }
 
-    /// Writes a still such as the meter close-up from `frame`. Returns false when there was
-    /// nothing to write. The still stands for its purpose only once accepted (`acceptStill`).
-    func saveStill(_ frame: SourceFrame, name: String) async -> Bool {
+    /// What became of a `saveStill`. The two failures need different words on screen: a photo
+    /// that couldn't be written is a phone problem the homeowner can retry or skip, while a
+    /// discarded world frame has already sent the flow back to finding the meter (B-20: both
+    /// used to read as a blurry photo).
+    enum StillSave: Equatable {
+        case saved
+        /// No image bytes to write, or the write failed (a full phone, say). Nothing new is
+        /// listed; a still saved earlier under the same name stays as it was.
+        case notWritten
+        /// Written, but in a world frame discarded meanwhile, so it isn't kept.
+        case worldDiscarded
+    }
+
+    /// Writes a still such as the meter close-up from `frame`. The still stands for its purpose
+    /// only once accepted (`acceptStill`).
+    func saveStill(_ frame: SourceFrame, name: String) async -> StillSave {
         let directory = directory
         let id = Self.purpose(of: name)
         let startedIn = epoch
-        let written = await Task.detached(priority: .utility) { () -> StoredKeyframe? in
-            guard let data = Self.data(of: frame.jpeg), (try? data.write(to: directory.appending(path: name), options: .atomic)) != nil else { return nil }
-            return Self.stored(frame, id: id, jpeg: data, in: directory)
+        let written = await Task.detached(priority: .utility) { () -> Result<StoredKeyframe, KeyframeWriteFailure> in
+            guard let data = Self.data(of: frame.jpeg) else { return .failure(.noPhoto) }
+            do {
+                try data.write(to: directory.appending(path: name), options: .atomic)
+            } catch {
+                return .failure(.writeFailed)
+            }
+            return .success(Self.stored(frame, id: id, jpeg: data, in: directory))
         }.value
-        guard let written else { return false }
+        let stored: StoredKeyframe
+        switch written {
+        case .success(let entry):
+            stored = entry
+        case .failure(let failure):
+            RuntimeLog.capture.error("still \(name, privacy: .public) not stored: \(failure.rawValue, privacy: .public)")
+            return .notWritten
+        }
         // Taken in a world frame that was discarded while the file was being written. The file is
         // left alone: a close-up in the new frame may already have written the same name, and
         // nothing reads a still that `stillFrames` doesn't list.
         guard startedIn == epoch else {
             RuntimeLog.capture.info("still \(name, privacy: .public) not kept: its world frame was discarded")
-            return false
+            return .worldDiscarded
         }
-        stillCatalog.save(written, purpose: id, fileName: name)
-        return true
+        stillCatalog.save(stored, purpose: id, fileName: name)
+        return .saved
     }
 
     /// The homeowner accepted the still saved as `name` (the meter number read from it was
@@ -197,14 +222,6 @@ final class KeyframeStore {
         stillCatalog.withdraw(Self.purpose(of: name))
     }
 
-    func thumbnail(ofStill name: String) async -> CGImage? {
-        let url = directory.appending(path: name)
-        return await Task.detached(priority: .utility) { () -> CGImage? in
-            guard let data = try? Data(contentsOf: url) else { return nil }
-            return ImageWork.uprightThumbnail(jpeg: data)
-        }.value
-    }
-
     /// Forgets keyframes and stills taken in a world frame that no longer exists (after a failed
     /// relocalization), the meter close-up included: export and the packet read only what
     /// `keyframes`, `stills` and `stillFrames` list, so a close-up skipped in the new frame exports

ios/HouseScan/Runtime/LaunchOptions.swift

@@ -28,6 +28,10 @@ import OSLog
 ///   ground, as ARKit refining the ground would (`ScanEngine.injectGroundForTest`). Replays carry
 ///   no plane evidence, so this is the only way a UI test reaches that path. Meters must be over
 ///   the engine's 1 cm refine threshold and at most `GroundPlaneChoice.maximumRaise`.
+/// - `-failCloseUpSave`: with `-replay`, every meter close-up's photo fails to save, as on a phone
+///   with no space left, so a UI test reaches the save-failure retake and the skip after it. The
+///   photo is dropped before `KeyframeStore.saveStill`, which then takes its own failure path.
+///   The retake reason stays up 10 s instead of 2, so the test's query can't miss it.
 /// - `-simulateAppStore`: run as an App Store install would, so the developer options and practice
 ///   meter are unavailable whatever the stored switch says (`DeveloperSettings`). It can only take
 ///   the switch away, never offer it.
@@ -42,6 +46,7 @@ struct LaunchOptions: Equatable {
     var autopilotSomethingThere = false
     var simulateAppStore = false
     var injectGroundRise: Float?
+    var failCloseUpSave = false
 
     init(
         arguments: [String] = ProcessInfo.processInfo.arguments,
@@ -58,6 +63,7 @@ struct LaunchOptions: Equatable {
         autopilotCantGetThere = arguments.contains("-autopilotCantGetThere")
         autopilotSomethingThere = arguments.contains("-autopilotSomethingThere")
         simulateAppStore = arguments.contains("-simulateAppStore")
+        failCloseUpSave = arguments.contains("-failCloseUpSave")
         serverURL = (value(after: "-serverURL") ?? defaultServerURL).flatMap(Self.serverURL)
         sampleResult = arguments.contains("-sampleResult")
         if let gate = value(after: "-autopilotGate") { autopilotGate = URL(fileURLWithPath: gate, isDirectory: true) }

ios/HouseScan/Runtime/ScanEngine.swift

@@ -756,7 +756,7 @@ final class ScanEngine {
         state.coaching = coaching(for: frame.tracking, skip: nil)
         // After a retake request the shutter waits long enough for the reason to be read (and,
         // for "move closer", acted on) before the hold can start again.
-        if let retake = closeUpRetake, screenTime - retake.since < Self.retakeNotice {
+        if let retake = closeUpRetake, screenTime - retake.since < currentRetakeNotice {
             state.closeUp = .aiming(hold: 0, problem: retake.problem)
             return
         }
@@ -786,6 +786,10 @@ final class ScanEngine {
     /// Seconds a retake reason stays up before the next close-up can be taken. A guess to try
     /// on a phone, not measured: long enough to read one short line.
     private static let retakeNotice: Double = 2
+    /// Under `-failCloseUpSave` the reason stays up 10 s, so the UI test's query can't miss it:
+    /// the replay is close to the meter for under a second, and after the 2 s notice the gate's
+    /// "Move closer" replaces it.
+    private var currentRetakeNotice: Double { options.failCloseUpSave && replay != nil ? 10 : Self.retakeNotice }
 
     private var screenTime: Double { Double(DispatchTime.now().uptimeNanoseconds) / 1_000_000_000 }
 
@@ -827,42 +831,55 @@ final class ScanEngine {
         Task {
             // The task can start after a reset or after the flow left the close-up.
             guard scan == generation, state.phase == .meterCloseUp else { return }
-            let photo = await closeUpPhoto(frame)
+            var photo = await closeUpPhoto(frame)
+            if options.failCloseUpSave, replay != nil { photo.jpeg = .none }
             // Drawing a practice photo suspends: a reset meanwhile must not save into the new scan.
             guard scan == generation, state.phase == .meterCloseUp else { return }
             let saved = await store.saveStill(photo, name: "meter_close.jpg")
             guard scan == generation, state.phase == .meterCloseUp else { return }
-            if !saved {
-                retakeCloseUp(.blurry)
+            switch saved {
+            case .saved:
+                break
+            case .notWritten:
+                retakeCloseUp(.photoNotSaved)
+                return
+            case .worldDiscarded:
+                // The discard sent the flow back to finding the meter, so the phase guard above
+                // returns first. Past it, the meter was found again while this file was written:
+                // the shot belongs to the old frame, and the new close-up is left to take its own.
+                RuntimeLog.engine.info("close-up from a discarded world frame dropped")
                 return
             }
-            let thumbnail = await store.thumbnail(ofStill: "meter_close.jpg")
+            // Read back once, before the capture is acknowledged: the thumbnail and the meter
+            // reader both use these bytes, and a photo that can't be read back wasn't kept.
+            let file = store.directory.appending(path: "meter_close.jpg")
+            let readBack = await Task.detached(priority: .userInitiated) { () -> (jpeg: Data, thumbnail: CGImage?)? in
+                guard let jpeg = try? Data(contentsOf: file) else { return nil }
+                return (jpeg, ImageWork.uprightThumbnail(jpeg: jpeg))
+            }.value
             guard scan == generation, state.phase == .meterCloseUp else { return }
-            state.closeUp = .captured(thumbnail)
+            guard let readBack else {
+                RuntimeLog.engine.error("close-up photo could not be read back")
+                closeUpCredit.photoChecked(view, passed: false)
+                retakeCloseUp(.photoNotSaved)
+                return
+            }
+            state.closeUp = .captured(readBack.thumbnail)
             state.captureCount += 1
-            state.lastCapture = CaptureEvent(id: state.captureCount, kind: .closeUp, thumbnail: thumbnail)
-            await readMeterNumber(scan: scan, view: view)
+            state.lastCapture = CaptureEvent(id: state.captureCount, kind: .closeUp, thumbnail: readBack.thumbnail)
+            await readMeterNumber(readBack.jpeg, scan: scan, view: view)
         }
     }
 
-    /// Reads the meter number from the saved close-up, off the main actor, then offers the
-    /// candidates for the homeowner to pick from (never filling one in) or asks for a retake.
+    /// Reads the meter number from the saved close-up's bytes, off the main actor, then offers
+    /// the candidates for the homeowner to pick from (never filling one in) or asks for a retake.
     /// `view` is the shot's, which coverage may take once the reader has passed its photo.
-    private func readMeterNumber(scan: Int, view: CloseUpView?) async {
+    private func readMeterNumber(_ jpeg: Data, scan: Int, view: CloseUpView?) async {
         state.meterNumber = .reading
         let reader = MeterNumberReaders.make()
-        let photo = store.directory.appending(path: "meter_close.jpg")
-        let readout = await Task.detached(priority: .userInitiated) { () -> MeterReadout? in
-            guard let jpeg = try? Data(contentsOf: photo) else { return nil }
-            return await reader.read(jpeg: jpeg)
-        }.value
+        let readout = await Task.detached(priority: .userInitiated) { await reader.read(jpeg: jpeg) }.value
         guard scan == generation, state.phase == .meterCloseUp, state.meterNumber == .reading else { return }
-        closeUpCredit.photoChecked(view, passed: readout?.photoPassedChecks == true)
-        guard let readout else {
-            RuntimeLog.engine.error("close-up photo could not be read back for the meter number")
-            retakeCloseUp(.noNumber)
-            return
-        }
+        closeUpCredit.photoChecked(view, passed: readout.photoPassedChecks)
         guard !readout.candidates.isEmpty else {
             retakeCloseUp(readout.retake ?? .noNumber)
             return

ios/HouseScan/UI/Copy/ScanCopy.swift

@@ -260,6 +260,7 @@ enum ScanCopy {
         case .tracking: "Move slowly"
         case .numberTooSmall: "Move closer so the number looks bigger"
         case .noNumber: "Couldn't read the number. Hold still for another photo."
+        case .photoNotSaved: "Your phone couldn't save that photo. Hold on the meter to try again."
         }
     }
 

ios/HouseScanUITests/CloseUpSaveFailureUITests.swift

@@ -0,0 +1,49 @@
+import XCTest
+
+/// B-20: a meter close-up the phone couldn't save read "Hold still", as if the homeowner's hand
+/// had blurred it, and retaking with a steadier hand could never help. `-failCloseUpSave` makes
+/// every close-up's save fail on a replay, through the real engine and `KeyframeStore.saveStill`.
+final class CloseUpSaveFailureUITests: XCTestCase {
+    static let notSaved = "Your phone couldn't save that photo. Hold on the meter to try again."
+
+    override func setUp() {
+        continueAfterFailure = false
+    }
+
+    @MainActor
+    func testAPhotoThatDidNotSaveSaysSoAndCanBeSkipped() throws {
+        let app = XCUIApplication()
+        app.launchArguments = ["-replay", FullFlowUITests.fixture, "-sampleResult", "-failCloseUpSave"]
+        app.launch()
+        let any = app.descendants(matching: .any)
+
+        XCTAssertTrue(any["screen.onboarding"].waitForExistence(timeout: 30))
+        if app.buttons["action.onboardingSkip"].waitForExistence(timeout: 5) { app.buttons["action.onboardingSkip"].tap() }
+        XCTAssertTrue(app.buttons["action.finishOnboarding"].waitForExistence(timeout: 10))
+        app.buttons["action.finishOnboarding"].tap()
+        XCTAssertTrue(app.buttons["action.markMeter"].waitForExistence(timeout: 20))
+        app.buttons["action.markMeter"].tap()
+        XCTAssertTrue(any["screen.meterCloseUp"].waitForExistence(timeout: 15))
+
+        // Under `-failCloseUpSave` the reason stays up 10 s rather than 2 before the gate's own
+        // "Move closer" replaces it, so this query can't miss it.
+        let saidNotSaved = any.matching(identifier: "closeUp.problem").matching(NSPredicate(format: "label == %@", Self.notSaved)).firstMatch
+        XCTAssertTrue(saidNotSaved.waitForExistence(timeout: 60), "a close-up that didn't save never said so")
+        snap(app, "meterCloseUp-photoNotSaved")
+
+        // The save failure counted as one failed attempt; the next one (another failed save, or
+        // `CloseUpGate.failAfter` without a photo) brings the way out.
+        let skip = app.buttons["action.skipCloseUp"]
+        XCTAssertTrue(skip.waitForExistence(timeout: 90), "no way past a close-up that can't be saved")
+        skip.tap()
+        XCTAssertTrue(any["screen.wallWalk"].waitForExistence(timeout: 15), "skipping the unsaved close-up didn't lead on to the walk")
+    }
+
+    @MainActor
+    private func snap(_ app: XCUIApplication, _ name: String) {
+        let shot = XCTAttachment(screenshot: app.screenshot())
+        shot.name = name
+        shot.lifetime = .keepAlways
+        add(shot)
+    }
+}