Skip to content

ios: confirm before a repeated Can't get there ends the scan; no spot for a wall never walked - #135

Merged
AidenJohnston merged 6 commits into
betafrom
t3/41-walk-exit
Sep 27, 2026
Merged

AidenJohnston merged 6 commits into
betafrom
t3/41-walk-exit

Conversation

@AidenJohnston

@AidenJohnston AidenJohnston commented Sep 27, 2026 •

Copy link
Copy Markdown
Collaborator

Base is beta, not t3/ios-mvf: the build 4.1 fixes are already integrated there, including #102's 0.6 s input lock on the card's reply and #109's gap loop and "Show my result". This branch builds on them for the next TestFlight build.

Fixes #82. Part of #76: this is option (a), app only. It does not catch field test run 3 (see below). The server part, option (b), comes after the deadline.

What changed

A second "Can't get there" asks before ending the scan (#82, option C)

On the walk, "Can't get there" ends the wall on the side being walked. In field test run 3, a homeowner trying to stop tapped it seven times; each tap silently skipped a task or set an end. Now, when "Can't get there" comes on a walk card within WalkRefusals.repeatWindow (5 s) of the one that last ended a side, the walk asks "End the scan here?" instead of ending this side too.

  • Yes, end here ends every side still without an end where "Can't get there" would put it (WalkedEnd, unexplored), then finishes the walk as "Done with this wall" does.
  • Start over takes the place of "Yes, end here" when ending the open sides now would leave ends closer than a battery's width (WalkRefusals.endsTooClose). finishWalk would refuse those ends and put the homeowner back on the walk, and two more taps would ask again, a loop with no way out. The card then reads "You haven't walked enough of the wall to check it yet. Start over, or keep walking along the wall." The engine works this out when it asks.
  • Keep walking dismisses the question. The next "Can't get there" ends its side without asking.

While the question is up, the card's reply, "Wall ends here" and "Mark something" are hidden, as they are for the end question. It doesn't ask while #136's "Is this the next wall?" is up. The first "Can't get there" behaves as before. The UI demo engine never asks on its own, so existing UI tests that tap the reply keep their behaviour.

A wall walked on neither side shows no spot (#76, option (a))

endWalkCannotGoOn now records, per side, whether "Can't get there" ended that side before the homeowner got WalkRefusals.walkedMinimum (a battery's width, WallFrame.minWallLength, 0.79 m) from the meter. A side counts as refused only when both the farthest kept view on that side and the end itself are closer to the meter than that. The farthest kept view counts every view the scan kept, including views kept during aim and tilt tasks. The record holds the end's s, so it stops counting once that end is cleared or moved (a past-end request that was met, a corner). When both ends are still refused ends, the result card says "We couldn't measure your wall" with a "Scan again" button. The engine drops the spot, the nearest spot, the cable route, the clearances and the checks and requests made at them. showAR refuses, so "See it on your wall" can't appear. The server and scene.json are unchanged, so another client still gets a spot.

What the rule covers, and what it doesn't. It fires only when both sides were ended by "Can't get there" within 0.79 m (2 ft 7 in) of the meter, on views and on ends. It fails open: "Wall ends here", marked ends and the automatic end prompt are never recorded, so a normal walk keeps its spot.

The issue's proposal says "record whether the walk was refused"; this reads "refused" as "ended before walking it", so a homeowner who walks to a fence on each side and taps "Can't get there" still gets a result.

Why

The team chose option C for #82 and option (a) for #76 on 2026-09-27. Option C adds no new control to the walk. Option (a) needs no contract change before the deadline.

UI changes

Three new states, all in the screen-state audit (wallWalk-endScanQuestion, wallWalk-endScanTooShort, result-wallNotMeasured); their screenshots are in the Mac mini run's result bundle. No before capture: all three are new.

  • Walk: the card reads "End the scan here?" / "We'll check the part of the wall you've walked.", with two full-width answers, "Yes, end here" and "Keep walking", where the end question's answers go.
  • Walk, too little walked: "End the scan here?" / "You haven't walked enough of the wall to check it yet. Start over, or keep walking along the wall.", with "Start over" and "Keep walking".
  • Result: the card reads "We couldn't measure your wall" / "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." with "Scan again".

Verification

Mac mini, ci-local.sh t3/41-walk-exit walk-exit at the head de5357f (beta at 6bc1df0 merged): xcodegen drift clean, swift test for HouseScanKit passed (525 tests in 76 suites), and the device build succeeded. UI tests: FAIL, with 4 failures inherited from beta: testPracticeMeterFromReplay, testAddSomethingFromTheReviewMarksOnTheCamera, testWholeFlowThroughTheButtons and testAimRingShowsProgressAndItsLegend. beta 6bc1df0 fails those same four, and t3/41-beta-green is fixing them. This branch's tests passed: testEndScanQuestionKeepsWalkingOrEnds, testWallNotMeasuredShowsNoSpot and testCantGetThereEndsWhereThePhoneIs. Rerun once t3/41-beta-green is on beta.

Earlier runs:

  • 73d931e (beta at c60e6ff, with fix(ios): end a side at the phone; confirm and back out of the next wall (#71, #70) #136, merged): PASS, 503 HouseScanKit tests, 26 UI tests with 1 skipped by design.
  • 057cfd4: attempt 4 PASS. Attempt 3 failed only testAddSomethingFromTheReviewMarksOnTheCamera, while the Mac's load average was about 178. That test passes on beta, and this branch doesn't change the review.
  • 6f20468: attempt 2 PASS. Attempt 1 failed only AuditIssueKeyTests.testAGoneElementIsReadWithoutRecordingAFailure (a "Gravel" tap that didn't register under load). beta passed that test.

New tests:

  • WalkRefusalsTests (HouseScanKit): the 5 s window and its edges, Keep walking, both sides ended near the meter (synthetic) versus one side, a side walked before the refusal, an end at the phone out along the wall (run 2's shape), run 3 not caught, an end that moved, and endsTooClose.
  • ScreenStatesUITests: the three new states pass the accessibility audit and show their words and controls; testEndScanQuestionKeepsWalkingOrEnds taps both answers; testWallNotMeasuredShowsNoSpot checks that "See it on your wall" and the placement lines are absent.
  • The autopilot's -autopilotCantGetThere run answers the question with "Keep walking" if it comes up, then ends the side as before.

Needs a device. The simulator can't drive the real engine's walk:

  1. Walk a few steps each way, then tap "Can't get there" twice within a few seconds. The second tap should ask "End the scan here?". "Keep walking" goes back to the walk; "Yes, end here" goes to the feature review.
  2. Standing by the meter without walking, tap "Can't get there" twice quickly. The question should offer "Start over" instead of "Yes, end here", and "Start over" should start a new scan.
  3. With each side walked less than 2 ft 7 in, and the phone staying that close on both sides, end both with "Can't get there" and go on to the result. It should say "We couldn't measure your wall" with no spot and no "See it on your wall".
  4. Walk a few steps each way before each "Can't get there". The result should show a spot as before.

Risk

Files touched:

  • ios/HouseScanKit/Sources/HouseScanKit/Guidance/WalkRefusals.swift (new) and ios/HouseScanKit/Tests/HouseScanKitTests/WalkRefusalsTests.swift (new)
  • ios/HouseScan/Contract/ScanContract.swift, ResultPresentation+Reading.swift
  • ios/HouseScan/Runtime/ScanEngine+Actions.swift, ScanEngine+Ends.swift, ScanEngine+Export.swift, ScanEngine.swift, Autopilot.swift
  • ios/HouseScan/UI/Screens/WallWalkScreen.swift, ResultScreen.swift, ios/HouseScan/UI/Copy/ScanCopy.swift, ios/HouseScan/UI/Preview/DemoEngine.swift
  • ios/HouseScanUITests/ScreenStatesUITests.swift

No project.yml or project.pbxproj change.

Checklist

  • The scope is focused; related changes belong together
  • I explained what changed and why
  • I included before/after images for visible changes: N/A, all three screens are new; screenshots are in the Mac mini result bundle, not attached here
  • I included a video for motion or interaction changes: N/A, no device run yet

🤖 Generated with Claude Code

… spot for a wall never walked

A second "Can't get there" on a walk card within 5 s of the one that last
ended a side now asks "End the scan here?" (Yes, end here / Keep walking)
instead of silently ending the other side (#82, option C). Yes ends every
open side where "Can't get there" would and finishes the walk; Keep
walking dismisses it.

The engine now records which ends "Can't get there" set before the
homeowner had walked a battery's width on that side (WalkRefusals). When
both ends are still those, the result says "We couldn't measure your
wall" with no spot, no route and no "See it on your wall" (#76, option
(a); app only, no schema change).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 0396a404-88d6-464d-8c1d-3d85ee27ae36

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the size:L 100-499 effective changed lines. label Sep 27, 2026
AidenJohnston and others added 2 commits September 27, 2026 03:11
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A side now counts as refused only when both the walk and the end that
"Can't get there" set stayed within a battery's width of the meter. Once
ends land at the phone (#71), run 2's right end sits at the wall's
physical end even with few views kept on the way, and its result must
keep the spot.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@SamGu-NRX

Copy link
Copy Markdown
Owner

@AidenJohnston Review at 057cfd4 found an acceptance gap in the no-walk exit and one smaller follow-up.

  • The advertised no-walk device case cannot reach the new result. answerEndScan in ScanEngine+Actions.swift:438-444 sets the remaining end then calls finishWalk. With no walk, WalkedEnd.end makes both ends zero; finishWalk:332-342 clears them and returns to walking. Repeating Yes, end here repeats that loop. The PR body documents the short-wall rule, but it conflicts with its device test asking to continue to the no-wall result without walking. Please let the confirmed exit reach an explicit no-measurement outcome, or reconcile the acceptance requirement. This blocks that demo scenario, not the ordinary walked-wall flow. fix(ios): end a side at the phone; confirm and back out of the next wall (#71, #70) #136 allowing a valid phone position beyond kept photos does not solve the stationary-at-meter case.
  • Smaller follow-up: WalkRefusals.wasRefused:54-56 identifies an end solely by coordinate, while setEnd/clearEnd never invalidate its refusal. Refuse ends at -0.3/+0.3, let the short-wall check clear them, walk farther left, then explicitly mark the left end again at -0.3 and refuse the right at +0.6. The 0.9 m wall can finish, but the stale left refusal still matches and hides the result despite the walk. Invalidate refusal ownership when an end is explicitly replaced; preserve it deliberately if a temporary past-end request restores the same refusal.

Tests currently inject frozen DemoEngine states, whose answerEndScan manufactures completed ends; they do not test this live transition. Exact-head hosted iOS36305435180 skipped, CodeRabbit skipped the draft, and no Codex verdict is present. The reported Mac mini results belong to earlier6f20468 and include one UI failure; full accessibility and current-head runtime acceptance remain unverified. Source review only here; no extra CI launched.

AidenJohnston and others added 2 commits September 27, 2026 03:35
Keeps both sides in the walk screen and engine: the end-scan question
sits beside #136's next-wall confirmation, and "Can't get there" doesn't
ask over it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Conflict repair only. Export keeps beta's `self` capture beside the #76
check. The result's Details keeps beta's comment and hides Start over for
a wall not measured, as before; "Scan again" takes beta's new Start over
hint, since starting over no longer deletes the scan.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@AidenJohnston AidenJohnston left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review of #135 at 88912e5 (the beta 6bc1df0 merge). The merge didn't change this PR's logic: WalkRefusals, endWalkCannotGoOn, cannotAccessArea and answerEndScan are the same as at 73d931e. Sources I checked against: run3-2055.md, run1-2000.md and run2-2015.md on t3/device-test-4.1-analysis, and issues #82 and #76.

Must fix

  1. The #76 rule doesn't catch run 3, the case it was chosen for. WalkRefusals.swift:43-47 counts a side as walked when max(walkedFarthest, |end s|) >= 0.79 m. ScanEngine+Ends.swift:123 gets walkedFarthest from CoverageMap.walkedPositions, which holds every kept view in the scan, including the ones kept during the tilt-down task at the meter. The public analysis has run 3's numbers (A41-2055-06):

    • The farthest kept views are at −6.7 ft (2.04 m), from the yard detour on the tilt card (p00039), and +6.0 ft (1.83 m).
    • The phone stood at about +5 ft (1.5 m), about 3 m out, when "Walk slowly to your right" was refused. That is inside phoneOutLimit (4 m), so under #136 the right end lands at the phone, +1.5 m.
    • Build 4.1's recorded ends were −4.005 m and +0.525 m.

    Every one of these is at or above 0.79 m, so both sides count as walked. neitherSideWalked returns false, and replaying run 3 on this branch still gives "4 ft 7 in left of your meter" and "See it on your wall", with the spot inside the house.

    The PR description says run 3 is "an end at the phone standing by the meter" and so is still refused. The analysis says otherwise. WalkRefusalsTests.swift:34 ("Run 3's shape") uses made-up values (ends −0.3 / +0.6, walked 0.2 / 0), not run 3's.

    Failure scenario: a user repeats run 3 (a bad estimated-plane tap, wandering during the first tilt task, then "Can't get there" on both walk cards). They still get a spot inside the house, plus the AR view.

    To unblock, do one of these:

    • (a) Base "walked" on a signal that separates run 3 from run 1. Run 3's walk cards were each answered 0.2–0.5 s after they appeared. Run 1 walked right while a tilt card was up (A41-1, 2:21–2:46), so "moved along the side during its walk task" on its own would hide run 1's spot. One candidate: a side ended by "Yes, end here", or by "Can't get there" on a walk card that had been up for less than about 2 s, counts as refused unless the phone moved at least 0.79 m along that side while that walk card was up. Add a WalkRefusalsTests case with run 3's real numbers, and keep testCantGetThereEndsWhereThePhoneIs (run 1) green.
    • (b) If there isn't time before the demo, correct the PR description and the test name, and state on #76 that this rule does not cover run 3. The team chose option (a) for run 3 and needs to know it isn't covered.

Should fix

  1. An unmeasured wall still runs the server's follow-up views before saying so. At ScanEngine.swift:1814-1818, automaticGapQueue(result) works from the raw placement, not the presentation that withWallNotMeasured cleared. Failure scenario (run 3 had this): the homeowner who just quit sees "4 more views to finish" and "Show the ground out to about 14 ft", and only after those gets "We couldn't measure your wall". Skip the automatic gaps when walkRefusals.neitherSideWalked(...) is true.

  2. "Yes, end here" with nothing walked is a loop with no way out. ScanEngine+Actions.swift:495-503 ends the open sides and then calls finishWalk(), which at :389-399 clears both ends as too close and puts the homeowner back on the walk. WallWalkScreen has no "Start over". Failure scenario (run 3's user, with a good anchor):

    1. They tap "Can't get there" twice, get the question, and tap "Yes".
    2. The card says the ends were too close.
    3. They tap "Can't get there" twice again (keepWalking() has reset the window, so the first tap just ends a side), get the question again, and loop.

    Before ending anything, compute whether the ends would be too close. If they would, have the question offer "Start over" instead of "Yes, end here".

  3. Correct the PR description's run facts. Run 1 did not end "both sides that way 16 ft out". The analysis says the left end was −1.0 ft and the right end 20 ft 3 in, so under this rule run 1's left side counts as refused and only the right side keeps its spot. Both run 1's and run 3's facts in the description are the record the team decided from.

Notes

  • Demo safety (question 1): today's rule fails open, so a normal demo keeps its spot. Only ends set by "Can't get there" are recorded. "Wall ends here", a marked end and the automatic end prompt never are. The spot is hidden only when both ends came from "Can't get there", each side's farthest kept view and end were under 0.79 m (2 ft 7 in), and the ends were still at least 0.79 m apart. That is a narrow window: each side 0.4–0.79 m long. For a practice-meter demo indoors, walk at least 3 ft on one side, or end sides with "Wall ends here".
  • 0.79 m: as a battery's width it's a fair floor for "walked". Because it's compared with the maximum over all kept views (at the 2 m stand-off, aim-task views a metre to the side are normal), it really measures "the phone was ever 2 ft 7 in to one side", so it will rarely fire. A meter close to a corner on one side is fine: one refused side keeps the spot (run 1's left). The spot is hidden only when both sides are short, a wall of 0.79–1.58 m between obstacles, which can hardly fit a battery beside the meter anyway.
  • Can #82 fire by accident? (question 3): No.
    • It asks only on .walk or .markEnd, within 5 s of a refusal that ended a side (:436).
    • Refusals on tilt, aim and overhead tasks neither ask nor start the window.
    • "Can't get there" is on the card at the top, and the answers are in the bottom controls. Run 3's 7 taps on one spot stop at tap 3, and the later taps land on nothing.
    • "Keep walking" only clears the flag and lastEnded. It sets no end, writes no log entry and leaves the walk task unresolved, as described.
    • go() clears the question on every phase change.
    • Minor: the two answers have no input lock of their own. That's fine while they sit away from the reply.
  • #136 (question 4):
    • The question is suppressed while "Is this the next wall?" is up.
    • "Yes" puts each open side's end where WalkedEnd.choose would: the phone's place on its own side, or the farthest kept view across the meter.
    • A corner turn clears the end, so the refused record stops matching.
    • wasRefused compares Floats exactly (WalkRefusals.swift:57). That holds today: setEnd stores s as it is, refineGround's updateWall shifts it by exactly 0, and anchor corrections keep s. If a future change moves ends, the rule fails open.
  • Beta's spot check: presentAnswer (ScanEngine+Confirm.swift:62) reads state.result.spot, which the presentation sets to nil. So an unmeasured wall skips the spot check and never shows the spot photo. Good. ResultScene3D draws no battery either, since both spot and nearestSpot are nil.
  • Export: scene.json, the scan stamp and "Share scan" still carry the server's spot. That's intended until option (b).
  • Public-repo rules: no private material, prompt text or internal numbers in the diff or the description. 0.79 m is the existing public WallFrame.minWallLength.
  • Swift: there's no new concurrency. systemUptime is monotonic, and the since >= 0 guard is harmless. endWalkCannotGoOn reads walkedFarthest before setEnd, which is correct.

Verdict: blocked. Only the #76 claim blocks. The #82 half is ready, and the current #76 rule is demo-safe because it fails open. Must-fix 1 can be cleared by either (a) or (b).

🤖 Generated with Claude Code

…d; say run 3 isn't caught

With too little walked, "Yes, end here" put the homeowner back on the
walk: finishWalk refused the ends as too close, and the next two taps
asked again. When ending the open sides now would leave ends closer than
a battery is wide (WalkRefusals.endsTooClose), the question offers
"Start over" in place of "Yes, end here".

The #76 rule doesn't catch field test run 3: its kept views reached
about 2 m on both sides. The test that claimed run 3's shape now uses
honest synthetic numbers, and a new test records run 3's numbers as a
case the rule does not catch.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@SamGu-NRX

Copy link
Copy Markdown
Owner

@AidenJohnston At de5357f, the new Start over alternative addresses the stationary zero-length exit loop in source. It does not establish the earlier advertised no-walk-to-result path; please keep the PR body aligned with the disclosed run-3 limitation.

Two bounded follow-ups remain: the stale refusal coordinate survives clear/re-mark, as in5854202504; and ScanEngine.swift1814-1827 suppresses the displayed result but still queues raw server missingEvidence through automaticGapQueue1851-1863. Thus a wall classified not measured can ask for more views before showing that outcome. Bypass those automatic requests for the same no-measurement classification, with a regression that follows the live upload transition rather than only a frozen result screen. These are source findings, not a new claim that the ordinary walked-wall demo is blocked. No runtime acceptance claimed.

@AidenJohnston

Copy link
Copy Markdown
Collaborator Author

Review response at de5357f:

  1. Must-fix, run 3 (took option (b)). The An unwalked, tapped wall still gets a spot (here inside the house) with wall backing and cable route passing #76 rule stays as it is. The PR description now says it does not catch run 3: run 3's kept views reached about 2.0 m left and 1.8 m right of the meter. The old "Run 3's shape" test is now bothSidesEndedNearTheMeterMeansNeitherWasWalked, labelled synthetic. A new test, run3IsNotCaughtBecauseItsViewsReachedPastTheMinimum, records the limit with run 3's numbers. An unwalked, tapped wall still gets a spot (here inside the house) with wall backing and cable route passing #76 has a note: the app guard covers scans where both walks were refused near the meter, run 3's spot came from the bad meter tap (Meter tap on an estimated plane put the wall 1.1 m behind the real wall and 46° off, and nothing corrects it #69) that ios: ground needs a plausible meter height and never rises; estimated meter taps are checked and re-fitted to the detected wall #108 guards, and option (b) stays deferred.
  2. Should-fix 2, the server's follow-up views before "We couldn't measure your wall": not handled. It's listed under Risk.
  3. Should-fix 3, the "Yes, end here" loop: fixed. When ending the open sides would leave ends closer than a battery's width (WalkRefusals.endsTooClose, unit-tested), the question shows "Start over" in place of "Yes, end here", with its own card text. A new audited screen state, wallWalk-endScanTooShort, covers it.
  4. Run 1 facts: corrected. The left end was at −1.0 ft, so it counts as refused; the right end was at 20 ft 3 in, so it counts as walked; the spot stays.

Mac mini at de5357f: HouseScanKit passes (525 tests) and the device build succeeds. The UI tests fail only on the 4 tests beta 6bc1df0 itself fails (practice meter, add-something, whole flow, aim ring), which t3/41-beta-green is fixing. This branch's UI tests pass.

🤖 Generated with Claude Code

@AidenJohnston AidenJohnston left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up review of #135 at de5357f. I read git diff 88912e5 de5357f (9 files, +82/−11), the updated description, the reply comment and #76. This was a source review only; I ran nothing myself. The Mac mini numbers below are as reported.

1. Must-fix (run 3 claim): resolved, via option (b)

  • PR text: it now says plainly "It does not catch field test run 3" and gives run 3's real reach (−6.7 ft / +6.0 ft; ends −4.0 m / +0.5 m). Run 1 is corrected: its left end was at −1.0 ft and counts as refused, its right end counts as walked, and the spot stays. That also clears should-fix 4.
  • Tests: the made-up "Run 3's shape" test is renamed bothSidesEndedNearTheMeterMeansNeitherWasWalked and marked synthetic. The new run3IsNotCaughtBecauseItsViewsReachedPastTheMinimum feeds in run 3's numbers (walked 2.04 / 1.83 m, ends −4.0 / +0.525) and asserts !neitherSideWalked, which is what the code actually does. Using the build 4.1 right end (+0.525) instead of #136's +1.5 m doesn't change the outcome, because the walked distance alone is past 0.79 m.
  • #76: the scope note is posted. It says the rule does not catch run 3, that run 3's spot came from #69 (guarded by #108), and that (b) is deferred.

2. The "Yes, end here" loop: fixed, and there's no trap

  • cannotAccessArea now computes endScanTooShort with the same inputs that answerEndScan → endWalkCannotGoOn → finishWalk would use: map.leftEnd ?? walkedEnd(.left) and the same for the right. WalkRefusals.endsTooClose matches CoverageMap.endsTooClose (right − left < minWallLength). A missing end counts as too close, which is correct here: with no end, finishWalk's bothEndsMarked guard would return silently. WalkedEnd.choose never returns nil, so in practice that branch doesn't fire.
  • No trap: "Keep walking" is always shown next to it. If the homeowner keeps walking and then finishes with ends that are too close, finishWalk still clears the ends and shows the too-short card, as before. From there, two "Can't get there" taps within 5 s bring the question back with "Start over". So there's always a way out within two taps.
  • "Start over" resets cleanly: it calls the same startOver() → resetAll() that the result, uploading and unsupported screens use. That covers coverage, anchor, recorder, endKinds, walkRefusals = WalkRefusals() and wallTooShort, then go(.onboarding), which clears endScanQuestion and endScanTooShort. answerEndScan and go() also clear the new flag, and DemoEngine's reset clears both flags. This is the first time startOver is called from a live capture phase. breakWalkedPath is a no-op there because coverage is already nil, and findMeter sets the live mode back to idle, so I found nothing that breaks. The device check in step 2 of "Needs a device" will confirm it.

3. Demo safety: unchanged, a normal walk still gets its spot

The new code only changes which button the question shows, and only when the ends would land less than 0.79 m apart. In that case "Yes, end here" was already refused by finishWalk, so no scan that used to finish now can't. A demo that walks a few feet each way and ends with "Wall ends here" or "Done with this wall" never reaches the question. If it uses "Can't get there" after walking, the ends are ≥ 0.79 m apart, and the question offers "Yes, end here" as before. The #76 rule itself hasn't changed since the last review and still fails open.

Notes (non-blocking)

  • The question's choice is computed once, when it's asked. If the phone moves while the question is up, the button can be wrong in either direction:

    • Offered "Yes" but now too close: finishWalk refuses, clears the ends and shows the too-short card. That's the old one-time bounce, not a loop, because keepWalking() reset the window and the next ask recomputes.
    • Offered "Start over" when "Yes" would now work: "Keep walking" is right beside it.

    Recomputing in answerEndScan, or on each guidance update, would close this. It's not needed for the demo.

  • Stale doc comment: WalkRefusals.swift (header, around line 11) still says "device run 1 ended both sides that way 16 ft out". That's the run 1 fact the description just corrected, so please change it to match.

  • "Start over" is one tap and discards the scan in progress, with no confirm. That's acceptable, because it's offered only when the scan couldn't finish anyway, and it sits in the bottom controls away from the card's "Can't get there". The accessibility hint ("keeps your two most recent completed scans") is accurate.

  • Test coverage of the new path: endsTooCloseToFinish covers the helper, and the screen-state audit checks that action.endScanStartOver and action.keepWalking exist. No test taps "Start over" from the walk, and none drives the live cannotAccessArea → endScanTooShort transition, because the simulator can't. Step 2 of the description's device checklist covers this, so do run it on the phone.

  • Still open from Sam's comments and the earlier should-fix 2:

    • An unmeasured wall still gets the server's automatic follow-up views before it says "We couldn't measure your wall". This is listed under Risk.
    • The stale refusal coordinate after clear/re-mark is still unaddressed. It needs an exact Float match to fire.

    Neither affects a walked demo.

  • CI: the Mac mini run reports HouseScanKit at 525 tests passing and this branch's UI tests green. The only UI failures are the 4 inherited from beta 6bc1df0 (practice meter, add-something, whole flow, aim ring), which t3/41-beta-green is fixing. Rerun once that lands.

Verdict: ready-with-notes

🤖 Generated with Claude Code

@AidenJohnston
AidenJohnston marked this pull request as ready for review September 27, 2026 09:50
@codemetrics-ai

Copy link
Copy Markdown

Note

Review skipped

Codemetrics is installed on this repository, but it hasn't been added to your team yet.

To enable AI code reviews for this repo, go to your dashboard and add house-scanning-master to your team's repositories.

@AidenJohnston
AidenJohnston merged commit 5f3f4c0 into beta Sep 27, 2026
4 checks passed
AidenJohnston added a commit that referenced this pull request Sep 27, 2026
Merges beta 5f3f4c0 (#135). Review of #147:
- Removes the 1 s tracking debounce: a brief drop just before "Wall ends
  here" put run 2's end back at the meter (#71). The rule reads tracking
  at the tap, and WalkedEnd.maxPastEvidence bounds a jumped pose. Test:
  tapRightAfterABriefTrackingDropEndsAtThePhone.
- Choice.pastEvidence flags the 2 m bound apart from the kept-photo cap,
  and logEnd says which ("2 m past evidence", "cap" or "phone").
- The "you saw" flag is set and cleared wherever the leaves-out value is,
  in the engine and DemoEngine.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 effective changed lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants