Repository navigation
fix(ios): results say they need an installer's review, never that one is coming - #199
Conversation
…oming The app shows the server's answer and contacts nobody, yet the result and the scan's skip hints promised an installer's action: "An installer will confirm", "An installer will check this", "An installer confirms this on site", "an installer reviews every result for now", "The spot an installer will check" and "An installer will look at it instead". Nothing sends a scan to an installer or queues a review. The result now says what it needs: - The headline for a result a person has to settle reads "Needs an installer's review"; the other three answers are unchanged. - An unsure check that no photo can settle, and a "Still needed" view the phone can't capture, read "Needs an installer to check" (one constant). - The footnotes read "An installer has to confirm the spot on site before a battery goes in." and "The placement rules aren't final yet, so every result needs an installer's review for now." - The AR view says "The spot an installer needs to check". - Skip hints on the walk, the gap request, the close-up and the spot check say an installer "would need to" look, read or check instead. - The offline sample's summary says the spot "needs an installer's review". FullFlowUITests.testFullFlowFromReplay, which ends on the bundled sample (a manual review under unapproved rules), now asserts the rendered headline and both installer footnotes word for word. It skips them against a real server, whose answer can be any decision. The server's own summary and reasons aren't covered. Co-Authored-By: Claude Code <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 |
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. |
 |
![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: 8c47d6789d
ℹ️ 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".
…n Details (#199) "An installer has to confirm the spot on site before a battery goes in." shows on every result, including "Not on this wall" and "We couldn't measure your wall", which have no spot (Codex review on #199). It now reads "Before any battery goes in, an installer has to confirm where it goes on site." InstallerReviewUITests holds the demo's review sample still, opens Details and checks that the window check, which needs a person, reads "Needs an installer to check" while the ground check, which needs a photo, keeps "One more photo would settle this" (Greptile review on #199). XCUITest can't read accessibility hints, so the skip hints remain checked by review only. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The demo's review sample still offers a view to capture, so its headline is "One more look", not the installer answer (hosted run 37052663637). The installer headline stays checked on the bundled sample in testFullFlowFromReplay, which passed in that run. This test keeps the footnote and the two unsure notes. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
Stacked on #191 (
codex/public-assembly-20260930, 0b35f65). This PR adds three commits: 8c47d67, c73aa46 and 94f18ff.What changed
The app shows the server's answer and contacts nobody. Nothing sends a scan to an installer or queues a review. Yet the result screen and the scan's skip hints promised an installer's action, so a homeowner could reasonably wait for a call that never comes:
The app now says what the result needs, not what someone will do:
The other three headlines ("A battery fits here", "One more look", "Not on this wall") are unchanged. An unsure check still shows a person or a camera icon depending on whether a photo can settle it. Only the words changed; the layout didn't need to (ScanCopy.swift).
FullFlowUITests.testFullFlowFromReplayends on the bundled sample, a manual review under unapproved rules. It now checks the rendered headline and both installer footnotes word for word (FullFlowUITests.swift). Against a real server, whose answer can be any decision, it skips them. The newInstallerReviewUITestsholds the demo's review sample still, checks the footnote, and opens Details. It checks that the window check, which needs a person, reads "Needs an installer to check", and that the ground check, which needs a photo, still reads "One more photo would settle this" (InstallerReviewUITests.swift).UI changes
The result screen for the sample result (a spot a person has to settle), iPhone 17 Simulator, default text size, from
FullFlowUITests.testFullFlowFromReplay. Before is the app code at 0b35f65, captured at b926c92 with the same app source; after is this PR. Only the headline changes.The AR view's new title and the changed hints aren't captured: the sample result shows "Example spot, not your result" in AR, and the hints are VoiceOver text.
Verification
testFullFlowFromReplay,testSomethingThereChecksTheWallAgainandtestRejectedUploadGoesBackToReviewpassed, 3 of 3 (job app-review-ui). They cover the result, the spot check's "Something's there" path and the refused upload. The after capture comes from this run, at 8c47d67. It was taken before c73aa46 reworded the footnote, which is below the captured area.testFullFlowFromReplayandInstallerReviewUITests. The earlier run 37052663637 at c73aa46 failed onlyInstallerReviewUITests: it expected the installer headline on the demo's review sample, whose headline is "One more look". 94f18ff drops that check;testFullFlowFromReplaykeeps covering the headline on the bundled installer sample.Risk
docs/app-reviewdescribe earlier builds and still quote the old headline. They are point-in-time reviews and aren't updated here.Checklist
🤖 Generated with Claude Code