From ca98508371943576f43724a638acfe43a06d6b46 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Thu, 1 Oct 2026 14:42:59 -0500 Subject: [PATCH 1/9] chore(review): run Greptile again when PR commits change --- .greptile/config.json | 3 +++ CONTRIBUTING.md | 2 ++ 2 files changed, 5 insertions(+) create mode 100644 .greptile/config.json diff --git a/.greptile/config.json b/.greptile/config.json new file mode 100644 index 000000000..9d6735365 --- /dev/null +++ b/.greptile/config.json @@ -0,0 +1,3 @@ +{ + "autoReview": ["open", "push"] +} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index feca4fe13..c0b53fc1a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -22,6 +22,8 @@ State what the old code did before describing the new behavior. Link the reviewe The `size:*` label describes the effective diff. It is a review signal, not a merge gate. Do not set it by hand. +Greptile reviews new commits through [`.greptile/config.json`](.greptile/config.json). The dashboard's open-only trigger left follow-up commits unreviewed. This repository override adds push reviews without changing other repositories' settings. Greptile reads this setting from the PR's source branch, so older branches need the config too. For a branch without it, request a current review with `@greptileai review` after pushing. Use `@greptileai review this draft` for drafts. + ## Checks | Check | Runs on GitHub when | Local command | From 3e75514a625a204e2dfb5a648cd431f0946bcc5d Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 18:16:30 -0500 Subject: [PATCH 2/9] Show a passing spot as a possible spot, not a fit The server's checks pass on observed space beyond the area the homeowner confirms in the spot check, some of it inferred rather than seen (a walked path, a photo's view), and the answer doesn't say which. The app can't tell that all the space a battery needs was confirmed (B17), so no result says a battery fits. ResultReading's affirmative answer becomes `candidate`: "A possible battery spot", with a note that the scan couldn't confirm all the space and an installer needs to check the fit on site. Every path that read as a fit goes through it, policy-only manual review included, so a retry, a clear answer or a settled earlier answer can't restore one. The other answers are unchanged, and so are the server's answer, its binding, the scene and the packet. Every result surface agrees. The card shows only failing and unsure checks. Details calls its list the server's calculations on what the scan recorded, drops a pass's positive server reason (keeping its measurement), and hides a candidate's pass summary. The AR title, overlay and off-screen direction name the proposed spot, and passing clearance zones draw in the proposal's blue rather than clear-ground green. The result arrives with a light tap instead of the success haptic (B26). The card's pass ranking was the only use of ResultReading.margin, so it is removed. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../Contract/ResultPresentation+Reading.swift | 14 +- ios/HouseScan/UI/Copy/ScanCopy.swift | 29 ++- ios/HouseScan/UI/Result/ResultScene3D.swift | 16 +- ios/HouseScan/UI/ScanRootView.swift | 19 +- ios/HouseScan/UI/Screens/ResultARScreen.swift | 20 +- ios/HouseScan/UI/Screens/ResultScreen.swift | 59 ++++- ios/HouseScan/UI/Theme/Theme.swift | 7 + .../Scene/ResultCardActions.swift | 2 +- .../HouseScanKit/Scene/ResultReading.swift | 56 ++-- .../ResultCardActionsTests.swift | 6 +- .../ResultReadingTests.swift | 105 ++++++-- .../ProvisionalPlacementUITests.swift | 241 ++++++++++++++++++ 12 files changed, 458 insertions(+), 116 deletions(-) create mode 100644 ios/HouseScanUITests/ProvisionalPlacementUITests.swift diff --git a/ios/HouseScan/Contract/ResultPresentation+Reading.swift b/ios/HouseScan/Contract/ResultPresentation+Reading.swift index 19750bb65..7689343b4 100644 --- a/ios/HouseScan/Contract/ResultPresentation+Reading.swift +++ b/ios/HouseScan/Contract/ResultPresentation+Reading.swift @@ -76,10 +76,7 @@ extension ResultPresentation { checks.map { row in ResultReading.Check( id: row.id, outcome: row.outcome.placementOutcome, needsPerson: row.needsPerson, - viewCapturable: viewToTake(for: row) != nil, - margin: ResultReading.margin( - measured: row.measured.map(Double.init), threshold: row.threshold.map(Double.init), - plusMinus: row.plusMinus.map(Double.init), comparison: row.comparison?.placementComparison) + viewCapturable: viewToTake(for: row) != nil ) } } @@ -102,12 +99,3 @@ private extension CheckOutcome { } } } - -private extension RuleComparison { - var placementComparison: PlacementComparison { - switch self { - case .atLeast: .atLeast - case .atMost: .atMost - } - } -} diff --git a/ios/HouseScan/UI/Copy/ScanCopy.swift b/ios/HouseScan/UI/Copy/ScanCopy.swift index 24f23609e..a8829d756 100644 --- a/ios/HouseScan/UI/Copy/ScanCopy.swift +++ b/ios/HouseScan/UI/Copy/ScanCopy.swift @@ -627,10 +627,11 @@ enum ScanCopy { // MARK: Result - /// The answer in the homeowner's words (`ResultPresentation.answer`). + /// The answer in the homeowner's words (`ResultPresentation.answer`). A passing spot is a + /// possible one, never "fits": see `candidateNote`. static func headline(_ answer: ResultReading.Answer) -> String { switch answer { - case .fits: "A battery fits here" + case .candidate: "A possible battery spot" case .oneMoreLook: "One more look" case .installer: "Needs an installer's review" case .notHere: "Not on this wall" @@ -690,7 +691,18 @@ enum ScanCopy { return "Settles: \(titles.joined(separator: ", "))" } - static let seeOnWall = "See it on your wall" + /// Under a possible spot's headline and placement. The server's checks pass on space the scan + /// recorded, some of it beyond what the homeowner confirmed in the spot check, so the app + /// can't say all the space a battery needs is clear (`ResultReading`, B17). It says what the + /// fit still needs, never that a review was sent: the app contacts nobody. + static let candidateNote = "The scan suggests this spot, but it couldn't confirm all the space a battery needs. An installer needs to check the fit on site." + + /// The AR title over a possible spot, and the words for what the AR view draws there. + static let proposedSpotOverlay = "A proposed battery spot, drawn on your wall" + static let proposedSpotThisWay = "The proposed spot is this way" + static let proposedSpotOffScreen = "The proposed spot is off screen. Turn the phone toward the arrow." + + static let seeOnWall = "See this spot on your wall" /// For a spot an installer still has to confirm against the meter's working space. static let seeClosest = "See the closest spot" static let showMe = "Show me" @@ -703,8 +715,8 @@ enum ScanCopy { static let wallNotMeasuredDetail = "The scan stopped before you walked along the wall on either side of your meter, so we can't tell where a battery would fit. Scan again and walk a few steps each way." static let scanAgain = "Scan again" - /// Shown on every result, with or without a spot ("Not on this wall" has none), so it names - /// no spot. + /// Shown on every result but a possible spot, whose `candidateNote` says it already, with or + /// without a spot ("Not on this wall" has none), so it names no spot. static let installerConfirms = "Before any battery goes in, an installer has to confirm where it goes on site." static let rulesNotFinal = "The placement rules aren't final yet, so every result needs an installer's review for now." // The server's result covers where the battery goes, not the panel itself. @@ -807,9 +819,14 @@ enum ScanCopy { } } + /// The heading over every check in Details, and what a passing one means there: the server's + /// calculation on the scan's measurements, not space anyone confirmed clear. + static let calculatedTitle = "What the server calculated" + static let calculatedNote = "From what your scan recorded. A passing check means those measurements meet the rule, not that the space is confirmed clear." + static func outcomeWord(_ outcome: CheckOutcome) -> String { switch outcome { - case .pass: "Looks good" + case .pass: "Meets the rule on what the scan recorded" case .unsure: "Not sure yet" case .fail: "Doesn't work" } diff --git a/ios/HouseScan/UI/Result/ResultScene3D.swift b/ios/HouseScan/UI/Result/ResultScene3D.swift index a95a45813..678583e7f 100644 --- a/ios/HouseScan/UI/Result/ResultScene3D.swift +++ b/ios/HouseScan/UI/Result/ResultScene3D.swift @@ -220,7 +220,7 @@ struct ResultScene3D: View { } for (index, zone) in result.clearances.enumerated() { - var material = UnlitMaterial(color: SceneColor.outcome(zone.outcome)) + var material = UnlitMaterial(color: SceneColor.zone(zone.outcome)) material.blending = .transparent(opacity: .init(floatLiteral: 0.35)) // Stacked zones sit a few millimeters apart so overlapping ones don't flicker. let lift = ResultMarkLayout.zoneLift(index: index, base: Self.zoneBase) @@ -467,11 +467,11 @@ struct ResultScene3D: View { } var parts: [String] if spot.span.lowerBound > 0 { - parts = ["Battery \(Distance.spoken(spot.span.lowerBound)) right of your meter"] + parts = ["Proposed battery spot \(Distance.spoken(spot.span.lowerBound)) right of your meter"] } else if spot.span.upperBound < 0 { - parts = ["Battery \(Distance.spoken(-spot.span.upperBound)) left of your meter"] + parts = ["Proposed battery spot \(Distance.spoken(-spot.span.upperBound)) left of your meter"] } else { - parts = ["Battery below your meter"] + parts = ["Proposed battery spot below your meter"] } if let cable = result.cableLength { parts.append("cable \(Distance.spoken(cable))") @@ -549,7 +549,7 @@ enum ResultARModel { wall.world(s: s, height: height, out: out) - wall.meter } for (index, zone) in result.clearances.enumerated() { - var material = UnlitMaterial(color: SceneColor.outcome(zone.outcome)) + var material = UnlitMaterial(color: SceneColor.zone(zone.outcome)) material.blending = .transparent(opacity: .init(floatLiteral: 0.35)) let width = zone.span.upperBound - zone.span.lowerBound let middle = zone.span.lowerBound + width / 2 @@ -659,9 +659,11 @@ private enum SceneColor { } } - static func outcome(_ outcome: CheckOutcome) -> UIColor { + /// A clearance zone's tint, as `Palette.zone`: where the server's sweep passes, the + /// proposal's blue, not a green that reads as ground confirmed clear. + static func zone(_ outcome: CheckOutcome) -> UIColor { switch outcome { - case .pass: rgb(0x2FC273) + case .pass: signal case .unsure: rgb(0xF5B53D) case .fail: rgb(0xFF5A4E) } diff --git a/ios/HouseScan/UI/ScanRootView.swift b/ios/HouseScan/UI/ScanRootView.swift index 9cc143054..54c545573 100644 --- a/ios/HouseScan/UI/ScanRootView.swift +++ b/ios/HouseScan/UI/ScanRootView.swift @@ -93,15 +93,20 @@ struct ScanRootView: View { /// Haptics for the moments that matter, fired on the same state change the screen animates: /// a deliberate photo, the meter pinned, a mark placed or refused, a requested view done, the /// result. +/// +/// The result arrives with a light tap, whatever it says. `.success` there celebrated every +/// answer alike, a rejected wall included, and on a possible spot it said "done, it fits" when +/// the scan couldn't confirm the space (B17, B26). Coming back from the AR view taps nothing. private struct ScanHaptics: ViewModifier { let state: ScanViewState - // Two steps: the five-modifier chain took 215 ms to type-check on Swift 6.4, and CI's - // Swift 6.2 has failed on slower expressions before. + // Steps of at most three: the five-modifier chain took 215 ms to type-check on Swift 6.4, and + // CI's Swift 6.2 has failed on slower expressions before. func body(content: Content) -> some View { let captures = content .sensoryFeedback(.impact(weight: .light, intensity: 0.7), trigger: deliberateCaptureID) { _, new in new != nil } - .sensoryFeedback(.success, trigger: state.phase, condition: Self.isMilestone) + .sensoryFeedback(.success, trigger: state.phase, condition: Self.meterPinned) + .sensoryFeedback(.impact(weight: .light), trigger: state.phase, condition: Self.resultArrived) return captures .sensoryFeedback(.success, trigger: state.gap?.isSatisfied ?? false) { _, new in new } .sensoryFeedback(.impact(weight: .medium), trigger: state.features.count) { old, new in new > old } @@ -116,8 +121,12 @@ private struct ScanHaptics: ViewModifier { return capture.id } - private static func isMilestone(_ old: ScanPhase, _ new: ScanPhase) -> Bool { - (old == .findMeter && new == .meterCloseUp) || (old != .resultAR && new == .result) + private static func meterPinned(_ old: ScanPhase, _ new: ScanPhase) -> Bool { + old == .findMeter && new == .meterCloseUp + } + + private static func resultArrived(_ old: ScanPhase, _ new: ScanPhase) -> Bool { + old != .resultAR && new == .result } } diff --git a/ios/HouseScan/UI/Screens/ResultARScreen.swift b/ios/HouseScan/UI/Screens/ResultARScreen.swift index ef697d885..14bb00bbc 100644 --- a/ios/HouseScan/UI/Screens/ResultARScreen.swift +++ b/ios/HouseScan/UI/Screens/ResultARScreen.swift @@ -56,7 +56,7 @@ struct ResultARScreen: View { private static func overlayLabel(_ result: ResultPresentation) -> String { guard result.spot != nil else { return "The cable run and clearances, drawn on your wall" } return result.spotIsClean - ? "The battery, drawn on your wall at its spot" + ? ScanCopy.proposedSpotOverlay : "An outline of the spot an installer needs to check, drawn on your wall" } @@ -67,17 +67,17 @@ struct ResultARScreen: View { guard let result = state.result, result.spot != nil else { return Instruction(title: "Point at your meter", detail: nil) } - // Only a pass under approved rules may sound settled; a spot an installer still has to - // check says so, as the result screen does. - let title = result.decision == .pass && result.policyApproved && result.spotIsClean - ? "Your battery could go here" - : "The spot an installer needs to check" + // The same answer as the result screen: a possible spot says it's possible and what it + // still needs; any other spot an installer still has to check says so. + let candidate = result.answer == .candidate + let title = candidate ? ScanCopy.headline(.candidate) : "The spot an installer needs to check" let placement = ScanCopy.placement(result) guard !result.isSample else { // A sample spot drawn on the homeowner's real wall must not pass for their result. return Instruction(title: "Example spot, not your result", detail: ["No server checked this scan.", placement].compactMap { $0 }.joined(separator: " ")) } - return Instruction(title: title, detail: placement) + let onSite = candidate ? "An installer needs to check the fit on site." : nil + return Instruction(title: title, detail: [placement.map { "\($0)." }, onSite].compactMap { $0 }.joined(separator: " ")) } } @@ -107,7 +107,7 @@ private struct SpotDirection: View { if let chevron = chevronPlacement(in: size) { ZStack { TargetMarker(placement: .offScreen(chevron.point, angle: chevron.angle)) - Text("Your battery spot is this way") + Text(ScanCopy.proposedSpotThisWay) .font(.subheadline.weight(.semibold)) .foregroundStyle(Palette.chalk) .multilineTextAlignment(.center) @@ -122,7 +122,7 @@ private struct SpotDirection: View { } .frame(width: size.width, height: size.height) .accessibilityElement(children: .ignore) - .accessibilityLabel("Your battery spot is off screen. Turn the phone toward the arrow.") + .accessibilityLabel(ScanCopy.proposedSpotOffScreen) .accessibilityIdentifier("ar.spotDirection") } } @@ -187,7 +187,7 @@ struct BatteryOverlay: View, Animatable { private func drawClearances(in context: inout GraphicsContext, _ geometry: WallProjection) { for zone in result.clearances { guard let quad = geometry.groundQuad(s: zone.span, out: 0...zone.depth, height: 0.01) else { continue } - let color = Palette.outcome(zone.outcome) + let color = Palette.zone(zone.outcome) context.fill(quad, with: .color(color.opacity(0.28 * rise))) context.stroke(quad, with: .color(color.opacity(0.9 * rise)), style: StrokeStyle(lineWidth: 2, dash: [8, 6])) } diff --git a/ios/HouseScan/UI/Screens/ResultScreen.swift b/ios/HouseScan/UI/Screens/ResultScreen.swift index 9b8c88ae3..e4234a39b 100644 --- a/ios/HouseScan/UI/Screens/ResultScreen.swift +++ b/ios/HouseScan/UI/Screens/ResultScreen.swift @@ -127,7 +127,9 @@ struct ResultScreen: View { /// Plain small print, never boxes: none of it changes what the homeowner does next. private func footnotes(_ result: ResultPresentation) -> some View { VStack(alignment: .leading, spacing: 6) { - footnote(ScanCopy.installerConfirms, id: "result.installerConfirms") + if result.answer != .candidate { + footnote(ScanCopy.installerConfirms, id: "result.installerConfirms") + } if let notice = result.rulesNotice { footnote(notice, id: "result.rulesNotice") } @@ -160,7 +162,10 @@ struct ResultScreen: View { private func details(_ result: ResultPresentation) -> some View { DisclosureGroup(isExpanded: $detailsExpanded) { VStack(alignment: .leading, spacing: 22) { - if !result.summary.isEmpty { + // A possible spot's summary is the server's own pass ("fits every check"), which + // the card's candidate note contradicts on purpose; the summary of any other answer + // explains what failed or is unsure, so it stays. + if !result.summary.isEmpty, result.answer != .candidate { Text(result.summary) .font(Typeface.hint) .foregroundStyle(Palette.muted) @@ -300,6 +305,12 @@ private struct AnswerCard: View { .foregroundStyle(Palette.signalText) .fixedSize(horizontal: false, vertical: true) .accessibilityIdentifier("result.placement") + if answer == .candidate { + Text(ScanCopy.candidateNote) + .font(Typeface.hint) + .fixedSize(horizontal: false, vertical: true) + .accessibilityIdentifier("result.candidateNote") + } } else if let nearest = ScanCopy.nearest(result) { Text(nearest) .font(Typeface.hint) @@ -420,7 +431,7 @@ private struct CardCheckLine: View { HStack(alignment: .firstTextBaseline, spacing: 12) { Image(systemName: CheckSymbol.name(row.outcome)) .font(.body.weight(.semibold)) - .foregroundStyle(Palette.outcomeInk(row.outcome)) + .foregroundStyle(CheckSymbol.ink(row.outcome)) .frame(width: 22) VStack(alignment: .leading, spacing: 3) { Text(row.title) @@ -469,13 +480,19 @@ private struct Reveal: ViewModifier { } private enum CheckSymbol { + /// A pass is a muted outline: it met the rule on what the scan recorded, and a filled green + /// badge read as space confirmed clear. static func name(_ outcome: CheckOutcome) -> String { switch outcome { - case .pass: "checkmark.circle.fill" + case .pass: "checkmark.circle" case .unsure: "questionmark.circle.fill" case .fail: "xmark.circle.fill" } } + + static func ink(_ outcome: CheckOutcome) -> Color { + outcome == .pass ? Palette.muted : Palette.outcomeInk(outcome) + } } // MARK: - Details @@ -487,9 +504,16 @@ private struct ChecksList: View { var body: some View { VStack(alignment: .leading, spacing: 12) { - Text("What we checked") - .font(Typeface.sectionTitle) - .accessibilityAddTraits(.isHeader) + VStack(alignment: .leading, spacing: 4) { + Text(ScanCopy.calculatedTitle) + .font(Typeface.sectionTitle) + .accessibilityAddTraits(.isHeader) + Text(ScanCopy.calculatedNote) + .font(.subheadline) + .foregroundStyle(Palette.muted) + .fixedSize(horizontal: false, vertical: true) + .accessibilityIdentifier("result.calculatedNote") + } VStack(spacing: 0) { ForEach(Array(checks.enumerated()), id: \.element.id) { index, row in CheckRowView(row: row, photoOffered: photoOffered.contains(row.id)) @@ -507,20 +531,29 @@ private struct CheckRowView: View { var row: CheckRow var photoOffered: Bool + /// The server's reason, except for a pass: its reasons ("Nothing limits the clear space…") + /// state as fact what is only a calculation on recorded space. A pass keeps its measurement + /// against the rule. + private var reason: String? { + row.outcome == .pass ? nil : row.reason + } + var body: some View { HStack(alignment: .top, spacing: 12) { Image(systemName: CheckSymbol.name(row.outcome)) .font(.title3.weight(.semibold)) - .foregroundStyle(Palette.outcomeInk(row.outcome)) + .foregroundStyle(CheckSymbol.ink(row.outcome)) .frame(width: 28) .accessibilityHidden(true) VStack(alignment: .leading, spacing: 4) { Text(row.title) .font(Typeface.hint.weight(.semibold)) - Text(row.reason) - .font(.subheadline) - .foregroundStyle(Palette.muted) - .fixedSize(horizontal: false, vertical: true) + if let reason { + Text(reason) + .font(.subheadline) + .foregroundStyle(Palette.muted) + .fixedSize(horizontal: false, vertical: true) + } if let measurement = ScanCopy.measurement(row) { Text(measurement) .font(.subheadline.weight(.medium)) @@ -537,7 +570,7 @@ private struct CheckRowView: View { .padding(14) .accessibilityElement(children: .combine) .accessibilityLabel("\(row.title): \(ScanCopy.outcomeWord(row.outcome))") - .accessibilityValue([row.reason, ScanCopy.measurement(row, spoken: true), row.outcome == .unsure ? ScanCopy.unsureNote(photoOffered: photoOffered) : nil] + .accessibilityValue([reason, ScanCopy.measurement(row, spoken: true), row.outcome == .unsure ? ScanCopy.unsureNote(photoOffered: photoOffered) : nil] .compactMap(\.self).joined(separator: ". ")) // The card shows the deciding checks as `check.`; this is the full list. .accessibilityIdentifier("detail.check.\(row.id)") diff --git a/ios/HouseScan/UI/Theme/Theme.swift b/ios/HouseScan/UI/Theme/Theme.swift index 2313e425f..e11f9ba2d 100644 --- a/ios/HouseScan/UI/Theme/Theme.swift +++ b/ios/HouseScan/UI/Theme/Theme.swift @@ -68,6 +68,13 @@ enum Palette { } } + /// A result's clearance zone in AR. Where the server's sweep passes, the proposal's blue + /// rather than `covered` green, which read as ground confirmed clear: the checks pass there + /// on recorded space, not on space anyone confirmed (B17). + static func zone(_ outcome: CheckOutcome) -> Color { + outcome == .pass ? signal : self.outcome(outcome) + } + static func outcomeInk(_ outcome: CheckOutcome) -> Color { switch outcome { case .pass: passInk diff --git a/ios/HouseScanKit/Sources/HouseScanKit/Scene/ResultCardActions.swift b/ios/HouseScanKit/Sources/HouseScanKit/Scene/ResultCardActions.swift index 1a77f1aad..cfb7d8f88 100644 --- a/ios/HouseScanKit/Sources/HouseScanKit/Scene/ResultCardActions.swift +++ b/ios/HouseScanKit/Sources/HouseScanKit/Scene/ResultCardActions.swift @@ -23,7 +23,7 @@ public enum ResultCardActions { public static func primary(answer: ResultReading.Answer, hasSpot: Bool, spotIsClean: Bool, hasViewToTake: Bool, sourceAvailable: Bool) -> Primary? { switch answer { - case .fits: + case .candidate: return hasSpot && sourceAvailable ? .showAR(clean: true) : nil case .oneMoreLook: return offersView(capturable: hasViewToTake, sourceAvailable: sourceAvailable) ? .takeView : nil diff --git a/ios/HouseScanKit/Sources/HouseScanKit/Scene/ResultReading.swift b/ios/HouseScanKit/Sources/HouseScanKit/Scene/ResultReading.swift index 8330d35cd..008d614d5 100644 --- a/ios/HouseScanKit/Sources/HouseScanKit/Scene/ResultReading.swift +++ b/ios/HouseScanKit/Sources/HouseScanKit/Scene/ResultReading.swift @@ -3,8 +3,14 @@ import Foundation // What the result screen leads with, read from the server's answer. The app's screen turns these // into words; the rules that pick them live here so each one has a test against a decoded answer // (ResultReadingTests). They read the checks as well as `decision`: a manual_review held back -// only by rules that aren't approved yet still fits, and a spot that might stand in the meter's -// working space goes to an installer whatever the decision says. +// only by rules that aren't approved yet is still a candidate, and a spot that might stand in the +// meter's working space goes to an installer whatever the decision says. +// +// No answer says a battery fits. A spot whose checks all pass is a candidate: the server's checks +// read observed space beyond the area the homeowner confirms in the spot check, some of it +// inferred rather than seen (a walked path, a photo's view; `CoverageMap`, "Bounded +// exceptions"), and the answer doesn't say which. Until it does, the app can't tell that all the +// space a battery needs was confirmed (B17). The answer itself is unchanged: this reads it. public enum ResultReading { /// The server's check for the NEC 110.26 working space in front of the meter @@ -14,8 +20,9 @@ public enum ResultReading { /// What the screen says first. public enum Answer: String, Equatable, Sendable { - /// The spot passes every check; at most the rules' approval holds it back. - case fits + /// A proposed spot: every check passes on what the scan recorded, and at most the rules' + /// approval holds it back. Not a confirmed fit: an installer checks the space on site. + case candidate /// An unsure check that a view the camera can take now would settle. case oneMoreLook /// A person has to decide: a borderline measurement, an unknown attribute, a spot that @@ -33,16 +40,12 @@ public enum ResultReading { public var needsPerson: Bool /// True when a view the camera can take now would settle it. public var viewCapturable: Bool - /// How far the measurement clears its limit (`margin(measured:threshold:plusMinus:comparison:)`); - /// nil without both a measurement and a limit. - public var margin: Double? - public init(id: String, outcome: PlacementOutcome, needsPerson: Bool = false, viewCapturable: Bool = false, margin: Double? = nil) { + public init(id: String, outcome: PlacementOutcome, needsPerson: Bool = false, viewCapturable: Bool = false) { self.id = id self.outcome = outcome self.needsPerson = needsPerson self.viewCapturable = viewCapturable - self.margin = margin } } @@ -58,29 +61,26 @@ public enum ResultReading { if hasSpot, !spotIsClean(hasSpot: hasSpot, checks: checks) { return .installer } switch decision { case .pass: - return .fits + return .candidate case .reject: return .notHere case .manualReview: // Needs at least one check: over none, "every check passes" is vacuously true. - if hasSpot, !policyApproved, !checks.isEmpty, checks.allSatisfy({ $0.outcome == .pass }) { return .fits } + if hasSpot, !policyApproved, !checks.isEmpty, checks.allSatisfy({ $0.outcome == .pass }) { return .candidate } if checks.contains(where: { $0.outcome == .unsure && !$0.needsPerson && $0.viewCapturable }) { return .oneMoreLook } return .installer } } - /// Indices of the checks the result card shows, in order: every FAIL, then every UNSURE, then - /// the two PASSes closest to their limits (smallest `margin`; a pass without one sorts last). - /// At most `limit`. + /// Indices of the checks the result card shows, in order: every FAIL, then every UNSURE, at + /// most `limit`. No PASS: on the card a passing line read as space confirmed clear, which the + /// scan can't show (see the top of this file). Every check, passing ones included, stays in + /// the result's Details. public static func cardLines(_ checks: [Check], limit: Int = 3) -> [Int] { let indices = checks.indices let fails = indices.filter { checks[$0].outcome == .fail } let unsure = indices.filter { checks[$0].outcome == .unsure } - // Ties keep the server's order. - let passes = indices.filter { checks[$0].outcome == .pass } - .sorted { (checks[$0].margin ?? .infinity, $0) < (checks[$1].margin ?? .infinity, $1) } - .prefix(2) - return Array((fails + unsure + passes).prefix(limit)) + return Array((fails + unsure).prefix(limit)) } /// Whether a check's review line (`PlacementCheck.reviewThresholdFt`) explains its outcome, so @@ -104,20 +104,6 @@ public enum ResultReading { case .atLeast: return reviewThreshold > threshold && !(measured - error > reviewThreshold) } } - - /// How far `measured` clears `threshold` on the passing side, in units of `plusMinus` when it - /// is positive, else in the measurement's own units. Negative when it misses. Without a - /// comparison the side is unknown, so the distance to the limit either way. - public static func margin(measured: Double?, threshold: Double?, plusMinus: Double?, comparison: PlacementComparison?) -> Double? { - guard let measured, let threshold else { return nil } - let clearance = switch comparison { - case .atLeast: measured - threshold - case .atMost: threshold - measured - case nil: abs(measured - threshold) - } - guard let plusMinus, plusMinus > 0 else { return clearance } - return clearance / plusMinus - } } extension PlacementPolicy { @@ -175,9 +161,7 @@ extension PlacementResult { checks.map { check in ResultReading.Check( id: check.id, outcome: check.outcome, needsPerson: check.needsPerson, - viewCapturable: evidenceIndex(settling: check.id).map(capturable) ?? false, - margin: ResultReading.margin(measured: check.measuredFt, threshold: check.thresholdFt, - plusMinus: check.plusMinusFt, comparison: check.comparison) + viewCapturable: evidenceIndex(settling: check.id).map(capturable) ?? false ) } } diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/ResultCardActionsTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/ResultCardActionsTests.swift index 9a1cd9a53..36f547ecd 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/ResultCardActionsTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/ResultCardActionsTests.swift @@ -12,7 +12,7 @@ import Testing } @Test func withTheCameraUpEachAnswerKeepsItsButton() { - #expect(Self.primary(.fits) == .showAR(clean: true)) + #expect(Self.primary(.candidate) == .showAR(clean: true)) #expect(Self.primary(.oneMoreLook, hasViewToTake: true) == .takeView) #expect(Self.primary(.installer, spotIsClean: false) == .showAR(clean: false)) #expect(Self.primary(.installer) == .showAR(clean: true)) @@ -21,7 +21,7 @@ import Testing /// No spot, nothing to show in AR; no view to take, no "Show me". @Test func nothingToShowMeansNoButton() { - #expect(Self.primary(.fits, hasSpot: false) == nil) + #expect(Self.primary(.candidate, hasSpot: false) == nil) #expect(Self.primary(.installer, hasSpot: false) == nil) #expect(Self.primary(.oneMoreLook, hasViewToTake: false) == nil) } @@ -31,7 +31,7 @@ import Testing /// button already was. Starting over starts a new camera, so it stays. @Test func aFailedCameraOffersNothingThatNeedsIt() { #expect(Self.primary(.oneMoreLook, hasViewToTake: true, sourceAvailable: false) == nil) - #expect(Self.primary(.fits, sourceAvailable: false) == nil) + #expect(Self.primary(.candidate, sourceAvailable: false) == nil) #expect(Self.primary(.installer, spotIsClean: false, sourceAvailable: false) == nil) #expect(Self.primary(.notHere, hasSpot: false, sourceAvailable: false) == .startOver) } diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/ResultReadingTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/ResultReadingTests.swift index 3f525a0ef..e2d07e128 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/ResultReadingTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/ResultReadingTests.swift @@ -112,8 +112,9 @@ import simd #expect(result.answer { _ in true } == .oneMoreLook) } - /// A manual_review held back only by unapproved rules, with every check passing, fits. - @Test func allPassUnderUnapprovedRulesFits() throws { + /// A manual_review held back only by unapproved rules, with every check passing, is a + /// candidate like a pass: the rules' approval was never what made a spot a confirmed fit. + @Test func allPassUnderUnapprovedRulesIsACandidate() throws { var object = try Self.sampleObject() object["checks"] = [ Self.check("wall_backing", "pass"), @@ -123,7 +124,7 @@ import simd let result = try Self.decode(object) #expect(result.decision == .manualReview && !result.policy.autoApprove && result.spot != nil) #expect(result.spotIsClean) - #expect(result.answer { _ in true } == .fits) + #expect(result.answer { _ in true } == .candidate) } /// The same answer with `checks: []`, which the schema allows: no evidence is not a fit. @@ -208,7 +209,7 @@ import simd /// ios/HouseScanUITests/Fixtures/results, the answers the screenshots and UI tests show /// through `-uiDemoResultFile`. Each must stay in the schema and read as its name says. @Test(arguments: [ - ("pass", ResultReading.Answer.fits), ("reject-nearest", .notHere), + ("pass", ResultReading.Answer.candidate), ("reject-nearest", .notHere), ("unsure-view", .oneMoreLook), ("no-clean-spot", .installer), ("no-spot-no-nearest", .notHere), ("review-band", .installer), ("view-not-offered", .oneMoreLook), ]) @@ -236,9 +237,9 @@ import simd ResultReading.answer(decision: decision, policyApproved: approved, hasSpot: spot, checks: checks) } - #expect(answer(.pass, true, true, [pass]) == .fits) + #expect(answer(.pass, true, true, [pass]) == .candidate) // Only the rules' approval holds it back. - #expect(answer(.manualReview, false, true, [pass]) == .fits) + #expect(answer(.manualReview, false, true, [pass]) == .candidate) #expect(answer(.manualReview, false, false, [pass]) == .installer) #expect(answer(.manualReview, true, true, [pass, byView, byPerson]) == .oneMoreLook) #expect(answer(.manualReview, true, true, [pass, byPerson]) == .installer) @@ -255,28 +256,88 @@ import simd #expect(unexplained.needsPerson) } + // MARK: No confirmed fit (B17) + + /// The reading has no answer that says a battery fits: a passing spot rests on space the scan + /// can't show was confirmed, so the most it can be is a candidate. + @Test func noAnswerSaysABatteryFits() { + #expect(ResultReading.Answer(rawValue: "fits") == nil) + typealias Check = ResultReading.Check + let outcomes: [PlacementOutcome] = [.pass, .unsure, .fail] + // Each check set once with a check this app doesn't know, so a new server check can't + // open a shortcut to a stronger answer. + var sets: [[Check]] = [[]] + for id in ["wall_backing", "a_check_this_app_does_not_know", ResultReading.meterWorkingSpaceCheckID] { + sets = sets.flatMap { set in [set] + outcomes.map { set + [Check(id: id, outcome: $0, viewCapturable: true)] } } + } + for decision in [PlacementDecision.pass, .manualReview, .reject] { + for approved in [true, false] { + for hasSpot in [true, false] { + for checks in sets { + let answer = ResultReading.answer(decision: decision, policyApproved: approved, hasSpot: hasSpot, checks: checks) + guard answer == .candidate else { continue } + #expect(hasSpot && ResultReading.spotIsClean(hasSpot: hasSpot, checks: checks) && decision != .reject, + "\(decision) approved=\(approved) checks=\(checks.map { "\($0.id)=\($0.outcome)" })") + } + } + } + } + } + + /// An installer's review can still carry a spot, and the reading keeps it: only the words + /// over it change. + @Test func anInstallerReviewKeepsItsSpot() throws { + let result = try Self.sample(adding: Self.check(ResultReading.meterWorkingSpaceCheckID, "unsure", measured: -0.5, threshold: 0)) + #expect(result.answer { _ in true } == .installer) + #expect(result.spot != nil) + } + + /// Reading an answer changes nothing in it. A passing answer bound to the scene sent stays + /// bound, decodes to the same result before and after it is read, and keeps the server's + /// decision and every check's outcome: the candidate is how the app words it, not a new + /// answer. + @Test func readingAPassLeavesTheAnswerAsTheServerSentIt() throws { + let scene = Data(#"{"note":"the bytes this phone sent"}"#.utf8) + var object = try #require(try JSONSerialization.jsonObject(with: ResultBindingTests.answer(declaring: PacketFiles.sha256(scene))) as? [String: Any]) + object["decision"] = "pass" + object["checks"] = try #require(object["checks"] as? [[String: Any]]).map { check in + var check = check + check["outcome"] = "pass" + check["unsure_cause"] = NSNull() + return check + } + object["missing_evidence"] = [Any]() + let data = try JSONSerialization.data(withJSONObject: object) + #expect(try SceneSchemas.result().validate(data) == []) + try ResultBinding.check(answer: data, submittedScene: scene) + + let result = try PlacementResult.decode(data) + let sent = data + #expect(result.answer { _ in true } == .candidate) + #expect(ResultReading.cardLines(result.readingChecks { _ in true }).isEmpty) + #expect(data == sent) + #expect(try PlacementResult.decode(data) == result) + #expect(result.decision == .pass) + #expect(result.checks.allSatisfy { $0.outcome == .pass }) + try ResultBinding.check(answer: data, submittedScene: scene) + } + // MARK: The card's check lines - @Test func cardLinesPutFailuresFirstThenUnsureThenTheClosestPasses() { + /// The card leads with what failed and what is unsure. A passing check is never on it: the + /// card read a pass as space confirmed clear, which a candidate can't claim (B17). Every check + /// stays in Details. + @Test func cardLinesPutFailuresFirstThenUnsureAndNoPasses() { typealias Check = ResultReading.Check let checks = [ - Check(id: "wide", outcome: .pass, margin: 9), + Check(id: "pass", outcome: .pass), Check(id: "unsure", outcome: .unsure), - Check(id: "close", outcome: .pass, margin: 0.5), Check(id: "fail", outcome: .fail), - Check(id: "unmeasured", outcome: .pass), - Check(id: "closer", outcome: .pass, margin: 0.2), + Check(id: "unsure2", outcome: .unsure), + Check(id: "fail2", outcome: .fail), ] - #expect(ResultReading.cardLines(checks) == [3, 1, 5]) - let passes = checks.filter { $0.outcome == .pass } - #expect(ResultReading.cardLines(passes) == [3, 1]) // closer, close: two passes at most - } - - @Test func marginIsInUnitsOfTheErrorWhenThereIsOne() throws { - #expect(abs(try #require(ResultReading.margin(measured: 3.6, threshold: 3, plusMinus: 0.3, comparison: .atLeast)) - 2) < 1e-9) - #expect(ResultReading.margin(measured: 18, threshold: 20, plusMinus: nil, comparison: .atMost) == 2) - #expect(ResultReading.margin(measured: 2.5, threshold: 3, plusMinus: 0, comparison: .atLeast) == -0.5) - #expect(ResultReading.margin(measured: 2.5, threshold: 3, plusMinus: nil, comparison: nil) == 0.5) - #expect(ResultReading.margin(measured: nil, threshold: 3, plusMinus: 0.3, comparison: .atLeast) == nil) + #expect(ResultReading.cardLines(checks) == [2, 4, 1]) + #expect(ResultReading.cardLines(checks, limit: 5) == [2, 4, 1, 3]) + #expect(ResultReading.cardLines(checks.filter { $0.outcome == .pass }).isEmpty) } } diff --git a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift new file mode 100644 index 000000000..783438126 --- /dev/null +++ b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift @@ -0,0 +1,241 @@ +import Foundation +import XCTest + +/// A spot whose checks all pass is shown as a possible spot, never as a fit: the server's checks +/// read space beyond what the homeowner confirms in the spot check (B17). Every surface agrees: +/// the card, Details, the AR view and the 3D model. The answer itself is never changed. +final class ProvisionalPlacementUITests: XCTestCase { + private static let largestText = ["-UIPreferredContentSizeCategoryName", "UICTContentSizeCategoryAccessibilityXXXL"] + private static let candidate = "A possible battery spot" + + /// Words that would say the scan established a fit, or celebrate a pass as confirmed space. + private static let fitWords = ["fits here", "fits every", "could go here", "looks good", "your battery spot", "confirmed clear"] + + override func setUp() { + continueAfterFailure = false + } + + // MARK: The real engine + + /// The real engine on the synthetic replay, the test answering as the server through the gate + /// folder (`-answersFromGate`). The first answer names another scene, so "Try again" asks + /// again; the second is the bundled sample turned into a pass naming the scene sent. The + /// autopilot answers the spot check "It's clear". The result is a possible spot, and so is + /// the AR view's title: a retry and a clear answer don't make it a fit. + @MainActor + func testAPassAfterTryAgainAndItsClearIsAPossibleSpot() throws { + let gate = try Self.gate() + defer { try? FileManager.default.removeItem(at: gate) } + let server = GateServer(gate: gate) { body, index in + Self.passingAnswer(binding: index == 0 ? Data("another scene".utf8) : body) + } + defer { server.stop() } + let app = Self.launchEngine(gate: gate) + let any = app.descendants(matching: .any) + + let tryAgain = app.buttons["action.retryUpload"] + XCTAssertTrue(tryAgain.waitForExistence(timeout: 300), "the unbound answer never offered Try again") + tryAgain.tap() + XCTAssertTrue(any["screen.result"].waitForExistence(timeout: 120), "the passing answer never reached the result") + XCTAssertEqual(server.requests.count, 2) + XCTAssertEqual(any["result.headline"].label, Self.candidate) + XCTAssertFalse(any["result.spotRefused"].exists) + assertCandidateCard(app) + attach(app, name: "result-candidate-engine") + + // The autopilot opens the AR view once the result's gate opens. + try Data().write(to: gate.appending(path: "result")) + let instruction = any["instruction"] + XCTAssertTrue(any["screen.resultAR"].waitForExistence(timeout: 20)) + XCTAssertTrue(instruction.waitForExistence(timeout: 10)) + XCTAssertTrue(instruction.label.contains(Self.candidate), instruction.label) + XCTAssertTrue(instruction.label.contains("An installer needs to check the fit on site."), instruction.label) + assertNoFitWords(instruction.label) + attach(app, name: "resultAR-candidate-engine") + } + + /// "Something's there" withdraws the area and sends the scan again; the passing answer that + /// comes back names the same spot, so the earlier answer settles it and the result shows + /// without asking (`SpotConfirmations.settling`). That shortcut still shows a possible spot, + /// beside the notice that the homeowner said something stands there. + @MainActor + func testASpotSettledByAnEarlierAnswerIsStillAPossibleSpot() throws { + let gate = try Self.gate() + defer { try? FileManager.default.removeItem(at: gate) } + let server = GateServer(gate: gate) { body, _ in Self.passingAnswer(binding: body) } + defer { server.stop() } + let app = Self.launchEngine(gate: gate, extra: ["-autopilotSomethingThere"]) + let any = app.descendants(matching: .any) + + XCTAssertTrue(any["screen.result"].waitForExistence(timeout: 420), "the second passing answer never reached the result") + XCTAssertEqual(server.requests.count, 2, "the spot check's answer should send the scan once more") + XCTAssertEqual(any["result.headline"].label, Self.candidate) + XCTAssertTrue(any["result.spotRefused"].exists, "the settled answer's notice is missing") + assertCandidateCard(app) + attach(app, name: "result-candidate-settled") + } + + // MARK: Server answers in Fixtures/results + + /// pass.json, every check passing: the card, Details and the AR overlay at the default size. + @MainActor + func testAPassingAnswerReadsAsAPossibleSpotEverywhere() throws { + try checkPassFixture(textSize: [], name: "result-candidate") + } + + @MainActor + func testAPassingAnswerReadsAsAPossibleSpotAtLargestTextSize() throws { + try checkPassFixture(textSize: Self.largestText, name: "result-candidate-AX5") + } + + /// A rejected wall keeps its negative answer and its reasons. + @MainActor + func testARejectStaysNegative() throws { + let app = Self.launchFixture("reject-nearest", textSize: []) + defer { app.terminate() } + let any = app.descendants(matching: .any) + XCTAssertEqual(any["result.headline"].label, "Not on this wall") + XCTAssertFalse(any["result.candidateNote"].exists) + XCTAssertTrue(any["result.nearest"].label.hasPrefix("The closest spot")) + attach(app, name: "result-reject") + } + + @MainActor + private func checkPassFixture(textSize: [String], name: String) throws { + let app = Self.launchFixture("pass", textSize: textSize) + defer { app.terminate() } + let any = app.descendants(matching: .any) + XCTAssertEqual(any["result.headline"].label, Self.candidate) + assertCandidateCard(app) + let model = any.matching(NSPredicate(format: "label == '3D view of your wall'")).firstMatch + XCTAssertTrue(model.waitForExistence(timeout: 10)) + let modelValue = model.value as? String ?? "" + XCTAssertTrue(modelValue.hasPrefix("Proposed battery spot"), modelValue) + attach(app, name: name) + + // Details: the checks as calculations on the recorded scan, a pass without the server's + // positive reason, and no server summary that says the spot fits. + let details = any["result.details"] + scroll(to: details, in: app) + details.tap() + let note = any["result.calculatedNote"] + XCTAssertTrue(note.waitForExistence(timeout: 10)) + XCTAssertFalse(any["result.summary"].exists, "a possible spot's summary is the server's pass") + let row = any["detail.check.gas_clearance"] + XCTAssertTrue(row.exists) + XCTAssertEqual(row.label, "Distance from gas equipment: Meets the rule on what the scan recorded", row.label) + let rowValue = row.value as? String ?? "" + XCTAssertFalse(rowValue.contains("well clear"), "a pass kept the server's reason: \(rowValue)") + scroll(to: note, in: app) + attach(app, name: "\(name)-details") + + // The AR view draws the proposed spot. The demo's answer is a sample, so the AR title + // says so (`ResultARScreen`); the drawing names the spot as proposed. + scroll(to: any["action.showAR"], in: app) + app.buttons["action.showAR"].tap() + let overlay = any["ar.overlay"] + XCTAssertTrue(overlay.waitForExistence(timeout: 10)) + XCTAssertEqual(overlay.label, "A proposed battery spot, drawn on your wall") + assertNoFitWords(any["instruction"].label) + attach(app, name: "\(name)-AR") + } + + // MARK: Helpers + + /// The card of a possible spot: headline, placement, the note that says what isn't confirmed, + /// no passing check lines, no repeat of the installer footnote, and the AR button's words. + @MainActor + private func assertCandidateCard(_ app: XCUIApplication) { + let any = app.descendants(matching: .any) + XCTAssertTrue(any["result.placement"].exists) + let note = any["result.candidateNote"] + XCTAssertTrue(note.exists, "the candidate note is missing") + XCTAssertTrue(note.label.contains("couldn't confirm all the space a battery needs"), note.label) + let lines = any.matching(NSPredicate(format: "identifier BEGINSWITH 'check.'")) + XCTAssertEqual(lines.count, 0, "a passing check reached the card") + XCTAssertFalse(any["result.installerConfirms"].exists) + XCTAssertEqual(app.buttons["action.showAR"].label, "See this spot on your wall") + let texts = app.staticTexts.allElementsBoundByIndex.map(\.label).joined(separator: " | ") + assertNoFitWords(texts) + } + + private func assertNoFitWords(_ text: String, file: StaticString = #filePath, line: UInt = #line) { + let lowered = text.lowercased() + for words in Self.fitWords { + XCTAssertFalse(lowered.contains(words), "says \"\(words)\": \(text)", file: file, line: line) + } + } + + /// The bundled sample turned into a pass: every check passing, nothing more needed, naming + /// `binding` as the scene it answers. + private static func passingAnswer(binding: Data) -> Data { + let sample = try! Data(contentsOf: UploadRecoveryUITests.sampleResult) + var object = try! JSONSerialization.jsonObject(with: sample) as! [String: Any] + object["decision"] = "pass" + object["checks"] = (object["checks"] as! [[String: Any]]).map { check in + var check = check + check["outcome"] = "pass" + check["unsure_cause"] = NSNull() + return check + } + object["missing_evidence"] = [Any]() + var stats = object["stats"] as! [String: Any] + stats["input_sha256"] = UploadRecoveryUITests.sha256(binding) + object["stats"] = stats + return try! JSONSerialization.data(withJSONObject: object) + } + + /// A gate folder with every screen up to the spot check let through; the result's stays shut. + private static func gate() throws -> URL { + let gate = URL(fileURLWithPath: NSTemporaryDirectory()).appending(path: "housescan-gate-\(UUID().uuidString)", directoryHint: .isDirectory) + try FileManager.default.createDirectory(at: gate, withIntermediateDirectories: true) + for phase in ["onboarding", "findMeter", "meterCloseUp", "wallWalk", "markFeatures", "gapRequest", "uploading", "spotConfirm"] { + try Data().write(to: gate.appending(path: phase)) + } + return gate + } + + @MainActor + private static func launchEngine(gate: URL, extra: [String] = []) -> XCUIApplication { + let app = XCUIApplication() + app.launchArguments = [ + "-replay", FullFlowUITests.fixture, "-autopilot", "-autopilotHold", "1.5", "-autopilotGate", gate.path, + "-practiceMeter", "NO", "-serverURL", "http://placement.invalid", "-answersFromGate", + ] + extra + app.launch() + return app + } + + @MainActor + private static func launchFixture(_ name: String, textSize: [String]) -> XCUIApplication { + let file = URL(fileURLWithPath: #filePath).deletingLastPathComponent().appending(path: "Fixtures/results/\(name).json").path + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "result", + "-uiDemoResultFile", file] + textSize + app.launch() + XCTAssertTrue(app.descendants(matching: .any)["screen.result"].waitForExistence(timeout: 15)) + return app + } + + /// Scrolls until the element is inside the window, a third of the window at most per drag. + @MainActor + private func scroll(to target: XCUIElement, in app: XCUIApplication) { + let window = app.windows.firstMatch.frame + let start = app.scrollViews.firstMatch.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.6)) + for _ in 0..<12 where !window.contains(target.frame) { + let frame = target.frame + let shift: CGFloat = frame.maxY > window.maxY + ? -min(frame.maxY - window.maxY + 24, window.height / 3) + : min(window.minY - frame.minY + 24, window.height / 3) + start.press(forDuration: 0.05, thenDragTo: start.withOffset(CGVector(dx: 0, dy: shift))) + } + } + + @MainActor + private func attach(_ app: XCUIApplication, name: String) { + let shot = XCTAttachment(screenshot: app.screenshot()) + shot.name = name + shot.lifetime = .keepAlways + add(shot) + } +} From 1b7209fc73fcaf3db0e7eedf8849fe5ec8469965 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 18:20:43 -0500 Subject: [PATCH 3/9] Read a pass without a spot or passing checks as installer review ResultReading.answer read any pass decision as a candidate, so a pass with no spot or no checks still said "A possible battery spot", and the new exhaustive test caught it. A candidate now needs a spot and at least one check, every one passing; a pass that contradicts itself goes to an installer, with its checks shown as sent. A reject without a spot stays "Not on this wall", and the exhaustive test now checks that too. Details' pass word is now "Passes on recorded data", under the existing note on what the list is. The real-engine UI tests terminate their app, and the render sweep leaves out the scope note's own "not ... confirmed clear". Co-Authored-By: Claude Opus 5.5 (1M context) --- ios/HouseScan/UI/Copy/ScanCopy.swift | 2 +- .../HouseScanKit/Scene/ResultReading.swift | 14 ++++++++--- .../ResultReadingTests.swift | 23 +++++++++++++++++-- .../ProvisionalPlacementUITests.swift | 10 ++++++-- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/ios/HouseScan/UI/Copy/ScanCopy.swift b/ios/HouseScan/UI/Copy/ScanCopy.swift index a8829d756..82e9d22f5 100644 --- a/ios/HouseScan/UI/Copy/ScanCopy.swift +++ b/ios/HouseScan/UI/Copy/ScanCopy.swift @@ -826,7 +826,7 @@ enum ScanCopy { static func outcomeWord(_ outcome: CheckOutcome) -> String { switch outcome { - case .pass: "Meets the rule on what the scan recorded" + case .pass: "Passes on recorded data" case .unsure: "Not sure yet" case .fail: "Doesn't work" } diff --git a/ios/HouseScanKit/Sources/HouseScanKit/Scene/ResultReading.swift b/ios/HouseScanKit/Sources/HouseScanKit/Scene/ResultReading.swift index 008d614d5..563d9aa32 100644 --- a/ios/HouseScanKit/Sources/HouseScanKit/Scene/ResultReading.swift +++ b/ios/HouseScanKit/Sources/HouseScanKit/Scene/ResultReading.swift @@ -61,17 +61,25 @@ public enum ResultReading { if hasSpot, !spotIsClean(hasSpot: hasSpot, checks: checks) { return .installer } switch decision { case .pass: - return .candidate + // A candidate needs what makes one: a spot, and checks that all pass. A pass without a + // spot, with no checks, or with a check that didn't pass contradicts itself, and a + // person reads it; its checks stay as the server sent them. + return hasSpot && allPass(checks) ? .candidate : .installer case .reject: return .notHere case .manualReview: - // Needs at least one check: over none, "every check passes" is vacuously true. - if hasSpot, !policyApproved, !checks.isEmpty, checks.allSatisfy({ $0.outcome == .pass }) { return .candidate } + if hasSpot, !policyApproved, allPass(checks) { return .candidate } if checks.contains(where: { $0.outcome == .unsure && !$0.needsPerson && $0.viewCapturable }) { return .oneMoreLook } return .installer } } + /// At least one check, and every one passed. Over none, "every check passes" is vacuously + /// true and says nothing. + private static func allPass(_ checks: [Check]) -> Bool { + !checks.isEmpty && checks.allSatisfy { $0.outcome == .pass } + } + /// Indices of the checks the result card shows, in order: every FAIL, then every UNSURE, at /// most `limit`. No PASS: on the card a passing line read as space confirmed clear, which the /// scan can't show (see the top of this file). Every check, passing ones included, stays in diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/ResultReadingTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/ResultReadingTests.swift index e2d07e128..47cbc7729 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/ResultReadingTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/ResultReadingTests.swift @@ -275,15 +275,34 @@ import simd for hasSpot in [true, false] { for checks in sets { let answer = ResultReading.answer(decision: decision, policyApproved: approved, hasSpot: hasSpot, checks: checks) + let described = "\(decision) approved=\(approved) spot=\(hasSpot) checks=\(checks.map { "\($0.id)=\($0.outcome)" })" + // A reject without a spot stays negative whatever its checks say. + if decision == .reject, !hasSpot { #expect(answer == .notHere, "\(described)") } guard answer == .candidate else { continue } - #expect(hasSpot && ResultReading.spotIsClean(hasSpot: hasSpot, checks: checks) && decision != .reject, - "\(decision) approved=\(approved) checks=\(checks.map { "\($0.id)=\($0.outcome)" })") + // A candidate has a spot and at least one check, every one passing. + #expect(hasSpot && !checks.isEmpty && checks.allSatisfy { $0.outcome == .pass } && decision != .reject, + "\(described)") } } } } } + /// A pass that contradicts itself (no spot, no checks, or a check that didn't pass) goes to a + /// person rather than reading as a candidate, and its checks stay as sent. + @Test func anInconsistentPassGoesToAnInstaller() { + typealias Check = ResultReading.Check + let answer = { (spot: Bool, checks: [Check]) in + ResultReading.answer(decision: .pass, policyApproved: true, hasSpot: spot, checks: checks) + } + #expect(answer(false, []) == .installer) + #expect(answer(true, []) == .installer) + #expect(answer(false, [Check(id: "a", outcome: .pass)]) == .installer) + #expect(answer(true, [Check(id: "a", outcome: .pass), Check(id: "b", outcome: .unsure, viewCapturable: true)]) == .installer) + #expect(answer(true, [Check(id: "a", outcome: .pass), Check(id: "b", outcome: .fail)]) == .installer) + #expect(answer(true, [Check(id: "a", outcome: .pass)]) == .candidate) + } + /// An installer's review can still carry a spot, and the reading keeps it: only the words /// over it change. @Test func anInstallerReviewKeepsItsSpot() throws { diff --git a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift index 783438126..757c61bf1 100644 --- a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift +++ b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift @@ -31,6 +31,7 @@ final class ProvisionalPlacementUITests: XCTestCase { } defer { server.stop() } let app = Self.launchEngine(gate: gate) + defer { app.terminate() } let any = app.descendants(matching: .any) let tryAgain = app.buttons["action.retryUpload"] @@ -65,6 +66,7 @@ final class ProvisionalPlacementUITests: XCTestCase { let server = GateServer(gate: gate) { body, _ in Self.passingAnswer(binding: body) } defer { server.stop() } let app = Self.launchEngine(gate: gate, extra: ["-autopilotSomethingThere"]) + defer { app.terminate() } let any = app.descendants(matching: .any) XCTAssertTrue(any["screen.result"].waitForExistence(timeout: 420), "the second passing answer never reached the result") @@ -123,7 +125,7 @@ final class ProvisionalPlacementUITests: XCTestCase { XCTAssertFalse(any["result.summary"].exists, "a possible spot's summary is the server's pass") let row = any["detail.check.gas_clearance"] XCTAssertTrue(row.exists) - XCTAssertEqual(row.label, "Distance from gas equipment: Meets the rule on what the scan recorded", row.label) + XCTAssertEqual(row.label, "Distance from gas equipment: Passes on recorded data", row.label) let rowValue = row.value as? String ?? "" XCTAssertFalse(rowValue.contains("well clear"), "a pass kept the server's reason: \(rowValue)") scroll(to: note, in: app) @@ -155,7 +157,11 @@ final class ProvisionalPlacementUITests: XCTestCase { XCTAssertEqual(lines.count, 0, "a passing check reached the card") XCTAssertFalse(any["result.installerConfirms"].exists) XCTAssertEqual(app.buttons["action.showAR"].label, "See this spot on your wall") - let texts = app.staticTexts.allElementsBoundByIndex.map(\.label).joined(separator: " | ") + // Details' scope note says, truthfully, that a pass is "not ... confirmed clear": the sweep + // leaves that one qualified sentence out, whether or not Details is open. + let texts = app.staticTexts.allElementsBoundByIndex + .filter { $0.identifier != "result.calculatedNote" } + .map(\.label).joined(separator: " | ") assertNoFitWords(texts) } From ab7b5829ea811388f6474f605b7ccfb6efada61f Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 18:28:59 -0500 Subject: [PATCH 4/9] Keep fit claims out of every result's summary and footnotes Details hid only a candidate's summary, assuming any other summary explains what failed. A pass the reading sends to an installer still carries the server's pass summary, and the demo's review answer said "A spot 3 ft right of your meter fits" under One more look (Sol review). Details now hides the summary of any pass decision as well as any candidate; manual review and reject summaries stay. The demo's review summary names the best spot without calling it a fit, and the panel footnote says the scan suggests where the battery could go rather than covering where it can. A UI test opens Details on the demo's review and overlap answers and checks the summary is there and claims no fit. Co-Authored-By: Claude Opus 5.5 (1M context) --- ios/HouseScan/UI/Copy/ScanCopy.swift | 2 +- ios/HouseScan/UI/Preview/DemoEngine.swift | 2 +- ios/HouseScan/UI/Screens/ResultScreen.swift | 10 +++++---- .../ProvisionalPlacementUITests.swift | 22 +++++++++++++++++++ 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/ios/HouseScan/UI/Copy/ScanCopy.swift b/ios/HouseScan/UI/Copy/ScanCopy.swift index 82e9d22f5..16d7b130f 100644 --- a/ios/HouseScan/UI/Copy/ScanCopy.swift +++ b/ios/HouseScan/UI/Copy/ScanCopy.swift @@ -720,7 +720,7 @@ enum ScanCopy { static let installerConfirms = "Before any battery goes in, an installer has to confirm where it goes on site." static let rulesNotFinal = "The placement rules aren't final yet, so every result needs an installer's review for now." // The server's result covers where the battery goes, not the panel itself. - static let panelReview = "Your electrical panel still needs an electrician's review. This scan only covers where the battery can go." + static let panelReview = "Your electrical panel still needs an electrician's review. This scan only suggests where the battery could go." /// Which rules answered, for a reviewer: "Rules 2f52ec35". static func rulesHash(_ hash: String) -> String { diff --git a/ios/HouseScan/UI/Preview/DemoEngine.swift b/ios/HouseScan/UI/Preview/DemoEngine.swift index 506d3dae4..268defc29 100644 --- a/ios/HouseScan/UI/Preview/DemoEngine.swift +++ b/ios/HouseScan/UI/Preview/DemoEngine.swift @@ -1212,7 +1212,7 @@ final class DemoEngine: ScanActions { /// The offline sample result. Placeholder rules, so the decision is manual review. static let reviewSample = ResultPresentation( decision: .manualReview, - summary: "A spot 3 ft right of your meter fits, but it's close to a window, so it needs an installer's review.", + summary: "The best spot, 3 ft right of your meter, is close to a window, so it needs an installer's review.", policyApproved: false, spot: BatterySpot(span: 0.55...1.34, depth: 0.56, height: 1.1, offsetFromWall: 0.03), cableRoute: [SIMD2(0.16, 1.45), SIMD2(0.95, 1.45), SIMD2(0.95, 1.1)], diff --git a/ios/HouseScan/UI/Screens/ResultScreen.swift b/ios/HouseScan/UI/Screens/ResultScreen.swift index e4234a39b..e5b3e7ad1 100644 --- a/ios/HouseScan/UI/Screens/ResultScreen.swift +++ b/ios/HouseScan/UI/Screens/ResultScreen.swift @@ -162,10 +162,12 @@ struct ResultScreen: View { private func details(_ result: ResultPresentation) -> some View { DisclosureGroup(isExpanded: $detailsExpanded) { VStack(alignment: .leading, spacing: 22) { - // A possible spot's summary is the server's own pass ("fits every check"), which - // the card's candidate note contradicts on purpose; the summary of any other answer - // explains what failed or is unsure, so it stays. - if !result.summary.isEmpty, result.answer != .candidate { + // A pass's summary, and a possible spot's under manual review, is the server's own + // pass ("fits every check"), which the card's candidate note contradicts on + // purpose; a pass the reading sends to an installer contradicts itself and says the + // same. Manual review and reject summaries explain what failed or is unsure, so + // they stay. + if !result.summary.isEmpty, result.decision != .pass, result.answer != .candidate { Text(result.summary) .font(Typeface.hint) .foregroundStyle(Palette.muted) diff --git a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift index 757c61bf1..3f6877753 100644 --- a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift +++ b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift @@ -90,6 +90,28 @@ final class ProvisionalPlacementUITests: XCTestCase { try checkPassFixture(textSize: Self.largestText, name: "result-candidate-AX5") } + /// A result that isn't a possible spot keeps its summary in Details, and the summary never + /// claims a fit: the demo's review answer and its working-space overlap. + @MainActor + func testAReviewSummaryNeverClaimsAFit() throws { + for extra in [[], ["-uiDemoOverlap"]] { + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "result"] + extra + app.launch() + let any = app.descendants(matching: .any) + XCTAssertTrue(any["screen.result"].waitForExistence(timeout: 15)) + XCTAssertNotEqual(any["result.headline"].label, Self.candidate, "\(extra)") + let details = any["result.details"] + scroll(to: details, in: app) + details.tap() + let summary = any["result.summary"] + XCTAssertTrue(summary.waitForExistence(timeout: 10), "\(extra): a review answer's summary is missing") + assertNoFitWords(summary.label) + XCTAssertFalse(summary.label.lowercased().contains("fits"), summary.label) + app.terminate() + } + } + /// A rejected wall keeps its negative answer and its reasons. @MainActor func testARejectStaysNegative() throws { From 12cf77fc6b6aa117cc3fbed99cea9fcde30a844f Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 18:35:43 -0500 Subject: [PATCH 5/9] Capture the real candidate's AR instruction at the largest text size The prep-for-prod review found the AX5 check ran only on a demo answer, which is a sample with a different AR instruction, while the real candidate's longer instruction ran only at the default size. The settled engine test now runs at the largest text size, opens the AR view, checks its instruction names a possible spot and what the fit still needs, and that Done stays reachable, with screenshots of the card, its note and the AR view. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../ProvisionalPlacementUITests.swift | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift index 3f6877753..1e9403581 100644 --- a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift +++ b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift @@ -58,14 +58,17 @@ final class ProvisionalPlacementUITests: XCTestCase { /// "Something's there" withdraws the area and sends the scan again; the passing answer that /// comes back names the same spot, so the earlier answer settles it and the result shows /// without asking (`SpotConfirmations.settling`). That shortcut still shows a possible spot, - /// beside the notice that the homeowner said something stands there. + /// beside the notice that the homeowner said something stands there. At the largest text + /// size, so the AR view's longer instruction for a real candidate (placement, then what the + /// fit still needs) is captured where it is longest; a demo answer is a sample, whose AR + /// instruction differs. @MainActor func testASpotSettledByAnEarlierAnswerIsStillAPossibleSpot() throws { let gate = try Self.gate() defer { try? FileManager.default.removeItem(at: gate) } let server = GateServer(gate: gate) { body, _ in Self.passingAnswer(binding: body) } defer { server.stop() } - let app = Self.launchEngine(gate: gate, extra: ["-autopilotSomethingThere"]) + let app = Self.launchEngine(gate: gate, extra: ["-autopilotSomethingThere"] + Self.largestText) defer { app.terminate() } let any = app.descendants(matching: .any) @@ -74,7 +77,18 @@ final class ProvisionalPlacementUITests: XCTestCase { XCTAssertEqual(any["result.headline"].label, Self.candidate) XCTAssertTrue(any["result.spotRefused"].exists, "the settled answer's notice is missing") assertCandidateCard(app) - attach(app, name: "result-candidate-settled") + attach(app, name: "result-candidate-settled-AX5") + scroll(to: any["result.candidateNote"], in: app) + attach(app, name: "result-candidate-settled-AX5-note") + + try Data().write(to: gate.appending(path: "result")) + let instruction = any["instruction"] + XCTAssertTrue(any["screen.resultAR"].waitForExistence(timeout: 20)) + XCTAssertTrue(instruction.waitForExistence(timeout: 10)) + XCTAssertTrue(instruction.label.contains(Self.candidate), instruction.label) + XCTAssertTrue(instruction.label.contains("An installer needs to check the fit on site."), instruction.label) + XCTAssertTrue(app.buttons["action.closeAR"].isHittable, "Done is out of reach under the AR instruction") + attach(app, name: "resultAR-candidate-settled-AX5") } // MARK: Server answers in Fixtures/results From 8e9b07c9315c71dac90f15fe5817e7296bcebb9d Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 18:43:16 -0500 Subject: [PATCH 6/9] Leave unsure_cause out of the tests' passing answers The result schema's unsure_cause is an enum without null, so the bound all-pass answer in readingAPassLeavesTheAnswerAsTheServerSentIt failed validation (Actions run 37162046960, the run's only failure out of 714 package tests). Both that test and the UI test's passing answer now remove the key instead of setting it to null. Both answers validate against the schema with Python's jsonschema. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../Tests/HouseScanKitTests/ResultReadingTests.swift | 2 +- ios/HouseScanUITests/ProvisionalPlacementUITests.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/ResultReadingTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/ResultReadingTests.swift index 47cbc7729..d6d6b2496 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/ResultReadingTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/ResultReadingTests.swift @@ -322,7 +322,7 @@ import simd object["checks"] = try #require(object["checks"] as? [[String: Any]]).map { check in var check = check check["outcome"] = "pass" - check["unsure_cause"] = NSNull() + check.removeValue(forKey: "unsure_cause") return check } object["missing_evidence"] = [Any]() diff --git a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift index 1e9403581..5ed0e8fab 100644 --- a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift +++ b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift @@ -217,7 +217,7 @@ final class ProvisionalPlacementUITests: XCTestCase { object["checks"] = (object["checks"] as! [[String: Any]]).map { check in var check = check check["outcome"] = "pass" - check["unsure_cause"] = NSNull() + check.removeValue(forKey: "unsure_cause") return check } object["missing_evidence"] = [Any]() From c9295df2172cb0533c607f499bacb5a61467c6c1 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 20:00:26 -0500 Subject: [PATCH 7/9] Take the first match for each identifier in the provisional-placement UI tests Three demo-answer tests failed at the scroll helper with "Multiple matching elements found" (Actions run 37162669527): Details' DisclosureGroup puts its identifier, result.details, on more than one element, and reading a frame or tapping needs exactly one. Every lookup in the file now takes the first match, as ResultCardUITests' element helper does. The engine tests, the reject test and every other suite in that run passed. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../ProvisionalPlacementUITests.swift | 62 ++++++++++--------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift index 5ed0e8fab..f5ce8ea0a 100644 --- a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift +++ b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift @@ -37,17 +37,17 @@ final class ProvisionalPlacementUITests: XCTestCase { let tryAgain = app.buttons["action.retryUpload"] XCTAssertTrue(tryAgain.waitForExistence(timeout: 300), "the unbound answer never offered Try again") tryAgain.tap() - XCTAssertTrue(any["screen.result"].waitForExistence(timeout: 120), "the passing answer never reached the result") + XCTAssertTrue(any["screen.result"].firstMatch.waitForExistence(timeout: 120), "the passing answer never reached the result") XCTAssertEqual(server.requests.count, 2) - XCTAssertEqual(any["result.headline"].label, Self.candidate) - XCTAssertFalse(any["result.spotRefused"].exists) + XCTAssertEqual(any["result.headline"].firstMatch.label, Self.candidate) + XCTAssertFalse(any["result.spotRefused"].firstMatch.exists) assertCandidateCard(app) attach(app, name: "result-candidate-engine") // The autopilot opens the AR view once the result's gate opens. try Data().write(to: gate.appending(path: "result")) - let instruction = any["instruction"] - XCTAssertTrue(any["screen.resultAR"].waitForExistence(timeout: 20)) + let instruction = any["instruction"].firstMatch + XCTAssertTrue(any["screen.resultAR"].firstMatch.waitForExistence(timeout: 20)) XCTAssertTrue(instruction.waitForExistence(timeout: 10)) XCTAssertTrue(instruction.label.contains(Self.candidate), instruction.label) XCTAssertTrue(instruction.label.contains("An installer needs to check the fit on site."), instruction.label) @@ -72,18 +72,18 @@ final class ProvisionalPlacementUITests: XCTestCase { defer { app.terminate() } let any = app.descendants(matching: .any) - XCTAssertTrue(any["screen.result"].waitForExistence(timeout: 420), "the second passing answer never reached the result") + XCTAssertTrue(any["screen.result"].firstMatch.waitForExistence(timeout: 420), "the second passing answer never reached the result") XCTAssertEqual(server.requests.count, 2, "the spot check's answer should send the scan once more") - XCTAssertEqual(any["result.headline"].label, Self.candidate) - XCTAssertTrue(any["result.spotRefused"].exists, "the settled answer's notice is missing") + XCTAssertEqual(any["result.headline"].firstMatch.label, Self.candidate) + XCTAssertTrue(any["result.spotRefused"].firstMatch.exists, "the settled answer's notice is missing") assertCandidateCard(app) attach(app, name: "result-candidate-settled-AX5") - scroll(to: any["result.candidateNote"], in: app) + scroll(to: any["result.candidateNote"].firstMatch, in: app) attach(app, name: "result-candidate-settled-AX5-note") try Data().write(to: gate.appending(path: "result")) - let instruction = any["instruction"] - XCTAssertTrue(any["screen.resultAR"].waitForExistence(timeout: 20)) + let instruction = any["instruction"].firstMatch + XCTAssertTrue(any["screen.resultAR"].firstMatch.waitForExistence(timeout: 20)) XCTAssertTrue(instruction.waitForExistence(timeout: 10)) XCTAssertTrue(instruction.label.contains(Self.candidate), instruction.label) XCTAssertTrue(instruction.label.contains("An installer needs to check the fit on site."), instruction.label) @@ -113,12 +113,12 @@ final class ProvisionalPlacementUITests: XCTestCase { app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "result"] + extra app.launch() let any = app.descendants(matching: .any) - XCTAssertTrue(any["screen.result"].waitForExistence(timeout: 15)) - XCTAssertNotEqual(any["result.headline"].label, Self.candidate, "\(extra)") - let details = any["result.details"] + XCTAssertTrue(any["screen.result"].firstMatch.waitForExistence(timeout: 15)) + XCTAssertNotEqual(any["result.headline"].firstMatch.label, Self.candidate, "\(extra)") + let details = any["result.details"].firstMatch scroll(to: details, in: app) details.tap() - let summary = any["result.summary"] + let summary = any["result.summary"].firstMatch XCTAssertTrue(summary.waitForExistence(timeout: 10), "\(extra): a review answer's summary is missing") assertNoFitWords(summary.label) XCTAssertFalse(summary.label.lowercased().contains("fits"), summary.label) @@ -132,9 +132,9 @@ final class ProvisionalPlacementUITests: XCTestCase { let app = Self.launchFixture("reject-nearest", textSize: []) defer { app.terminate() } let any = app.descendants(matching: .any) - XCTAssertEqual(any["result.headline"].label, "Not on this wall") - XCTAssertFalse(any["result.candidateNote"].exists) - XCTAssertTrue(any["result.nearest"].label.hasPrefix("The closest spot")) + XCTAssertEqual(any["result.headline"].firstMatch.label, "Not on this wall") + XCTAssertFalse(any["result.candidateNote"].firstMatch.exists) + XCTAssertTrue(any["result.nearest"].firstMatch.label.hasPrefix("The closest spot")) attach(app, name: "result-reject") } @@ -143,7 +143,7 @@ final class ProvisionalPlacementUITests: XCTestCase { let app = Self.launchFixture("pass", textSize: textSize) defer { app.terminate() } let any = app.descendants(matching: .any) - XCTAssertEqual(any["result.headline"].label, Self.candidate) + XCTAssertEqual(any["result.headline"].firstMatch.label, Self.candidate) assertCandidateCard(app) let model = any.matching(NSPredicate(format: "label == '3D view of your wall'")).firstMatch XCTAssertTrue(model.waitForExistence(timeout: 10)) @@ -153,13 +153,13 @@ final class ProvisionalPlacementUITests: XCTestCase { // Details: the checks as calculations on the recorded scan, a pass without the server's // positive reason, and no server summary that says the spot fits. - let details = any["result.details"] + let details = any["result.details"].firstMatch scroll(to: details, in: app) details.tap() - let note = any["result.calculatedNote"] + let note = any["result.calculatedNote"].firstMatch XCTAssertTrue(note.waitForExistence(timeout: 10)) - XCTAssertFalse(any["result.summary"].exists, "a possible spot's summary is the server's pass") - let row = any["detail.check.gas_clearance"] + XCTAssertFalse(any["result.summary"].firstMatch.exists, "a possible spot's summary is the server's pass") + let row = any["detail.check.gas_clearance"].firstMatch XCTAssertTrue(row.exists) XCTAssertEqual(row.label, "Distance from gas equipment: Passes on recorded data", row.label) let rowValue = row.value as? String ?? "" @@ -169,12 +169,12 @@ final class ProvisionalPlacementUITests: XCTestCase { // The AR view draws the proposed spot. The demo's answer is a sample, so the AR title // says so (`ResultARScreen`); the drawing names the spot as proposed. - scroll(to: any["action.showAR"], in: app) + scroll(to: any["action.showAR"].firstMatch, in: app) app.buttons["action.showAR"].tap() - let overlay = any["ar.overlay"] + let overlay = any["ar.overlay"].firstMatch XCTAssertTrue(overlay.waitForExistence(timeout: 10)) XCTAssertEqual(overlay.label, "A proposed battery spot, drawn on your wall") - assertNoFitWords(any["instruction"].label) + assertNoFitWords(any["instruction"].firstMatch.label) attach(app, name: "\(name)-AR") } @@ -185,13 +185,13 @@ final class ProvisionalPlacementUITests: XCTestCase { @MainActor private func assertCandidateCard(_ app: XCUIApplication) { let any = app.descendants(matching: .any) - XCTAssertTrue(any["result.placement"].exists) - let note = any["result.candidateNote"] + XCTAssertTrue(any["result.placement"].firstMatch.exists) + let note = any["result.candidateNote"].firstMatch XCTAssertTrue(note.exists, "the candidate note is missing") XCTAssertTrue(note.label.contains("couldn't confirm all the space a battery needs"), note.label) let lines = any.matching(NSPredicate(format: "identifier BEGINSWITH 'check.'")) XCTAssertEqual(lines.count, 0, "a passing check reached the card") - XCTAssertFalse(any["result.installerConfirms"].exists) + XCTAssertFalse(any["result.installerConfirms"].firstMatch.exists) XCTAssertEqual(app.buttons["action.showAR"].label, "See this spot on your wall") // Details' scope note says, truthfully, that a pass is "not ... confirmed clear": the sweep // leaves that one qualified sentence out, whether or not Details is open. @@ -255,11 +255,13 @@ final class ProvisionalPlacementUITests: XCTestCase { app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "result", "-uiDemoResultFile", file] + textSize app.launch() - XCTAssertTrue(app.descendants(matching: .any)["screen.result"].waitForExistence(timeout: 15)) + XCTAssertTrue(app.descendants(matching: .any)["screen.result"].firstMatch.waitForExistence(timeout: 15)) return app } /// Scrolls until the element is inside the window, a third of the window at most per drag. + /// Lookups here take the first match: a DisclosureGroup's identifier ("result.details") + /// matches more than one element, and reading a frame needs exactly one. @MainActor private func scroll(to target: XCUIElement, in app: XCUIApplication) { let window = app.windows.firstMatch.frame From a7b4ec30068f41fe8b647a998e89080b56ec905a Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 20:03:29 -0500 Subject: [PATCH 8/9] Fold the AR card at the largest text sizes while the spot is in view At AX-XXXL the AR card held the title, the placement and the on-site caveat in full and covered nearly the whole camera (run 37162669527, resultAR-candidate-settled-AX5), so "See this spot on your wall" showed little of the spot. While the phone tracks and the result has a spot, the AR screen now sets CameraChrome's existing `aims`: at the accessibility sizes the card keeps its title ("A possible battery spot", or the sample's "Example spot, not your result") and folds the rest under Details, leaving at least the chrome's 180 pt camera window. "Point at your meter" while tracking is lost keeps every word in view. CameraChrome itself is unchanged. The settled engine test, at AX-XXXL, checks the folded card, measures the open camera between it and Done, opens Details to read the placement and caveat, folds it again, measures again, and returns with Done, with screenshots of each state. Co-Authored-By: Claude Opus 5.5 (1M context) --- ios/HouseScan/UI/Screens/ResultARScreen.swift | 13 +++++- .../ProvisionalPlacementUITests.swift | 44 +++++++++++++++++-- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/ios/HouseScan/UI/Screens/ResultARScreen.swift b/ios/HouseScan/UI/Screens/ResultARScreen.swift index 14bb00bbc..fa37ca3b5 100644 --- a/ios/HouseScan/UI/Screens/ResultARScreen.swift +++ b/ios/HouseScan/UI/Screens/ResultARScreen.swift @@ -41,7 +41,8 @@ struct ResultARScreen: View { CameraChrome( instruction: instruction, isReplay: state.isReplay, - isAutopilot: state.isAutopilot + isAutopilot: state.isAutopilot, + aims: showsSpot ) { Button("Done") { actions.closeAR() } .buttonStyle(.primary) @@ -53,6 +54,16 @@ struct ResultARScreen: View { } } + /// Whether the spot is on the wall in view: the phone tracks and the result has a spot. Then + /// the homeowner is looking at the spot, so at the accessibility text sizes the card folds + /// its detail (the placement, and what the fit still needs) under Details and leaves the + /// camera open (`CameraChrome.aims`). At AX-XXXL the full card covered nearly the whole + /// camera (run 37162669527, resultAR-candidate-settled-AX5). "Point at your meter" while + /// tracking is lost keeps every word in view: it is what to do, and nothing is drawn yet. + private var showsSpot: Bool { + state.tracking == .normal && state.result?.spot != nil + } + private static func overlayLabel(_ result: ResultPresentation) -> String { guard result.spot != nil else { return "The cable run and clearances, drawn on your wall" } return result.spotIsClean diff --git a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift index f5ce8ea0a..c5e72094d 100644 --- a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift +++ b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift @@ -81,14 +81,50 @@ final class ProvisionalPlacementUITests: XCTestCase { scroll(to: any["result.candidateNote"].firstMatch, in: app) attach(app, name: "result-candidate-settled-AX5-note") + // The AR view at AX-XXXL: the card folds to its title and Details, so the camera stays + // open on the spot; Details holds the placement and what the fit still needs. try Data().write(to: gate.appending(path: "result")) let instruction = any["instruction"].firstMatch + let details = any["instruction.details"].firstMatch + let detail = any["instruction.detail"].firstMatch + let done = app.buttons["action.closeAR"] XCTAssertTrue(any["screen.resultAR"].firstMatch.waitForExistence(timeout: 20)) XCTAssertTrue(instruction.waitForExistence(timeout: 10)) - XCTAssertTrue(instruction.label.contains(Self.candidate), instruction.label) - XCTAssertTrue(instruction.label.contains("An installer needs to check the fit on site."), instruction.label) - XCTAssertTrue(app.buttons["action.closeAR"].isHittable, "Done is out of reach under the AR instruction") - attach(app, name: "resultAR-candidate-settled-AX5") + XCTAssertTrue(details.waitForExistence(timeout: 10), "the AR card didn't fold at the largest text size") + XCTAssertEqual(instruction.label, Self.candidate) + XCTAssertFalse(detail.exists) + assertOpenCamera(below: details, above: done, in: app, "folded") + attach(app, name: "resultAR-candidate-settled-AX5-folded") + + details.tap() + XCTAssertTrue(detail.waitForExistence(timeout: 5), "Details didn't open the placement and caveat") + XCTAssertTrue(detail.label.contains("right of your meter"), detail.label) + XCTAssertTrue(detail.label.contains("An installer needs to check the fit on site."), detail.label) + assertNoFitWords(detail.label) + attach(app, name: "resultAR-candidate-settled-AX5-details") + + details.tap() + XCTAssertTrue(detail.waitForNonExistence(timeout: 5), "Details didn't close") + assertOpenCamera(below: details, above: done, in: app, "folded again") + attach(app, name: "resultAR-candidate-settled-AX5-refolded") + XCTAssertTrue(done.isHittable) + done.tap() + XCTAssertTrue(any["screen.result"].firstMatch.waitForExistence(timeout: 10), "Done didn't return to the result") + } + + /// The camera between the folded card and Done is at least the chrome's least open window + /// (`CameraChromeLayout.minCameraWindow`, 180 pt), measured from the elements' frames, not + /// inferred from Done being hittable. + @MainActor + private func assertOpenCamera(below card: XCUIElement, above done: XCUIElement, in app: XCUIApplication, _ state: String, + file: StaticString = #filePath, line: UInt = #line) { + let open = done.frame.minY - card.frame.maxY + XCTAssertGreaterThanOrEqual(open, 180, "\(state): \(open) pt of camera between the card (\(card.frame)) and Done (\(done.frame))", + file: file, line: line) + let note = XCTAttachment(string: "\(state): open camera \(open) pt, card bottom \(card.frame.maxY), Done top \(done.frame.minY), window \(app.windows.firstMatch.frame)") + note.name = "resultAR-open-camera-\(state)" + note.lifetime = .keepAlways + add(note) } // MARK: Server answers in Fixtures/results From 0fb7380ce4a9da5190aca1412719962f573d25b1 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 20:06:21 -0500 Subject: [PATCH 9/9] Measure the AR camera window from the card's bottom, clipped to the screen The open-camera check measured from the Details label, which leaves out the card's padding under it, and could count space off screen (Sol review of a7b4ec30). It now adds that padding, clips both edges to the window, and requires Done to be hittable before measuring. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../ProvisionalPlacementUITests.swift | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift index c5e72094d..5e84bef0c 100644 --- a/ios/HouseScanUITests/ProvisionalPlacementUITests.swift +++ b/ios/HouseScanUITests/ProvisionalPlacementUITests.swift @@ -112,16 +112,22 @@ final class ProvisionalPlacementUITests: XCTestCase { XCTAssertTrue(any["screen.result"].firstMatch.waitForExistence(timeout: 10), "Done didn't return to the result") } - /// The camera between the folded card and Done is at least the chrome's least open window - /// (`CameraChromeLayout.minCameraWindow`, 180 pt), measured from the elements' frames, not - /// inferred from Done being hittable. + /// The open camera between the folded card and Done, measured from the elements' frames and + /// clipped to the window, is at least the chrome's least open camera window + /// (`CameraChromeLayout.minCameraWindow`, 180 pt). The card's bottom is its Details label + /// plus the card's padding under it (8 pt, `InstructionCard.details`), so the area isn't + /// overstated; Done must be on screen and hittable first. @MainActor - private func assertOpenCamera(below card: XCUIElement, above done: XCUIElement, in app: XCUIApplication, _ state: String, + private func assertOpenCamera(below details: XCUIElement, above done: XCUIElement, in app: XCUIApplication, _ state: String, file: StaticString = #filePath, line: UInt = #line) { - let open = done.frame.minY - card.frame.maxY - XCTAssertGreaterThanOrEqual(open, 180, "\(state): \(open) pt of camera between the card (\(card.frame)) and Done (\(done.frame))", + XCTAssertTrue(done.isHittable, "\(state): Done isn't reachable", file: file, line: line) + let window = app.windows.firstMatch.frame + let top = max(details.frame.maxY + 8, window.minY) + let bottom = min(done.frame.minY, window.maxY) + let open = bottom - top + XCTAssertGreaterThanOrEqual(open, 180, "\(state): \(open) pt of camera between the card (bottom \(top)) and Done (top \(bottom))", file: file, line: line) - let note = XCTAttachment(string: "\(state): open camera \(open) pt, card bottom \(card.frame.maxY), Done top \(done.frame.minY), window \(app.windows.firstMatch.frame)") + let note = XCTAttachment(string: "\(state): open camera \(open) pt, card bottom \(top), Done top \(bottom), window \(window)") note.name = "resultAR-open-camera-\(state)" note.lifetime = .keepAlways add(note)