Repository navigation
ios: three small fixes from field test run 1 (#25, #26, #31) - #34
Conversation
|
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 |
The marking prompt and the mark and remove labels lowercased the feature name, so they read "Tap the ac unit" and "Mark ac unit". ScanCopy.noun keeps "AC" in capitals. The result's measurement lines and the 3D view's summary now give VoiceOver feet and inches spelled out (Distance.spoken), since it reads "ft" and "in" as letters. The on-screen text is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The upload delegate held progress at 0.99 until the server answered, and the server works out the spot inside that request. So the screen sat on "Sending measurements, 99%" for the whole analysis and "Check clearances" never lit up. Report 1 once every byte of the body is sent. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
In field test run 1 "Slow down" showed twice while the tester stood still with "What do you see?" open: turning the phone to aim reads as moving to the capture gate. Hide that one prompt while the tray is open; tracking problems still show. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
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. |
|
The current head |
The ternary between two static methods made the compiler fail with "failed to produce diagnostic for expression" on CI. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Follow-up on my build finding: |
SamGu-NRX
left a comment
SamGu-NRX
left a comment
There was a problem hiding this comment.
I read the behaviour against the code at 57a3ae1. All three do what the description says. It merges cleanly with the current t3/ios-mvf head (75dea41, checked with git merge-tree).
- #31, upload stuck at 99%.
didSendBodyDatareports 1 oncetotalBytesSentreaches the expected size.ScanEnginethen moves.uploadingto.analyzing, so "Check clearances" lights up while the server works. The progress callback only acts while the state is still.uploading, so a late delegate call can't pull a finished result back to.analyzing. A body of unknown size (totalBytesExpectedToSend≤ 0) still sends no progress, and the existing.analyzingassignment aftersubmitreturns covers that case. - #26, "Slow down" while standing still. It's hidden only while the mark tray is open, and the card's tone follows the same filtered value. Once a kind is picked, the tray closes and the marking prompt takes over, since
instructionputsstate.markingahead of coaching. So the whole run 1 case is covered: tray open, then marking. Tracking problems and the other coaching cases still show. The capture gate still refuses photos while the phone turns. That's fine, since nobody walks with the tray open. - #25, "Tap the ac unit".
ScanCopy.nouncovers every place the head lowercases a feature name: the marking prompt and the Mark, Add and Remove labels.Distance.spokenalready exists, so the VoiceOver strings compile, and removing the duplicatefeetAndInchesfromResultScene3Dis safe.
One thing for the phone check: the "Sending measurements" step now completes the moment the body is sent. On a fast connection it may flash past, so watch that the step list still reads in order.
The window-row assertion read "at least 3 ft" from the VoiceOver value. The B-16 fix (#34) spells lengths out there ("3 feet"), so the test would fail once both land. It now checks "The rule is at least 3". Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Review: no open findings at 72144a7. I rechecked the six-file PR diff after the base merge. The compiler issue from e760327 is fixed, and the current-head iOS check passed. CodeRabbit skipped this non-default-base PR; this is the independent read-only code-review gate, not a claim that the three specific behaviors have been checked on a phone. A new code push needs another review. |
* Add the capture ingest API on the volume backend. Provisional WS-00 contracts stay in this branch until those PRs merge. Multipart returns 501. Terraform modules are validated with terraform test and are not applied. * Expose the Modal app as app so modal deploy can find it. The dev deploy only attaches the hss-dev-api secret. The GCP secret waits until that project exists. * Serialize capture create and delete, and keep late uploads on the packet hash. Create holds a lock and writes the capture before the idempotency key. Delete takes the same per-capture lock as register and finalize. A file registered after accept must match the packet sha256 and size. * Keep orchestrator status, persist dict state, and close the ingest gaps. Finalize no longer writes processing over a status the orchestrator already stored, manifest rechecks omit nulls, and ModalDictState keeps capture records in the dict. Upload tokens are checked before the body, reviewer rights come from a role claim, device results omit verdicts, and delete removes capture prefixes. * common: WS-01 additive Lock LocalFsStorage index updates so parallel promotion can save the volume index. * Restrict review HTML to reviewers and bind the real services module. A device GET /review is 403, so the hidden reject stays off the owner page. The finalize hook test follows the services object import returns. * Keep path_invalid for dot segments after the storage-id merge. RelPath now rejects those paths before the route. The ones the route would refuse still return path_invalid, and amendment and still ids match the new patterns. Tests that build the app set dev vision settings so the prod default does not raise. * Expect schema when RelPath rejects a dot segment. PR 47's path pattern fails those values before the route. The tests now expect that schema error. packet.json still returns path_invalid. * Choose replay file meta from the file role. mini-arkit stills reuse keyframe images. Matching stills first sent still meta on a keyframe and finalize never started. * Claim each ModalDictState event sequence under one lock. Eight writers were reading and rewriting the same event list, so sequence numbers collided and events disappeared. Each event is now its own key. * Accept every demo request as the device principal demo. HSS_AUTH_MODE=none skips the bearer. Hidden-reject masking stays on /result, and /review is reachable. Static tokens and JWT remain unused. --------- Co-authored-by: hss-cloud-agent <hss-cloud-agent@users.noreply.github.com>
2 participants
Three small fixes for issues found in field test run 1 (PR #23). This is based on
t3/ios-mvf(PR #10), so it merges into that branch, notmain. @SamGu-NRX it's your branch, so merging is your call.What changes
ScanCopy.noun(kind)keeps "AC" in capitals. It's used in the marking prompt and in the Mark, Add and Remove VoiceOver labels. VoiceOver also gets feet and inches spelled out (Distance.spoken) in the result's measurement lines and the 3D view's summary. The on-screen text doesn't change.Not in this PR
#24, #28 and #29 all come from the same guidance and coverage logic and need a design call, so they're left for you. #27 needs a team decision. #30 needs more field runs.
Testing
Not built locally: this was written on Windows, without Xcode. The iOS CI job compiles the app and runs the HouseScanKit and UI tests. The changed code is all in the app target, which has UI tests but no unit tests. So CI checks that it compiles and that the existing flows still pass. The new behaviour itself still needs checking on a phone:
GitHub only closes linked issues when a PR merges into
main, so close #25, #26 and #31 by hand when this lands.🤖 Generated with Claude Code