Repository navigation
Ship beta to main: the app, server and experiments as submitted - #179
Merged
Merged
Conversation
- Ground entries stop 0.002 ft short of each corner as the server computes it from the written walls, not the phone's corner, so rounding can't let one cross and bring back the exterior sector. - Each sighting records whether depth confirmed it; when depth shows a row hidden, the unconfirmed ones go. Rebuilds replay kept cameras in capture order (segment, then frame time). - While the ground is a guess, a seen wall height also needs foot rows down to the guess less its error, so a guess too high can't hide the real foot. The strip's covered cells are unchanged. - The pending overhead view keeps the walked-path segment it was captured in. - The rounding-allowance comments say exactly what they absorb and allow. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The marking prompt and the mark and remove labels lowercased the feature name, so they read "Tap the ac unit" and "Mark ac unit". ScanCopy.noun keeps "AC" in capitals. The result's measurement lines and the 3D view's summary now give VoiceOver feet and inches spelled out (Distance.spoken), since it reads "ft" and "in" as letters. The on-screen text is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The upload delegate held progress at 0.99 until the server answered, and the server works out the spot inside that request. So the screen sat on "Sending measurements, 99%" for the whole analysis and "Check clearances" never lit up. Report 1 once every byte of the body is sent. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
In field test run 1 "Slow down" showed twice while the tester stood still with "What do you see?" open: turning the phone to aim reads as moving to the capture gate. Hide that one prompt while the tray is open; tracking problems still show. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The ternary between two static methods made the compiler fail with "failed to produce diagnostic for expression" on CI. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
"Measured 3 ft. The rule is 20 ft" read the cable run's maximum as a minimum. The server sends each check's comparison (at_least / at_most); the result row now carries it and the measurement line reads "The rule is at least 3 ft" or "at most 20 ft". A check without a comparison keeps the bare distance. The wording lives in the app (ScanCopy), not HouseScanKit, so the check is a UI-test assertion on the demo result's window row; the kit already tests decoding `comparison` (PlacementResultTests). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…eter first Device run 1 set both ends within 4 in of the meter: "Can't get there" put an end at the unbroken covered reach, which was 0 because nothing had asked for the ground in front of the meter. The scan then left out the 19 ft the walk had photographed. - "Can't get there" while the walk asks to walk a side, and a new "Wall ends here" during the walk, put that side's end at the phone's place along the wall chain, kept within the stretch walked on that side (HouseScanKit WalkedEnd). With the phone across the meter, or its place lost, the end goes as far as the walk went on that side instead of to the meter. - The walk's first request is the ground in front of the meter. The end prompt follows the walked distance, not unbroken coverage (B-06). An aim task keeps its stretch while the lag window's middle drifts under its half-width. - The wall strip shows where an end would land (a dashed line, the stretch past it dimmed, and a line when that cuts off part of the walk), and says the ends in its VoiceOver value. The end prompt shows the reticle it marks at. - Past an unexplored end with nothing between the ends in view, photos are refused and the card says "You're past the end of the wall". - -autopilotCantGetThere replays run 1 on the synthetic wall; testCantGetThereEndsWhereThePhoneIs checks the ends and the exported scene. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The window-row assertion read "at least 3 ft" from the VoiceOver value. The B-16 fix (#34) spells lengths out there ("3 feet"), so the test would fail once both land. It now checks "The rule is at least 3". Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The first real-phone run's wall already had a battery, and the scene couldn't say so. `battery` is now an object type, held to clearances.battery_ft (3 ft; Base's help page and IRC 2021 R328), measured in plan like the AC clearance under the strict rule. The cable detours over it on its route, as over a box, since it stands flush against the wall. The check appears when the scene marks a battery (or the rules make its reach larger than the AC clearance's); otherwise it would repeat the AC check's coverage, so every earlier answer is unchanged apart from the rules hash. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…go to review (#35) - A server band request whose span reaches past a marked end has no capture request (GapPlanner.reachesPastEnd): the coverage map records nothing past an end, so its bar stayed at 0 % and "I can't get there" was the only way out. Ground past a limit end is still recorded, so a ground request there stays capturable. It goes to review, like a request beyond capture. - A past_end request that ends without its end marked again no longer leaves that side without an end. Skipped, the end it cleared comes back with its kind and mark time. Met, the end moves on to the far edge of the ground the request showed, still unexplored (GapPlanner.endAfterPastEnd), so the next past_end request asks for the 2 m after it instead of the ground at the meter. - ServerGapTests: aBandRequestPastAMarkedEndIsNotACaptureRequest, aMetPastEndRequestMovesItsEndOn. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Build 3.1 reached a spot and the AR view for the first time, but the spot overlaps the meter's working space and the wall export ran from 7 ft 6 in left of the meter to 6 in right. The scan was lost, so this comes from the recording and the code at ad91620. Findings are filed as #38 to #45 and as comments on #24 to #29, #31 and #35. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Cells seen before an end was set stay in the coverage map, and the lagging-band and see-behind tasks read them: after "Can't get there" put the right end 6 in from the meter on device run 2, the walk kept asking to tilt up around 7 ft right and to tilt down at 5 ft 3 in right. Past an end nothing is observed (coveredFraction) or skipped (markSkipped), so the task could never fill and "Can't get there" didn't clear it. laggingBand, hiddenNearCamera and the see-behind check now count only cells between the ends. GuidancePlannerTests.noCameraTaskPastAMarkedEnd. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Field test run 2's working-space line read "Measured 1 ft 3 in. The rule is 0 in" for a spot 1 ft 3 in inside the meter's working space: the server's measured_ft was -1.25 and Distance.feetAndInches drops the sign. A measurement below zero now reads "Overlaps by 1 ft 3 in", and a minimum of 0 reads "The rule is no overlap". Less than half an inch of overlap still reads "Measured 0 in". The app target has no unit-test bundle (adding one means regenerating the project with XcodeGen), so the check is a screen state: -uiDemoOverlap puts run 2's line in the sample result, and result-overlap asserts the row's VoiceOver value and runs the accessibility audit on it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
vendoredCopiesMatchServer compares the kit's copy with origin/t3/server, which gained the `battery` object type at 074bf1d (#27). Every iOS run from then on fails in HouseScanKit (run 36275541111 on this PR). The copy is byte for byte from 074bf1d, and its recorded hash and the provenance follow; the other two vendored files are unchanged there. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The object types gain battery (issue #27). The drift test compares with the server branch's head, so every iOS run failed until this copy moved. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Two bugs from the review of 3c7b83a: - "Wall ends here" during a tilt or step-back request offered the first side without an end, left first. After walking right with the left end unmarked, it ended the LEFT side at its farthest walked point, far from the phone, while its hint says "where you're standing". It now ends the side of the meter the phone is on (WalkedEnd.side, the same s along the chain WalkedEnd.end uses), falling back to the old choice when the phone's place is lost, it stands at the meter, or its side is already ended. The strip's dashed line and VoiceOver value follow, since both read the same EndPreview. - Ending both sides at the meter without walking still gave a wall of no length. finishWalk now refuses ends closer together than WallFrame.minWallLength (0.79 m, one Base Core battery's width), clears them so the walk goes on, and the card says the ends were too close. WalkedEndTests covers both: the side choice with the phone right of the meter and the left end open, and ends at the meter being too close. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
After the check's answer asks for several views, "I can't get there" on one of them used to stop the loop: the next upload showed the result, so "2 more views to finish" delivered only one view. Now it skips only the current view: the next answer raises the next item it lists, and the result shows once none is left. Each item is raised at most once per pass (GapPlanner.nextServerRequest); a past-end item is known by its side, since raising it clears the end and it comes back with a different request. A new "Show my result" button on the follow-up request stops early, and the skip button's VoiceOver hint says where it leads. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…s past (#35) aRequestSpanIsReadInTheServersFeet asked for ground 0.02 ft past an unexplored end to show it isn't met. Past the end it now has no capture request, so the test checks that, and reads the shortfall from a request planned without the end. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The server added the `battery` object type (issue #27) after this branch's last CI run, and vendoredCopiesMatchServer fails on every t3/ios-mvf run until the copy is refreshed. One enum value and its description; the app doesn't export batteries yet. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…uests Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # ios/HouseScanKit/Tests/HouseScanKitTests/JSONSchemaValidatorTests.swift
…-fixes Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # ios/HouseScan/UI/Copy/ScanCopy.swift # ios/HouseScanKit/Tests/HouseScanKitTests/JSONSchemaValidatorTests.swift
A ray nearly parallel to the wall met its line hundreds of meters away, so "Wall ends here" could put an end there and the strip's preview drew a tape that long. A hit farther along the wall from the camera than the coverage map's sighting distance is now refused. An object marked past an end that came before a corner named the wall round that corner, which the export leaves out of walls[]. It now names the nearest written wall, whose line the server continues past its end. Found by the caretaker's review of 75dea41. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # ios/HouseScan/UI/Screens/WallWalkScreen.swift
…he floor (#163) An AC unit stands on the ground in front of the wall, so a ray aimed at it meets the ground first and the wall's plane under the floor. The #140 check refused those taps. ObjectTap.place now falls back to the ground hit for an object that stands on the ground, when the wall hit is below the ground or there is none, with the fence tap's 8 m out bound and the wall-end reach check on the ground hit. Doors, windows and gas meters keep the wall-only rule. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…Can't-get-there flow asks for The synthetic replay saw the wall only to 6.5 ft: its walk looks 20 degrees down, and its three tilt-up views share one position, so the 7 ft row never got a second. The server's wall request (out_ft 6.500001) could not settle. - make-synthetic-replay.swift appends 20 frames (k00042-k00061) after the existing 41, which keep their numbers and bytes: a walk along the wall 22 degrees up, 0.5 m apart. Both fixtures regenerated; additions only. The project lists the new files among those kept out of the UI test bundle. - A wall request above the walk's band plays the tilt-up run, as an overhead request does (GapPlan.asksAboveTheWalk); the autopilot lets the whole run play before it ends the walk. - The held-back window is planned over the walk's frames, and for the Can't-get-there flow against the ends that flow sets (EndPolicy.walked), so its gap is one that flow asks for. testCantGetThereEndsWhereThePhoneIs sometimes never reached the gap screen. - Tests: the first 41 frames still see 6.5 ft; all 61 see 7.5 ft over the request's span and settle it; the Can't-get-there held-back gap lies inside its ends. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> (cherry picked from commit 4461f13)
A detected vertical plane standing roughly parallel in front of the wall (FarSurface) now marks where the open space ends. With LiDAR depth, a reading nearer than a sample counts as the end of the space, not as something in front of the wall, when the sample lies past that surface or what the reading met is that surface. Such rows are neither seen nor hidden, so the walk no longer asks to look past a corridor's far wall. Anything nearer the wall than the surface still hides. The engine measures the surface from ARKit's planes whenever they, the wall or the stretch seen change, on phones with and without LiDAR. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A server walk-out needs the walk to pass its clearance plus the wall's position error. In a space narrower than that, the line lies behind the far wall or fence and no walk can meet it; on build 7.1 the card sat at 0 % while the homeowner walked against the far wall. GapPlanner.walkOutBlock now finds where the line lies past the detected far surface less the room a homeowner takes behind the phone. The card then says the space ends about X out and that "I can't get there" is the answer, the dotted line behind the wall goes, and the skip is logged with the reason. While a walk-out is up the card also gives the phone's distance out and the distance that counts there, rounded to 3 in; the reply and the guidance log use the request without that reading, so it doesn't relock the reply on every step. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Without LiDAR, a narrow space's far wall or fence reached the server only as the walked path less its position error, which could never show the facing gap. Where the walk came within a homeowner's depth of a surface ARKit's planes found facing the wall, the space is known clear up to it, so the scene now carries that surface as a `facing` entry with its error written out: the wall's position error plus a detected plane's default. The server then checks the real space, as it does the mesh's facing. Only when the mesh measured no facing gap: the mesh sees the gap itself and anything standing in it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The log message is an escaping autoclosure, which can't capture the inout request. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ized one Greptile noted the deflate attempt allocated a second buffer almost as large as each entry. The encoder now streams through 64 KiB, so the only added memory is the compressed output, and stops once that output stops being a saving. The write() and byte-identical doc comments say what they now mean, and the deflate tests are behind the same canImport(Compression) as the code. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…a guessed ground (#163) Most AC taps now land on the ground, so the bracket centred on the tap sat half under the floor. It now stands on the ground under the tap. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Only the build 7.1 packet shows the planes; the build 5.1 one showed the far wall in the mesh. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> (cherry picked from commit ee8cdcc)
ios: land an AC tap on the ground when its ray meets the wall under the floor
ios: bring the t3/ios-mvf UI test flake fixes to beta (reply-capsule contrast, audit the walk once the autopilot holds it, the held-back gap the Can't-get-there flow asks for).
…#164)" This reverts commit 64b9719. Held back for the demo. Replayed on the build 7.1 geometry, the far surface could be the corridor wall's own detected plane, where the chain piece runs about 11 degrees off it, and the export then sent a facing gap of under 3 ft that would fail the corridor's spots. It can come back once the own-plane rejection is proven on field data, and once it no longer reports a surface the walk went well past. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…164) Replayed on the build 7.1 corridor, FarSurface picked the corridor wall's own detected plane: the chain's piece runs about 11 degrees off it, so the plane passed the 0.5 m minimum two thirds of the way along and read as a far surface under 3 ft out. A candidate plane now has to stand at least minOut in front of every piece its outline's ends lie along, at both ends; a straight plane clear of a piece at both ends is clear of it everywhere. An end round a corner is held to no piece, so a corridor's far wall ending at the corridor's mouth still counts. The plane the meter's wall was refit to can be left out by id. Tests: a wall's own plane 10 degrees off a chained piece, crossing 0.5 m inside the requested span, is never chosen while the corridor's far wall is; a 7.6 m far wall 11 degrees off at 1.8 to 2.2 m counts and a 0.75 m plane doesn't. Also notes on the #160 path that a large flat obstacle can qualify, leaving what's behind it unseen rather than hidden. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ARKit refines its planes about ten times a second, so the headline's distance could move or come and go, re-arming the reply's lock each time. The engine now keeps where the space ends for the request once the card has shown it, and leaves out the plane the meter's wall was refit to when it measures the far surface. The walk-out and skip logs write their distances to the centimetre. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…164) Round-2 review of #168: holding each outline end to whichever piece's stretch it lay along still let the build 7.1 corridor wall's own plane through. Its near end ran past the corner, so it was judged against the meter's wall, 0.515 m in front of it. The chain's end pieces also run to infinity, which rejected a fence whose end lay along a side piece. Each end is now held to the piece nearest it in plan among those within maxAngle of parallel to the plane, measured against that piece's whole line, and the plane is rejected if either end is within minOut of it or behind it. Pieces turned further from the plane are ignored, as they are when a cell meets it, so a corridor's far wall is never held to the wall across its mouth. Tests: an own plane running 0.52 m past a convex corner is rejected; a fence parallel to the meter's wall that runs up to a side wall is kept. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ios: treat a corridor's far wall or a side yard's fence as where the space ends
|
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. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Important Review skippedToo many files! This PR contains 742 files, which is 642 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (154)
📒 Files selected for processing (742)
You can disable this status message by setting the 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. Comment |
This was referenced Sep 27, 2026
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+155
to
+164
| skip=() | ||
| if [ "$FULL_UI" != "true" ]; then | ||
| skip=(-skip-testing:HouseScanUITests/ScreenStatesUITests/testEveryStatePassesTheAccessibilityAudit) | ||
| echo "::notice::Skipped the every-state accessibility audit (about 10 min). Add the full-ui label to run it on this pull request." | ||
| fi | ||
| xcodebuild -project ios/HouseScan.xcodeproj -scheme HouseScan -configuration Debug \ | ||
| -destination "id=$SIM_UDID" \ | ||
| -resultBundlePath "$RUNNER_TEMP/HouseScanUITests.xcresult" \ | ||
| -only-testing:HouseScanUITests "${skip[@]}" \ | ||
| COMPILER_INDEX_STORE_ENABLE=NO test |
There was a problem hiding this comment.
The job runs all of HouseScanUITests and skips only the accessibility audit. That includes testPracticeMeterFromReplay, which the PR description identifies as red on beta. As submitted, an iOS change cannot get a green CI run until that test is fixed or excluded.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/ios.yml
Line: 155-164
Comment:
**Known-red test blocks iOS CI**
The job runs all of `HouseScanUITests` and skips only the accessibility audit. That includes `testPracticeMeterFromReplay`, which the PR description identifies as red on beta. As submitted, an iOS change cannot get a green CI run until that test is fixed or excluded.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+43
to
+45
| - run: uv run python -m meter_eval.leakcheck | ||
| env: | ||
| METER_HMAC_KEY: ${{ secrets.METER_HMAC_KEY }} |
There was a problem hiding this comment.
When a fork pull request changes the meter-closeup experiment, this job runs without access to the repository's METER_HMAC_KEY secret. The leak check then looks for a local key that is not provisioned on the runner and raises FileNotFoundError, so the check fails regardless of the contributor's changes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/meter-closeup.yml
Line: 43-45
Comment:
**Fork pull requests fail checks**
When a fork pull request changes the meter-closeup experiment, this job runs without access to the repository's `METER_HMAC_KEY` secret. The leak check then looks for a local key that is not provisioned on the runner and raises `FileNotFoundError`, so the check fails regardless of the contributor's changes.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| parts = got.split() | ||
| if len(parts) != 3 or parts[0] != "element" or parts[1] not in ("vertex", "face"): | ||
| return [f"lidar.mesh: expected an element line, got {got!r}"] | ||
| counts[parts[1]] = int(parts[2]) |
There was a problem hiding this comment.
A packet can have a valid manifest and a matching mesh hash while its PLY header contains element vertex abc. That line passes the header-shape check, but int(parts[2]) raises an uncaught ValueError. Instead of listing the invalid mesh in its report, packet validate crashes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packet/packet/validate.py
Line: 370
Comment:
**Malformed mesh crashes validation**
A packet can have a valid manifest and a matching mesh hash while its PLY header contains `element vertex abc`. That line passes the header-shape check, but `int(parts[2])` raises an uncaught `ValueError`. Instead of listing the invalid mesh in its report, `packet validate` crashes.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
This was referenced Oct 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings
mainup to what ships: everything onbeta(the TestFlight branch) lands onmain.beta(449 non-merge: the iOS capture app, the server, experiments, docs, CI).main's own 2 commits (MIT license, live-site link) stay; the merge has no conflicts.Merge with "Create a merge commit", not squash or rebase, so every commit keeps its author and date on
main.After this lands, the follow-ups are #22, #134 and the conflict-resolved versions of #18 and #90, each of which merges cleanly on top.
Known state:
testPracticeMeterFromReplayis red on beta (Sam's practice-meter work ont3/ios-mvf, not yet ported). It doesn't block TestFlight.🤖 Generated with Claude Code