Repository navigation
Conversation
…261001 chore(review): run Greptile again when PR commits change
Preserve main's push-review configuration while carrying the unchanged tested 485f1a6 app into a separate continuation branch. Co-Authored-By: Claude Code <noreply@anthropic.com>
The server's checks pass on observed space beyond the area the homeowner confirms in the spot check, some of it inferred rather than seen (a walked path, a photo's view), and the answer doesn't say which. The app can't tell that all the space a battery needs was confirmed (B17), so no result says a battery fits. ResultReading's affirmative answer becomes `candidate`: "A possible battery spot", with a note that the scan couldn't confirm all the space and an installer needs to check the fit on site. Every path that read as a fit goes through it, policy-only manual review included, so a retry, a clear answer or a settled earlier answer can't restore one. The other answers are unchanged, and so are the server's answer, its binding, the scene and the packet. Every result surface agrees. The card shows only failing and unsure checks. Details calls its list the server's calculations on what the scan recorded, drops a pass's positive server reason (keeping its measurement), and hides a candidate's pass summary. The AR title, overlay and off-screen direction name the proposed spot, and passing clearance zones draw in the proposal's blue rather than clear-ground green. The result arrives with a light tap instead of the success haptic (B26). The card's pass ranking was the only use of ResultReading.margin, so it is removed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ResultReading.answer read any pass decision as a candidate, so a pass with no spot or no checks still said "A possible battery spot", and the new exhaustive test caught it. A candidate now needs a spot and at least one check, every one passing; a pass that contradicts itself goes to an installer, with its checks shown as sent. A reject without a spot stays "Not on this wall", and the exhaustive test now checks that too. Details' pass word is now "Passes on recorded data", under the existing note on what the list is. The real-engine UI tests terminate their app, and the render sweep leaves out the scope note's own "not ... confirmed clear". Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Details hid only a candidate's summary, assuming any other summary explains what failed. A pass the reading sends to an installer still carries the server's pass summary, and the demo's review answer said "A spot 3 ft right of your meter fits" under One more look (Sol review). Details now hides the summary of any pass decision as well as any candidate; manual review and reject summaries stay. The demo's review summary names the best spot without calling it a fit, and the panel footnote says the scan suggests where the battery could go rather than covering where it can. A UI test opens Details on the demo's review and overlap answers and checks the summary is there and claims no fit. Co-Authored-By: Claude Opus 5.5 (1M context) <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. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
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 iOS result model and screens now present qualifying passing results as possible battery spots, not confirmed fits. Check details identify recorded-data outcomes, and meter pinning and result arrival use separate haptics. Greptile reviews are enabled for open and push events. ChangesProvisional Placement Results
Scan Haptic Feedback
Greptile Review Setup
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change presents passing results as possible battery spots, not confirmed fits. No merge-blocking issue was found in the reviewed changes. Physical-device haptic behavior still needs verification, as the author notes. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change makes placement advice more conservative: passing calculations describe a possible spot, not a verified fit. The inspected confirmation and AR flows do not upgrade that answer or grant new camera authority. Remaining uncertainty concerns complete before-and-after coverage and the effective permissions of automated repository reviews. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
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. |
 |
The prep-for-prod review found the AX5 check ran only on a demo answer, which is a sample with a different AR instruction, while the real candidate's longer instruction ran only at the default size. The settled engine test now runs at the largest text size, opens the AR view, checks its instruction names a possible spot and what the fit still needs, and that Done stays reachable, with screenshots of the card, its note and the AR view. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The result schema's unsure_cause is an enum without null, so the bound all-pass answer in readingAPassLeavesTheAnswerAsTheServerSentIt failed validation (Actions run 37162046960, the run's only failure out of 714 package tests). Both that test and the UI test's passing answer now remove the key instead of setting it to null. Both answers validate against the schema with Python's jsonschema. 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". |
… UI tests Three demo-answer tests failed at the scroll helper with "Multiple matching elements found" (Actions run 37162669527): Details' DisclosureGroup puts its identifier, result.details, on more than one element, and reading a frame or tapping needs exactly one. Every lookup in the file now takes the first match, as ResultCardUITests' element helper does. The engine tests, the reject test and every other suite in that run passed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
At AX-XXXL the AR card held the title, the placement and the on-site caveat
in full and covered nearly the whole camera (run 37162669527,
resultAR-candidate-settled-AX5), so "See this spot on your wall" showed
little of the spot. While the phone tracks and the result has a spot, the
AR screen now sets CameraChrome's existing `aims`: at the accessibility
sizes the card keeps its title ("A possible battery spot", or the sample's
"Example spot, not your result") and folds the rest under Details, leaving
at least the chrome's 180 pt camera window. "Point at your meter" while
tracking is lost keeps every word in view. CameraChrome itself is unchanged.
The settled engine test, at AX-XXXL, checks the folded card, measures the
open camera between it and Done, opens Details to read the placement and
caveat, folds it again, measures again, and returns with Done, with
screenshots of each state.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
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". |
…creen The open-camera check measured from the Details label, which leaves out the card's padding under it, and could count space off screen (Sol review of a7b4ec3). It now adds that padding, clips both edges to the window, and requires Done to be hittable before measuring. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! 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". |
|
@coderabbitai review |
✅ Action performedReview finished.
|
This branch has not been deployed
1 participant
What changed
A passing placement calculation now produces "A possible battery spot", not "A battery fits here". The location, cable estimate and AR preview remain useful, while the result explains that the scan has not confirmed all the space a battery needs.
candidateanswer and no affirmativefitscase. A candidate requires a spot and nonempty, all-passing checks. Inconsistent pass data goes to installer review.This follows #210 and targets its frozen
codex/candidate-20261003branch. The branch also preserves main's #194 push-review configuration. No source PR base has been advanced.Why
The solver can rely on observed regions beyond the smaller area the homeowner confirms. The current result contract does not identify all supporting regions, and neither a camera view nor the presence of LiDAR proves that those regions are empty. A useful proposed location should not be presented as a verified fit.
The homeowner's answer, retry path and settled-confirmation shortcut all use the same result reading. A previous "It's clear" answer cannot upgrade it to a confirmed fit.
UI changes
The card keeps the proposed location and explains the on-site check. Details labels checks as calculations on recorded data. AR keeps the battery preview and calls it proposed. Actual negative and unsure checks retain their meaning and available follow-up actions.
After-only captures: no earlier hosted run captured a passing result, so there is no matching "before" frame. All frames come from hosted run 37170744330 at 0fb7380 on the iPhone 17 Simulator (iOS 26.5). The engine frames use the real engine on the synthetic replay, with test-served answers. The others use
Fixtures/resultsanswers, which the demo labels as samples.The test measured 400 pt of open camera between the folded card and Done, both before and after opening Details.
Verification
ProvisionalPlacementUITests. The previous head, c9295df, passed the same way in 37166686298.unsure_causeto null, which the schema doesn't allow.result.details.CameraChrome(aims:).Risk
This contains the claim shown to the homeowner. It does not repair the underlying observation/confirmation-area mismatch, prove geometry or obstacle detection, or establish physical-phone acceptance. The supporting-region contract remains open.
Known accessibility limits, unchanged here:
Raw server answers, response hashes, scene exports and packet data are unchanged. The app does not arrange an installer visit or send a review request. Working-space uncertainty and sample-result disclosures remain distinct.
Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit