ios: log meter-anchor corrections so marks can be checked against drift

[AidenJohnston]

Part of #73

Base: t3/41-ground-and-meter-tap. Stacked on #108 and must merge after it; when this goes onto beta, merge #108 first.

What changed

In build 4.1 (run 2), a window outline stayed where it was tapped while the meter pin followed ARKit's corrections to the meter's anchor, and the outline ended up on the panel boxes. In that build refreshMeterFromAnchor copied only the anchor's translation into the wall, and the marks kept their world points.

That part is already fixed on t3/ios-mvf. Commit 37729cc (the #10 review's anchor-turn fix, merged after 4.1 was built) applies each anchor correction, turn included, to the wall, the kept cameras and every mark's tapped points as one rigid move. A mark's position relative to the meter, and so its exported s, no longer changes when ARKit corrects its map. This PR adds what that fix left out:

• A drift log. MeterAnchorTracking now keeps the anchor's pose from when the meter was anchored and counts the corrections applied since. Each applied correction writes one RuntimeLog.capture line with how far that correction moved the meter and turned the wall, the total move (x, y, z) and turn since the tap, how many marks moved with it, and the phase. On a walk away and back, a line means ARKit corrected its map and the marks followed. No line, with the marks off their objects, means drift that ARKit never corrected, which an anchor can't fix. The rate is already limited: a correction applies only once the anchor has moved 2 cm or turned 0.4 degrees since the last one.
• Resets clear the tracking. resetSpatialState and resetAll now set meterTracking to nil alongside the anchor id, so no tap-time pose survives into a new world frame. The next meter tap sets it again (setMeterAnchor). Replays have no anchor, so their marks keep their world points as before.
• Tests (AnchoredMarksTests). The anchor moves (0.2, 0, 0.1) m and turns 3 degrees about gravity. A window's two corners and a fence tap keep their place in the anchor's frame, and their s, height and distance out from the moved wall (what ScanEngine.project reads) match the tap. The test also checks that a point left at its tapped world position, as in 4.1, comes out about 20 cm off in s. A second test checks the totals after two corrections, with a sub-threshold nudge in between.

Why this differs from the brief

The brief asked for anchor-local points stored on MarkedFeature and rebuilt with anchor.transform * local, plus new AnchoredPoints helpers. Since 37729cc, refreshMeterFromAnchor does the same thing incrementally: it applies correction.point to each mark, and the correction is the anchor's move and turn about gravity. A second copy of each mark in anchor coordinates would be redundant state that could disagree with the first. So this PR leaves MarkedFeature, ScanContract.swift and ScanEngine+Actions.swift unchanged, and it doesn't conflict with #52's edits to feature/project.

The brief also said not to turn WallFrame.along/outward after the tap. 37729cc already turns them with the anchor, through WallFrame.apply, and CoverageMap.apply moves the kept cameras with them, so coverage stays consistent. This PR doesn't change that.

Verification

• Checked every caller of MeterAnchorTracking (the engine's setMeterAnchor and refreshMeterFromAnchor, and the tests in SpatialSourceTests.swift). The new stored property is set in the only initializer, so the synthesized Equatable still covers it.
• No Swift toolchain on the machine that wrote this, so nothing was compiled or run here. iOS CI has to build the app and run AnchoredMarksTests and the existing MeterAnchorCorrectionTests.
• No server files changed, so the server suite wasn't run.

Needs CI or a device

• CI: the build, the new HouseScanKit tests, and the log line's OSLog float formatting (format: .fixed(precision:) on Float).
• Device (U16): walk about 5 m from the meter and back after marking a window. Read the capture log's meter anchor corrected lines to tell a corrected map from drift, and check whether the outline stays on the window. That check decides whether AR overlays drift off their objects after walking away (window outline drawn over the panel) #73 can close.

Risk

• The log goes to the device's unified log, not the scan packet, so a field test has to capture it with Console or log collect. Putting it in the packet would be a contract change (unit U3).
• Whether the wall's ends should follow the anchor is still an open question. This PR leaves them as 37729cc has them.

Files touched

• ios/HouseScanKit/Sources/HouseScanKit/Geometry/MeterAnchorTracking.swift: anchoredPose, corrections, sinceAnchored.
• ios/HouseScan/Runtime/ScanEngine.swift: refreshMeterFromAnchor logs through the new logAnchorCorrection; resetSpatialState and resetAll clear meterTracking; reprojectFeatures doc comment.
• ios/HouseScanKit/Tests/HouseScanKitTests/AnchoredMarksTests.swift (new).

Dependencies

• Stacked on ios: ground needs a plausible meter height and never rises; estimated meter taps are checked and re-fitted to the detected wall #108 (t3/41-ground-and-meter-tap, unit U2), which changes ingest and the meter tap next to this code. Merge it first.
• No dependency on Mark an existing battery or another box on the wall (#27) #52: this PR doesn't touch feature or project.
• The device check belongs to unit U16.

Checklist

• The scope is focused; related changes belong together
• I explained what changed and why
• N/A before/after images: nothing on screen changes; the new output is a log line
• N/A video: no motion or interaction changes

🤖 Generated with Claude Code

[coderabbitai]

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: 3c33f083-0cb0-4a7f-988a-1f67e2c9a7a2

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[AidenJohnston]

Adversarial review of f141a62 (U10, Part of #73), base t3/41-ground-and-meter-tap.

I checked the PR's main premise and it holds. 37729cc is on t3/ios-mvf, beta and this base, but not in 45017ba (the 4.1 build). On this branch, refreshMeterFromAnchor already moves the wall, the kept cameras and every mark's points with each yaw-only correction. Each correction maps the last pose's origin onto the new one, so the applied corrections add up to exactly YawCorrection(from: anchoredPose, to: pose). Storing a second, anchor-local copy on MarkedFeature would only give the two copies a chance to disagree, so dropping that part of the brief is justified, and the body says so. What this PR adds is the diagnostic, and the diagnostic has two gaps that could lead the U16 device check to the wrong conclusion.

Must fix

1. The log level makes the "no line" test unreliable (ios/HouseScan/Runtime/ScanEngine.swift:478). The line goes out with RuntimeLog.capture.info. By default, unified logging keeps .info messages only in memory buffers and doesn't write them to the log store. The body's Risk section says a field test can pick the line up with log collect, but an archive collected after the walk may not contain it. Console.app also hides info messages unless Action > Include Info Messages is on. Either way, the tester sees no meter anchor corrected lines. The PR's own rule then reads that as "drift that ARKit never corrected", which is the wrong answer to the exact question #73 needs settled. Fix: log these lines at .notice (RuntimeLog.capture.notice, which is persisted), or at least fix the body and ios/README.md to say "live Console stream with Include Info Messages on" and drop log collect.

2. A missing line has several causes besides uncorrected drift (ScanEngine.swift:453, LiveCapture.swift:401). A line is written only if all of these hold: the frame has meterAnchor, frame.meterAnchorID == meterAnchorID, coverage != nil, and the correction is over 2 cm or 0.4 degrees. LiveCapture sets meterAnchor from frame.anchors.first { $0.identifier == id }, so if ARKit drops the anchor (the 4.1 analysis's hypothesis (a) for the AR result drawing nothing), there is no line, and the marks also stop following the anchor. On the device, that looks exactly like "no line, marks off their objects = uncorrected drift". Fix: add a line that is always written. For example, when the scan is sent or the result opens (or on each phase change after the meter tap), log the tracking totals and corrections, even when the count is 0, plus whether the last frame had the meter anchor. Also log one line when the anchor goes from present to missing in frames, and one when it comes back. The check then rests on lines that are there, not on a line being absent.

Should fix

1. The line can't be matched to the packet or the video (ScanEngine.swift:478). The 4.1 analyses lined events up using the manifest t (video time = t minus a constant). This line has only the unified log's wall-clock time. Adding the capture clock (frame.timestamp or captureClock) and the camera's distance from the meter lets U16 put each correction on the video timeline and see whether it came at the walk back.
2. No test runs the engine's mark move (AnchoredMarksTests.swift:42 vs ScanEngine.swift:458). The test applies correction.point to the points itself, the same one-liner the engine uses, so deleting the features loop in refreshMeterFromAnchor would still pass. The app target isn't reachable from HouseScanKit tests. Moving "move these points with the correction" into a HouseScanKit helper that the engine calls, or adding an app-target test, would make it a real regression guard. The math in the current test is right: the stale corner comes out at s ≈ 1.803 against 2.0, so > 0.05 passes with room to spare.
3. iOS CI hasn't run on this head. iOS build was skipped because the PR is a draft; the only checks were PR Size (pass) and CodeRabbit (skipped review). Before merging, run the iOS workflow on the branch with workflow_dispatch or mark the PR ready, so the build, AnchoredMarksTests and MeterAnchorCorrectionTests actually run.

Notes

• Mental compile, with nothing found: OSLogInterpolation takes Float with format: .fixed(precision:), and Int interpolates. self. is explicit on state.phase inside the autoclosure, matching line 1078. CoverageMap.wall is a non-optional WallFrame, so coverage.map { $0.wall.meter } type-checks. The new public let anchoredPose is set in the only init, and the synthesized Equatable and Sendable still hold. No code compares MeterAnchorTracking values. The test helpers (MeterAnchorCorrectionTests.turned/wallHitPose, standardWall, the nearlyEqual overloads, WallFrame.world(_:)/wallPoint(_:)) exist and are visible to the test target. I also traced correctionsSinceAnchoredAddUp by hand: the totals come out to (0.2, 0, 0.05) and 3 degrees.
• Clearing meterTracking in resetSpatialState and resetAll is safe. Every setMeterAnchor call site (markMeter, refitWallToDetectedPlane) sets it again. It also stops showResultInCamera from using a pose from the old world frame when a result survives a spatial reset.
• A re-fit (setMeterAnchor from refitWallToDetectedPlane) restarts the totals from the re-fitted pose. The doc comment says so. U16 should know this if a re-fit happened in the run.
• Rate: every applied correction writes a line, including on pose-only frames. If a non-LiDAR anchor jitters by more than 2 cm while ARKit optimises its map, that's up to one line per pose frame. That's acceptable for a diagnostic.
• Scope: three files. No project.yml or project.pbxproj changes, and nothing touches feature/project, markMeter, ingest, refineGround or showResultInCamera. Keeping WallFrame.along/outward turning with the anchor comes from 37729cc, and the body says so; this PR doesn't decide it.
• Public-repo check of the diff, body and commit message: no names, meter numbers or serials, addresses, emails, private content or media.
• The PR body has Part of #73, the base, the stacking on #108, CI-only versus device-only items, the files touched, the dependencies and the attribution line. "Part of" is right: whether #73 can close depends on the U16 walk.

Verdict: blocked (two small logging changes: the log level, and a line that is always written so a missing line isn't the evidence)

🤖 Generated with Claude Code

[AidenJohnston]

Fresh adversarial review of f141a62 (unit U10, Part of #73), stacked on #108.

I checked the premise first. The PR says the 4.1 build (beta 45017ba) moved only the meter, and that 37729cc, which already moves the wall, kept cameras and marks with the anchor's full yaw correction, landed after that build. Both are true: git merge-base --is-ancestor 37729cc 45017ba fails, and 45017ba's refreshMeterFromAnchor copies only the translation and then reprojects the fixed world points. So the placement half of the #73 hypothesis is already fixed on t3/ios-mvf. This PR adds the diagnostic the issue asked for ("log the anchor's transform over time and how far it has moved since the tap") and clears the stale tracking on resets.

Must fix

None found.

Should fix

1. The drift log may not reach the person running the U16 walk, and a missing line reads as the wrong answer. ios/HouseScan/Runtime/ScanEngine.swift:478 logs at .info. The unified log keeps info messages in memory only, and Console.app hides them unless Action > Include Info Messages is on. The PR body's decision rule is "no line, with the marks off their objects, means drift ARKit never corrected". Scenario: the tester follows ios/README.md (open Console, filter by subsystem), leaves info messages off, or collects the log after the buffer has turned over. They see no meter anchor corrected lines and conclude it was uncorrected VIO drift, when ARKit did correct the map. Fix: log this one line at .notice (persisted, shown by default), or add an explicit U16 step to turn on info messages and first confirm that the tap-time meter marked on ... line is visible. Then a missing correction line means something. The line holds only floats, ints and the phase enum, so .notice raises no privacy concern.

Notes

• Deviation from the brief is documented and sound. The brief asked for anchor-local copies on MarkedFeature plus AnchoredPoints helpers. The incremental approach is equivalent here. Each applied YawCorrection(from: pose, to: anchor) maps the old anchor frame onto the new one, sub-threshold nudges aren't dropped (pose isn't advanced until the threshold is reached), and under .gravity alignment the dropped tilt is noise. So the composed corrections keep every mark's anchor-local coordinates, which is what AnchoredMarksTests.swift:44-45 pins. A second anchor-local copy would be redundant state. This is not a silent design decision: the PR body explains it.
• Mental compile passes. MeterAnchorTracking keeps its explicit init(pose:), so no memberwise init appears and pose.map(MeterAnchorTracking.init) (ScanEngine.swift:1677) still resolves. The synthesized Equatable covers anchoredPose (simd_float4x4 is Equatable). The unlabeled tuple return at MeterAnchorTracking.swift:98 converts to (moved:yaw:). Every public member used across the module boundary is public: anchoredPose, corrections, sinceAnchored, YawCorrection. The private static func origin doesn't clash with the local origin in update(to:). The OSLog interpolations (Float with .fixed(precision:), Int, self.state.phase.rawValue with privacy: .public) match patterns already in the file (ScanEngine.swift:1072, :1078). The helpers the tests use all exist with those signatures: MeterAnchorCorrectionTests.turned(_:degrees:), wallHitPose(meter:outward:), standardWall(), WallFrame.world(_:)/wallPoint(_:), and the nearlyEqual overloads.
• I checked the test arithmetic by hand. On standardWall(), the stale point at s = 2.0 lands at s ~ 1.803 after the 3 degree turn and (0.2, 0, 0.1) move, about 20 cm off, so AnchoredMarksTests.swift:54-55 (> 0.05) holds. In correctionsSinceAnchoredAddUp, the 0.1 degree turn about the anchor's own origin has zero move and is below 0.4 degrees, so it returns nil. The 5 cm move then gives the expected (0.2, 0, 0.05). wallPoint clamping doesn't bite, because a single-piece wall's span is unbounded.
• The tests run on HouseScanKit, not the engine. marksKeepTheirPlaceRelativeToTheMeter repeats the engine's points.map(correction.point) and checks WallFrame.wallPoint, which is what ScanEngine.project reads (ScanEngine+Actions.swift:287). That's the best possible without an app-target test. It partly overlaps MeterAnchorCorrectionTests.rotationBeforeDisplay, but it adds the s, height and out checks and the 4.1 contrast.
• Rate limit. The brief said "rate-limited". The PR relies on the 2 cm / 0.4 degree threshold. Plain ARAnchors only move on map corrections, so volume should be low. A correction also runs coverage.apply and republishes, which costs far more than the log line. Acceptable.
• Totals reset on re-fit. setMeterAnchor from refitWallToDetectedPlane (ScanEngine.swift:430) starts a new MeterAnchorTracking, so "since anchored" means since the last re-fit. The doc comment at MeterAnchorTracking.swift:73-74 says so, and re-fit happens only during the close-up, before any mark exists.
• Resets. resetSpatialState and resetAll (ScanEngine.swift:1162, :1610) now clear meterTracking. Side effect: showResultInCamera (:1244) can no longer attach a result to a stale pose from a dropped world frame after a spatial reset. That's an improvement, and it doesn't touch U1's code.
• ScanEngine.swift:455: ?? 0 is dead after the coverage != nil guard. Harmless.
• Scope. 3 files, no project.yml/project.pbxproj edits, and markMeter, refineGround, feature, project and showResultInCamera are untouched. Trial merges (git merge-tree) against #100, #117, #109, #104, #102, #99, #52, #87, #56, #120 and #123 are all clean.
• Public-repo check. The diff, PR body and commit message contain no names, meter numbers or serials, addresses, emails, private material or media.
• PR body. Has Part of #73, base and stacking order (#108 first), what was verified vs what needs CI or a device, files touched, dependencies and the attribution line.
• CI. gh pr checks 119: PR Size passed, CodeRabbit skipped (draft), and iOS build skipped because the workflow skips drafts. No iOS run exists on this head (gh run list --branch t3/41-anchored-marks). Nothing here has been compiled. Before ready, dispatch the iOS workflow on this branch, or mark it ready, so the build and AnchoredMarksTests actually run.
• #73 stays open. Whether the 4.1 symptom was a corrected map or uncorrected drift is still for the U16 device walk, as the PR says (see Should fix 1).

Verdict: ready-with-notes

🤖 Generated with Claude Code

[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.

[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.

[SamGu-NRX]

@greptileai

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Adds logging and tracking for meter anchor corrections.

The PR appears safe to merge; the remaining issue is a non-blocking gap in regression coverage.

Findings

1.  Engine path remains untested ▶

Fix with agent prompt

### Issue 1
ios/HouseScanKit/Tests/HouseScanKitTests/AnchoredMarksTests.swift:42-45
This test moves the wall and points directly, without running `ScanEngine.refreshMeterFromAnchor`. It would still pass if the engine stopped updating stored feature points, so it does not catch the regression it is meant to prevent. An engine-level check would cover that path.

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 records per-correction and since-anchor movement in the capture log, clears anchor tracking during spatial resets, and adds geometry tests for marks moving with an anchor correction.

• The new tests verify the geometry but do not cover the engine path that updates stored marks.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Meter anchor pose update] --> B[Tracking threshold check]
  B -->|Correction accepted| C[Move coverage and stored feature points]
  C --> D[Publish wall and coverage]
  D --> E[Log step, totals, count, and phase]

Loading

_{Reviews (1) · Last reviewed commit: "Log each meter-anchor correction with it..."}

[greptile-apps]

Engine path remains untested

This test moves the wall and points directly, without running ScanEngine.refreshMeterFromAnchor. It would still pass if the engine stopped updating stored feature points, so it does not catch the regression it is meant to prevent. An engine-level check would cover that path.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ios/HouseScanKit/Tests/HouseScanKitTests/AnchoredMarksTests.swift
Line: 42-45

Comment:
**Engine path remains untested**

This test moves the wall and points directly, without running `ScanEngine.refreshMeterFromAnchor`. It would still pass if the engine stopped updating stored feature points, so it does not catch the regression it is meant to prevent. An engine-level check would cover that path.

---

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]

@codex review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-27T11:16:14.528500Z	c8136b1	Manual request

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: c8136b139d

ℹ️ 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".

[AidenJohnston]

Updated for review. Head is now c8136b1.

Base merge

f5d0ebf merges the updated #108 head (6a6a159), which now contains t3/ios-mvf's anchor repair. It conflicted in ScanEngine.swift and MeterAnchorTracking.swift, because refreshMeterFromAnchor is gone and SpatialUpdate now applies the correction.

• The correction line is now written in applySpatialUpdate, after the correction has moved the wall and the marks. The step is how far the meter moved.
• MeterAnchorTracking keeps both the base's correction log, used for poses captured earlier, and this PR's totals since the anchor was set.
• The two resets that clear meterTracking are kept.

Sam's #120 note (stored positions and anchor corrections)

I checked whether the same problem applies here. The drift totals compare raw anchor poses with each other (pose against anchoredPose), so they keep no position recorded in an old frame. The re-fit was the one place where it went wrong. It used to start a new tracker, which reset the totals but would also have dropped the base's correction log. Now anchorAgain resets only the totals, the correction count and the presence counts, and keeps the log. That way a re-fit's own move isn't logged as an ARKit correction, and the close-up photo keeps its corrections. Test: AnchoredMarksTests.anchoringAgainStartsTheTotalsButKeepsTheLog. The phone's position at the tap, which is a stored position, is moved with corrections in #108.

Review items

• Info-level lines don't persist (review 1, Must-fix 1; review 2, Should-fix 1): fixed in c8136b1. The lines are now .notice. This is the same change as beta's 1b3229a, which never reached this branch, redone on top of SpatialUpdate.
• A missing line was the evidence (review 1, Must-fix 2): fixed in c8136b1. The app now writes one line when frames stop carrying the meter's anchor and one when they carry it again (MeterAnchorPresence). It also writes a summary on every phase change after the meter is anchored, including when there were zero corrections, and always when the scan is sent and when the AR result opens. Test: anchorPresenceReportsOnlyChanges.
• The line can't be matched to the packet or the video (review 1, Should-fix 1): fixed in c8136b1. The lines now include t from the capture clock and the phone's distance from the meter.
• No test runs the engine's mark move (review 1, Should-fix 2; Greptile): not changed. The move happens in ScanEngine, and the app has no unit-test target. Testing it would mean adding one in project.yml, or moving MarkedFeature into HouseScanKit, and both are bigger than this PR. The tests still pin the geometry the engine relies on.
• CI hadn't run (review 1, Should-fix 3): it runs on this push; see below.
• The dead ?? 0 note from review 2 no longer applies, because the step is now worked out from the meter's position before the correction.

CI

The previous run (36305052363) failed with "screen.gapRequest never appeared" in testCantGetThereEndsWhereThePhoneIs, a known base flake. Base b90fb8e failed the same way in runs 36307478637 and 36307476187.

On c8136b1, GitHub run 36314535285 is green: the build, HouseScanKit, and the UI tests with the audit.

I also ran it on the Mac mini with ci-local.sh. HouseScanKit passed 454 tests, the xcodegen drift check passed, and the device build passed. In the UI tests, testCantGetThereEndsWhereThePhoneIs passed. The one failure was testSomethingThereChecksTheWallAgain, with "screen.wallWalk: Contrast failed" on action.cannotAccess. That is the known contrast flake, and it passed in the GitHub run.

Sam's review-clear is still needed. Nothing here has been run on a device: the drift lines need the U16 walk.

🤖 Generated with Claude Code