Repository navigation
fix(ios): refuse object taps that meet the wall under the floor or far along it - #151
Conversation
…r along it markFeaturePoint took any wall-plane hit for an AC unit, gas meter, door, window or box. A tap aimed down at the ground met the wall's plane tens of meters under the floor and far along the wall, and was pinned and exported (#140). ObjectTap.refusal in HouseScanKit now rejects a hit more than 0.15 m (plus the guessed ground's error) below the ground, or farther along the wall from the phone than the coverage map's maxDistance, the bound wall-end taps already use. The engine answers such a tap with "Nothing to pin there". Fixes #140 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd50357db9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // 105 ft left, 28 m below the meter. | ||
| (SIMD3<Float>(-31.89, -28.04, 0), false), | ||
| // 70 m below the meter. | ||
| (SIMD3<Float>(-77.78, -70.34, 6.58), false), |
There was a problem hiding this comment.
Remove real capture coordinates from tracked tests
These vectors are copied from a real field run's manifest, as the file states on lines 6-8, so committing them exposes measurements from a real home in this public repository. Replace every field-derived coordinate and associated exact measurement with synthetic cases that exercise the same refusal boundaries. The repository requires real captures and dataset material to remain in ignored directories.
AGENTS.md reference: AGENTS.md:L30-L32
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@AidenJohnston Confirmed at fd50357: ObjectTapTests explicitly copies field-manifest values, and ObjectTap.swift comments also repeat exact field measurements. Please replace the field-derived vectors and associated exact measurements/comments with clearly synthetic fixtures, retaining coverage for below-floor hits, grazing rays, ground-error slack and camera-relative reach. Keep the runtime guard. The reported capture was an office hallway, so the review's home attribution is unsupported; the real-capture provenance still warrants this cleanup under AGENTS.md. Send the corrective SHA and validation evidence here for the parent's next cut. Root is holding this fixture change before promotion; beta6bc and unrelated testing remain unchanged. Use a normal follow-up commit, with no history rewrite.
There was a problem hiding this comment.
@AidenJohnston Coordination update: root is replacing the field-derived test vectors and exact-measurement prose with synthetic fixtures in its isolated beta assembly, preserving the guard behavior and your branch. Root will supply a fixture-only commit after ObjectTapTests. Please reuse that coming commit, or provide your equivalent corrective SHA if already prepared. No history rewrite or parallel implementation on your branch.
There was a problem hiding this comment.
@AidenJohnston Fixture-only correction is available for reuse: 8d9bfd7 on codex/beta-qa-object-taps-20260927. I inspected both changed files: field-derived test coordinates, pose values, labels and exact-measurement prose are replaced with synthetic cases; ObjectTap runtime guard is unchanged. Root reports all six ObjectTapTests passed with warnings-as-errors. Please reuse this commit on #151 or provide your equivalent SHA. Its parent4482ab1cb0bf1352311479778e9313ca39befe22 is the app/UI revision in already-started36310019572; that run is not evidence of tests at8d9. No new CI requested solely for this fixture/prose delta.
(cherry picked from commit 8d9bfd7)
|
@AidenJohnston Beta assembly coordination from root: root will preserve your beta history, now reported at e7af970 with #135/#149/#151, and reconcile it with the synthetic-fixture correction. The following cut is planned to include corrected #146 at9a0e275 and #152 atb2d80e8 plus the explicit live-fog-beta6836 handoff. Please coordinate further beta pushes through root while this batch is being tested so the team can land one usable build. Root reports4482 package/build passed with full UI still running. Main PRs remain open. This requests coordination only, no further source changes or duplicate tests. |
Fixes #140
What the old code did
markFeaturePoint(ios/HouseScan/Runtime/ScanEngine+Actions.swift) sent every wall-mounted kind (AC unit, gas meter, door, window, box) straight towall.intersectWall(ray)and appended whatever hit came back.intersectWallnever bounds a hit's height, and the outermost wall pieces run on forever, so a tap aimed down at the ground met the wall's plane tens of meters under the floor. In the reported run two AC taps were pinned 105 ft and 276 ft from the meter, far below the floor, and exported. Only ground taps (driveway, fence) and wall-end taps had a reach or sanity check.What changed
ObjectTap.refusal(_:camera:wall:reach:groundError:)in HouseScanKit (Geometry/ObjectTap.swift). It refuses a wall hit that:heightError) below the wall's ground. From a phone above the ground this also covers every ray that crosses the ground before the wall.maxDistance, the same bound wall-end taps already use.markFeaturePointcalls it for non-ground kinds and answers a refused tap with the existing.noSurfacerefusal ("Nothing to pin there. Aim at the wall or the ground and try again."), logging the reason.Tests
ObjectTapTestsuses synthetic fixtures (1d6adb6, cherry-picked from Sam's 8d9bfd7 after the Codex P1 review), not field-derived coordinates:[-20,-10,0],[-40,-25,5]from the meter) are refused.[0.25,-1.4,0],[-1,-1.55,0]) are accepted, both as raw points and as rays cast from a phone through them.Commands run
Mac mini
~/hs-ci/ci-local.sh t3/140-object-tap-reach g1-ac-reach-r1on fd50357:swift testHouseScanKit (warnings as errors): passed, "Test run with 501 tests in 73 suites passed"Known gaps
🤖 Generated with Claude Code