Repository navigation
Conversation
…drawn When no kept photo showed the spot, or there was no wall to draw on, the spot check still asked about "the blue outline", gave only the distance along the wall, and VoiceOver described the missing outline. The answer decides which clearance claims survive, so the homeowner has to know what space they are judging. One decision now drives the question, the picture and VoiceOver: the outline is drawn only with both a photo and a wall. Without one, the screen shows no photo, asks "Is anything standing in this space?", and lists the area's three extents: where it runs beside the meter, how far out from the wall (areaDepth) and how high up (spotHeight). The outlined photo's VoiceOver value now reads all three too. The words-only lines use the primary color, since Ink on Surface measured 1.06:1 in dark mode. An end of the span that rounds to 0 in now reads as the meter. Demo flags -uiDemoSpotNoPhoto and -uiDemoSpotNoWall reach those states; SpotConfirmUITests covers photo, no photo (default and AX5), no wall, left and straddling spans, and answering without a photo. Co-Authored-By: Claude Code <noreply@anthropic.com>
A -1 in end rounds to 1 in, not 0, so the example contradicted the rule. The extents comment now says it describes the outlined area only, not every clearance the server checks. Co-Authored-By: Claude Code <noreply@anthropic.com>
SamGu-NRX
added
the
full-ui
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
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 BaseScanning to your team's repositories. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (2)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (9)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe spot-confirm flow now selects its question and area presentation based on whether the area can be outlined. When an outline is unavailable, the screen describes the area with words. New demo cases and UI tests cover both presentation states. ChangesSpot confirmation
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant ScanEngine
participant SpotCheck
participant ScanCopy
participant SpotConfirmScreen
ScanEngine->>SpotCheck: Check whether photo and wall allow an outline
ScanEngine->>ScanCopy: Select question for outline state
ScanEngine->>SpotConfirmScreen: Present check and matching question
SpotConfirmScreen->>SpotCheck: Request outline using wall
SpotConfirmScreen->>ScanCopy: Display area description for outline state
Merge Risk: ⚪ Minimal · up to The fallback is ready for normal build and UI-test checks; those checks have not yet been confirmed as passing. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 5 files. (4 skipped: 4 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1bc23a0ee
ℹ️ 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".
| @@ -0,0 +1,203 @@ | |||
| { | |||
There was a problem hiding this comment.
Regenerate and commit HouseScan.xcodeproj for these two new fixture files. The HouseScanUITests synchronized folder excludes Fixtures, and the generated project.pbxproj enumerates each excluded result file, but this commit adds spot-left.json and spot-straddle.json without corresponding membership exceptions. In the inspected .github/workflows/ios.yml pipeline, XcodeGen regenerates the project and the drift step will therefore fail before tests run.
AGENTS.md reference: AGENTS.md:L58-L58
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, and it was the cause of hosted run 37107514444. Fixed in c1b2566 with the two exclusion lines XcodeGen 2.46.0 generated in that run. The resulting project.pbxproj blob is 21c9c0a, matching CI's diff.
| private func content(_ check: SpotCheck) -> some View { | ||
| ScrollView { | ||
| let outline = outlinable(check) | ||
| let question = outline == nil ? ScanCopy.spotQuestionInPerson : ScanCopy.spotQuestion |
There was a problem hiding this comment.
When the photo or wall is missing, this branch displays spotQuestionInPerson, but spotCheckRequest in Runtime/ScanEngine+Confirm.swift still always serializes ScanCopy.spotQuestion. Consequently, the saved packet's guidance log says the homeowner answered a question about a marked area and blue outline even though the new screen showed neither, making the audit record disagree with the interaction it is meant to bind to the answer.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same finding as Greptile's thread on this line. Fixed in 659f668: the logged question is now the one shown (spotQuestionShown(outlined:) from SpotCheck.outline(on:)), built after the photo loads.
The synced HouseScanUITests folder lists each excluded fixture by name, so XcodeGen 2.46.0 adds spot-left.json and spot-straddle.json. These are the exact lines CI generated in run 37107514444; the resulting blob is 21c9c0a. Co-Authored-By: Claude Code <noreply@anthropic.com>
The guidance log always recorded the outline question, and built the request before the photo loaded, so a check asked in words was logged as "inside the blue outline", and a photo that failed to load was logged as shown. The request and pending record are now built after the photo loads, from SpotCheck.outline(on:), the same decision the screen uses: the question shown, and a photo ID only when that photo is outlined. Each of the words-only card's three lines is now its own accessibility element with a spoken label, so VoiceOver steps through them and the UI tests and audit see the rendered text. The tests check each line's label and that it is on screen, scrolling to it at AX5. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…line fully on screen CI run 37108821815 failed SpotConfirmUITests in two ways. The "Where to look" title read as "Show": the identifier on Label(_:systemImage:) reached the "eye" symbol, whose VoiceOver name is "Show". The symbol is now hidden and the label combined, so the title is one header element read as its words. At AX5 the area lines failed the whole-line visibility check: the test scrolled until isHittable, which needs only the element's middle on screen, and a 249 pt line stopped with its bottom off the window. The test now drags by the part off screen until the whole frame is inside the window, the condition it asserts. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The reachability and result-notice tests now use #204's whole-frame scrolling instead of isHittable, so they scroll to the condition they assert. A notice taller than the window is read top edge first, then bottom edge, as a homeowner would scroll through it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
… branch: describe the spot-check area when no photo is available Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
This branch has not been deployed
1 participant
What changed
When no usable photo is available, the spot check currently asks the homeowner to inspect a blue outline that is not on screen. It gives only the distance along the wall, and VoiceOver also describes the missing outline.
The fallback now asks "Is anything standing in this space?" and shows a "Where to look" card with the area's position beside the meter, distance out from the wall and height above the ground. Those dimensions match the existing photo outline. VoiceOver reads each of the card's three lines as its own element, with units spelled out. The photo-backed layout stays unchanged, while its spoken description gains the depth and height.
The packet's guidance log now records the question the screen actually showed. It names a keyframe only when that photo was shown with its outline. Before, it always logged the outline question, and it logged a photo that failed to load as shown.
ScanEngine.presentAnswernow builds that record after the photo loads, using the same photo-and-wall check as the screen.Stacked on #191 at
75f6c377. This PR changes no clearance values or confirmation consequences.Why
The homeowner needs to locate the area before answering. One photo-and-wall availability check now controls the question, rendering and accessibility description, so they cannot disagree about whether an outline exists. Text in the fallback also uses the adaptive primary color instead of near-black text on a dark card.
UI changes
The missing-photo fallback is the changed screen. Before-and-after captures are not available yet: local build admission was refused for memory pressure. The new UI tests attach photo, no-photo, largest-text, left-of-meter and straddling-meter frames when they run. These will be inspected and added after hosted CI produces them.
Verification
e1bc23a0stopped at the project-drift check before building, because the two new fixtures were missing fromproject.pbxproj.c1b2566cadds the two lines that run generated. Hosted CI on659f668cis the first build and runtime check.Risk
This describes only the area the existing outline covers, not every region supporting the server's clearance checks. It does not resolve camera-only visibility limitations. The guidance log change alters packet log text only; scene.json and the packet's fields are unchanged. Phone usability and dark-mode rendering remain unverified.
Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit