[AidenJohnston]

The iOS workflow on t3/ios-mvf was red on intermittent UI test failures, so pull requests into it inherited a red check. Commits already on the base have fixed every one of them; the evidence is below. The current base no longer fails the tests this touches: 20 of 20 Mac mini runs passed without any change from here, and base push run 36318952517 on 6be087d passed with the full audit. So this PR is optional hardening of the full-flow audit, plus a triage record. It doesn't fix a live failure.

What it changes

• The walk is audited twice (FullFlowUITests):
  • In motion, report-only, when the walk first appears. Findings are attached as audit-wallWalk-moving and printed as AUDIT lines, as with HOUSESCAN_AUDIT_REPORT_ONLY. They don't fail the job.
  • Strictly, once the autopilot holds the walk. The app leaves <phase>.held in the gate folder while it waits on a gate, and the test waits for that file first. At that point both ends are set and "Done with this wall" is on offer.
• Why both. The in-motion audit found real bugs, and they only show mid-transition: the frozen every-state audit can't see them, and pull requests skip it anyway. But while the walk moves, two audit passes read two different moments. A view fading out has no identifier, so it is keyed by its label or by "no element", and two unrelated transient issues can match and fail as one. Keeping the in-motion audit visible but non-failing lets its reports be read without it deciding the job.
• The autopilot's gate waits four minutes, not two (4925f37). The strict audit and the walk's map check (up to 90 s) now both run after .held appears, so two minutes was too tight for a slow runner. Greptile raised this on ios: bring the t3/ios-mvf UI test flake fixes to beta #169.
• Docs: LaunchOptions and the README describe .held; the LiDAR test's comment no longer times the walk.

The failures and what fixed them

1. screen.wallWalk contrast / Dynamic Type in the full-flow tests: real bugs, fixed by Sam. My first diagnosis here was wrong. I blamed the audit's timing alone, but the in-motion audit was catching real mid-transition bugs:

• 1faa79b: the map's legend squeezed three entries into one row, drawn over each other, and a leaving entry faded over its replacement. This is the "(no element)" failure in run 36307476187. The commit also adds the frozen wallWalk-fullLegend state.
• b1a2444: "Can't get there" and "Can't see past it" were half-drawn over each other when the reply's title changed.
• 397b6d6: the reply's fill was 14% white over the camera. It is now solid.

The runs with an empty id or label (36305050676 'Mark something', 36304552823) and the Dynamic Type one (36302958452) came from the same moving walk. I didn't find a separate bug behind them.

2. testCantGetThereEndsWhereThePhoneIs: screen.gapRequest never appeared: fixed by 4461f13. Runs 36307478637, 36305052363, 36302961646, 36301217657 and 36296803483. The app went from the review straight to the upload, because the held-back gap wasn't one this flow asks for. 4461f13 plans the window against the ends this flow sets.

3. Failed to get matching snapshot: fixed by bc5a1ac. Six runs, all on commits without it.

4. Failed to terminate dev.housescanning.housescan: Simulator infrastructure, fixed by 20bf369. Three runs, all with a workflow that didn't boot the Simulator first. None since.

5. feature.ac not tappable: fixed by 3040033. The d4e008c runs.

6. testWholeFlowThroughTheButtons: missing meter.candidate.0: fixed by 1faa79b. The Simulator stalled the app's main thread for 14.6 s. 1faa79b raises the picker wait from 15 s to 30 s.

7. testWholeFlowThroughTheButtons: missing result.details: seen once on the Mac mini, not fixed. In 1 of 7 runs, the AR view's Done tap registered in XCTest but the app stayed on the AR view. I haven't found why, and it hasn't appeared in CI.

Testing

On the Mac mini (Xcode 26.6, iPhone 17 on iOS 26.5, the workflow's xcodebuild flags):

Build	Tests	Result
base d9abb8e, no change from here	testCantGetThereEndsWhereThePhoneIs x10, testFullFlowFromReplay x10 (-test-iterations 10, every iteration run)	20 of 20 passed
this branch at 32f6e71 (d9abb8e merged)	all of FullFlowUITests x3	12 of 12 passed; the in-motion audit ran 12 times and reported nothing
this branch at ad942d8 (older base)	testCantGetThereEndsWhereThePhoneIs x10, FullFlowUITests x4, testWholeFlowThroughTheButtons x7	10/10, 16/16, 6/7 (item 7)

Base push run 36318952517 (6be087d, with the every-state audit): passed. CI on this pull request's first head: run 36319754126 passed. 4925f37 changes only the gate's timeout and wasn't in the Mac mini builds.

My first A/B used base 29ebe84, which had neither b1a2444 nor 1faa79b. Its one failure there was the action.cannotAccess overlap that b1a2444 fixed, so it proved nothing about the gate.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests

  • Expanded scan-flow validation to include wall-walk scans, with checks both while movement is underway and after the screen settles.
  • Added report-only auditing so findings can be recorded during motion without failing the test; settled-screen checks remain strict.
  • Extended wait times for scan-flow synchronization.

• Documentation

  • Clarified how automated scan testing identifies when a screen is being held for inspection.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 809cbf29-1ec9-4c23-bc8b-51792200b044

📥 Commits

Reviewing files that changed from the base of the PR and between d9abb8e and 4925f37.

📒 Files selected for processing (4)

• ios/HouseScan/Runtime/LaunchOptions.swift
• ios/HouseScan/Runtime/ScanEngine.swift
• ios/HouseScanUITests/FullFlowUITests.swift
• ios/README.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The app now writes a held-phase marker while waiting for a phase gate file. UI tests use the marker to coordinate report-only audits of moving screens with the existing settled-screen audits.

Changes

Held-screen audits

Layer / File(s)	Summary
Gate marker and documentation ios/HouseScan/Runtime/ScanEngine.swift, ios/HouseScan/Runtime/LaunchOptions.swift, ios/README.md	waitForGate(_:) now waits up to 240 seconds and writes a <phase>.held marker when the phase gate file is absent. The launch-option and README descriptions document the marker.
UI audit coordination ios/HouseScanUITests/FullFlowUITests.swift	Selected phases receive a report-only audit while moving. The test then waits up to 150 seconds for the held marker before continuing with the settled-screen audit. Report-only findings are attached and printed instead of failing the test.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Other

Sequence Diagram(s)

sequenceDiagram
  participant FullFlowUITests
  participant ScanEngine
  participant GateFolder
  participant audit
  ScanEngine->>GateFolder: write phase.held while waiting for phase gate file
  FullFlowUITests->>audit: audit moving screen in report-only mode
  FullFlowUITests->>GateFolder: wait for phase.held
  GateFolder-->>FullFlowUITests: phase.held exists
  FullFlowUITests->>audit: audit settled screen

Loading

Suggested reviewers: samgu-nrx

Merge Risk: ⚪ Minimal · up to 4925f

The wall-walk screen is audited in motion and then strictly after the app signals it is held. No concrete merge-blocking issue remains.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 4925f

The coordination change remains explicitly enabled through launch options. The supported UI tests isolate each run in a fresh directory and retain a strict held-screen audit. No material security risk introduced or worsened by this PR was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The added filesystem effect is an empty, phase-named marker in the launch-configured directory, using the app's existing filesystem authority. Controlling that destination requires control over the relevant launch arguments; the inspected change does not establish a new network source, tenant boundary or credential-bearing sink.

Trust Boundaries and Controls

• observed — The test owns the gate directory and releases each phase only after processing it. UUID-specific directories isolate supported concurrent runs. Strict audit failure remains enabled unless the existing report-only environment setting is selected; the new report-only argument is used for the moving audit, not the held audit.

Resilience and Maintainability Implications

• observed — A missing wallWalk held marker triggers a test assertion after a bounded wait, and the test disables continuation after failure. The app also has a bounded gate deadline, although swallowed cancellation errors can keep the inherited wait active until that deadline. Neither mechanism is used as a security authorization control in the inspected flow.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: auditing the wall walk in motion and after it is held, with failure triage context.
Description check	✅ Passed	The description is detailed and covers the changes, rationale, verification results, known failure, and risk context. It uses equivalent headings for changes and testing, although it omits explicit Ri…
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 u…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

📝 Generate docstrings

• Commit to this branch
• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codemetrics-ai]

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.

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Adjusts UI test timing and adds a held-state signal file.

The PR appears safe to merge, though the moving audit should preserve single-pass findings to provide the intended transition coverage.

Findings

1.  Single-pass findings disappear ▶

Fix with agent prompt

### Issue 1
ios/HouseScanUITests/FullFlowUITests.swift:327
The new in-motion audit reports only findings seen in at least two passes, which are six seconds apart. A defect visible only during a brief walk transition can be gone by the second pass, leaving no `AUDIT` line or attachment for anyone to review. Record first-pass findings in this report-only audit so those transitions remain visible.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR adds a gate marker so the full-flow test can strictly audit the held wall walk while running a separate report-only audit during motion. It also extends the gate timeout and documents the marker. The moving audit can discard defects that appear in only one pass.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Walk appears] --> B[In-motion report-only audit]
  B --> C[Wait for wallWalk.held]
  C --> D[Strict held-walk audit]
  D --> E[Release wallWalk gate]

Loading

_{Reviews (1) · Last reviewed commit: "ios: the autopilot's gate waits four min..."}

[SamGu-NRX]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[SamGu-NRX]

@greptileai review

Please review the current head 4925f37fb70cb875b34b16fa03eff75218c1905f against this PR's actual base. Catch-up review requested by Sam after enabling BaseScanning in Greptile.

[greptile-apps]

Single-pass findings disappear The new in-motion audit reports only findings seen in at least two passes, which are six seconds apart. A defect visible only during a brief walk transition can be gone by the second pass, leaving no AUDIT line or attachment for anyone to review. Record first-pass findings in this report-only audit so those transitions remain visible.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ios/HouseScanUITests/FullFlowUITests.swift
Line: 327

Comment:
**Single-pass findings disappear** The new in-motion audit reports only findings seen in at least two passes, which are six seconds apart. A defect visible only during a brief walk transition can be gone by the second pass, leaving no `AUDIT` line or attachment for anyone to review. Record first-pass findings in this report-only audit so those transitions remain visible.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[SamGu-NRX]

Caretaker check at 4925f37: confirmed, but the filtering predates this PR and only diagnostic evidence is lost. The new moving audit runs the existing collector with reportOnly (FullFlowUITests.swift:327), and AccessibilityAudit.swift:118-131 drops findings seen fewer than twice. A defect that clears before the second pass, six seconds later, leaves no AUDIT line. Keeping two sightings as the failure rule is deliberate (lines 17-28). Report-only mode could still attach findings seen once. Non-blocking for this PR.

[greptile-apps]

That distinction makes sense. Keeping the two-sighting threshold for failures is deliberate, while retaining single-sighting findings only in reportOnly would improve diagnostic visibility without changing pass/fail behavior. Since the filtering predates this PR and the impact is limited to optional evidence, I agree this is non-blocking and can be tracked as follow-up hardening.