Repository navigation
fix(onboarding): Watch instructions layout, specific pairing errors, find-us fits on screen - #21426
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Git-on-my-level
added
the
ci:full
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. |
![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: 23e4945f09
ℹ️ 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".
| "firstCaptureProcessing": "Creating your first conversation…", | ||
| "firstCaptureResult": "Your first conversation", | ||
| "firstCaptureEmpty": "I didn't catch enough speech, try again with a few sentences", | ||
| "watchCaptureTitle": "Record from your Apple Watch", |
There was a problem hiding this comment.
All eleven additions here are copied verbatim from app_en.arb into every one of the 48 non-English ARBs, so Arabic, Spanish, Japanese, and other users see the new Watch and pairing-recovery flow in English; this is especially harmful for the permission and stale-bond remediation text. Supply per-locale values and regenerate l10n instead of shipping English fallbacks.
AGENTS.md reference: app/AGENTS.md:L118-L120
Useful? React with 👍 / 👎.
| key: ValueKey(provider.pairingFailureVersion), | ||
| reason: provider.pairingFailureReason!, | ||
| onDismiss: provider.dismissPairingFailure, | ||
| onRetry: widget.onRescan, |
There was a problem hiding this comment.
When an unsaved device's handleTap fails, this binds Try Again to _scanAgain, so the action only restarts discovery and never retries handleTap; the user must tap the device row a second time. For staleBond this is a dead end: DeviceProvider.initiateConnection returns while staleBondRecoveryRequired is set, and forgetting the OS bond does not clear that Dart latch, so the advertised retry cannot discover or pair until a restart or a separate app-side forget path. Give the card the failed-device recovery callback, including clearing the stale latch on an explicit retry, rather than onRescan.
Useful? React with 👍 / 👎.
| if (_isDisposed || epoch != _scanEpoch) return; | ||
| if (shouldReportEmptyDeviceScan(deviceCount: deviceList.length)) { | ||
| pairingFailureVersion++; | ||
| pairingFailureReason = DeviceConnectFailureShownFailureReason.notFoundOrDropped; |
There was a problem hiding this comment.
When the first scan is empty at 10 seconds but a periodic scan later discovers a device, for example after the user turns it on, this assignment remains set because onDevices updates the rows but never clears pairingFailureReason. The page consequently keeps showing “Couldn't reach your Omi,” suppresses the devices-found status, and keeps the artwork reduced even while a tappable device is present; clear the scan-originated timeout state when nonempty discovery results arrive.
Useful? React with 👍 / 👎.
| if (_isDisposed || epoch != _scanEpoch) return; | ||
| if (shouldReportEmptyDeviceScan(deviceCount: deviceList.length)) { | ||
| pairingFailureVersion++; | ||
| pairingFailureReason = DeviceConnectFailureShownFailureReason.notFoundOrDropped; |
There was a problem hiding this comment.
When native onScanBlocked fires after this timer is created, such as Android returning scanner_unavailable or the radio/permission changing between the readiness check and native scan start, _emitPairingBlocked first publishes the applicable reason but this still-live callback unconditionally replaces it with notFoundOrDropped. That swaps permission/power guidance for incorrect reachability advice and emits a second misclassified Shown event; cancel the timer on a native block or guard this fallback when a failure reason already exists.
Useful? React with 👍 / 👎.
| reason: provider.pairingFailureReason!, | ||
| onDismiss: provider.dismissPairingFailure, | ||
| onRetry: widget.onRescan, | ||
| onGuide: () => ConnectionGuideSheet.show(context), |
There was a problem hiding this comment.
When the failure card's How to pair action is used, most notably for the 10-second empty-scan state that now replaces FindDevicesPage's prior empty state, this opens the sheet directly and bypasses _showConnectionGuide(), so the active Connection Guide Opened event is never emitted. The footer and old empty-state entry points both emit it, so guide usage after failures will be systematically undercounted; route this action through a tracked callback or emit before showing the sheet.
Useful? React with 👍 / 👎.
…lity errors Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a269e70463
ℹ️ 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".
| if (pairingFailureReason == DeviceConnectFailureShownFailureReason.notFoundOrDropped) { | ||
| pairingFailureReason = null; |
There was a problem hiding this comment.
When Connect starts with Bluetooth off on Android, the global listener's Enable button turns the adapter on and calls deviceService.discover() directly (bluetooth_guidance_listener.dart:144-178), so a successful nonempty result reaches onDevices without going through _scanAgain. Fresh evidence in the final tree is this direct post-enable path: because this branch clears only notFoundOrDropped, bluetoothOff remains set, leaving device rows visible while the card still tells the user to turn on Bluetooth and the illustration remains shrunk; clear resolved scan-block reasons when discovery succeeds while preserving connection failures such as stale bond.
Useful? React with 👍 / 👎.
| DeviceConnectFailureShownFailureReason.notFoundOrDropped => l10n.pairingNotFoundOrDropped, | ||
| DeviceConnectFailureShownFailureReason.staleBond => l10n.pairingBondRecovery, |
There was a problem hiding this comment.
FoundDevices routes Bee, Plaud, Fieldy, Friend Pendant, and Limitless rows through the same handleTap path (found_devices.dart:393-403), but these two failure branches resolve to text that literally says “Couldn't reach your Omi” or “Forget Omi.” A connection failure for any of those supported products therefore tells the user to troubleshoot or forget a different device; carry the attempted device name/type into the card or use generic “device” copy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0390834f37
ℹ️ 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".
| /// timeout, missing device or dropped link. Keep one reachability bucket and | ||
| /// use the existing stale-bond latch for the distinct Bluetooth-settings action. | ||
| DeviceConnectFailureShownFailureReason connectFailureReason(Object error, {bool staleBond = false}) { | ||
| if (error is BluetoothAdapterUnavailableException) { |
There was a problem hiding this comment.
When Bluetooth is switched off or its permission is revoked after discovery but before a device row is tapped, NativeBleTransport.connect throws BluetoothAdapterUnavailableException, but DeviceConnection.connect wraps it in DeviceConnectionException, DeviceService.ensureConnection catches that and returns null, and OnboardingProvider.handleTap substitutes StateError('Device initialization did not complete'). Consequently this new branch never receives the adapter exception on the production pairing path, so the card and Device Connect Failure Shown report not_found_or_dropped instead of the actionable Bluetooth reason; preserve or unwrap the cause before calling connectFailureReason.
Useful? React with 👍 / 👎.
1 participant
Android pairs after opening Connect at 26%, versus 54% on iOS (new mobile signups observed September 13–October 12). The current failure copy offers no actionable cause; the Apple Watch paragraph uses title weight with its action mid-screen, and the acquisition survey hides its final choices below Continue.
This change pins Watch's primary action using the shared first-capture layout, presents body-weight recording steps while preserving paired/installed guidance, and fits all 12 acquisition choices in an ordered two-column layout at 393×852. Large text uses scrollable rows. Option labels, values, and order stay unchanged.
Connect displays an inline failure card with a visible dismiss, Try Again, and How to pair. Exposed Bluetooth power, Bluetooth/Nearby devices/location permissions, stale-bond, combined reachability, and unknown cases have actionable localized messages. Device Connect Failure Shown is a new registered event with failure_reason and platform, emitted after the card's first frame. Device Connect Failed and the registry contract stay unchanged. A 10-second empty scan retains No Omi Found, its instructions and Contact Support, and emits no Device Connect Failure Shown. A visible saved or discovered device clears the reachability card. Bluetooth-off and unauthorized exception types take precedence over stale-bond advice.
The connection service catches transport errors and returns null: not found, timed out, and connected-then-dropped cannot be separated here. They use not_found_or_dropped and shared reachability guidance. Follow-up: preserve typed transport causes at the service boundary in a separately reviewed connection change; this PR keeps transport and connection/launch-timeout behavior unchanged.
Validation:
bash app/test.sh test/widgets/onboarding_polish_test.dart: bootstrap and all 138 protected contract tests passed. The focused fixture was then corrected to use an inert provider rather than a live service singleton.ONB_POLISH_RENDER=1 flutter test test/widgets/onboarding_polish_test.dart: all 6 tests passed; Watch bottom inset/paired/installed guidance, every reason→copy, event properties and one event per presentation, connect dismiss, all 12 survey options visible at 393×852, 2.0-scale scrolling to Other and its editor, plus 21 original renders; the two provider-driven empty-scan renders are covered below.flutter test test/widgets/first_capture_test.dart test/widgets/onboarding_survey_sync_test.dart test/widgets/onboarding_bottom_inset_test.dart test/providers/onboarding_provider_connect_failure_test.dart test/services/devices/pairing_failure_test.dart test/utils/analytics/onboarding_funnel_analytics_test.dart: all 31 neighboring regressions passed. Updated only the first-capture pairing assertion/fixture for the new inline card; the sibling lane's five red test files are untouched.ONB_POLISH_RENDER=1 flutter test test/providers/onboarding_provider_pairing_presentation_test.dart test/widgets/onboarding_polish_test.dart test/widgets/first_capture_test.dart test/services/devices/pairing_failure_test.dart test/providers/onboarding_provider_connect_failure_test.dartpassed all 37 tests. This drives the real OnboardingProvider timer and onDevices paths, verifies telemetry via a recording AnalyticsAdapter, retains stale-bond guidance, and tests exception precedence. Connect-failure and empty-scan renders were regenerated on iOS/Android and opened; the empty-state controls inherit the themed real font.flutter analyze: no issues.python3 app/scripts/l10n.py check: 49 locales, 3,877 consistent keys.python3 scripts/check_event_registry.py --write: 183 events and executable consumers pass.make preflight: all 24 selected checks passed on the final commit; metadata-only preflight also passed all 5 selected checks.CI at
a269e7046334eb49daeac059b3640325b8de592e: Dart Analyze & Tests ran underci:fulland completed failure. Analyzer passed. The only five failing cases are the known main failures tracked by #21425: launch-timeout connection preservation (1), category large-text layout (2), AI app generator large-text layout (1), and settings search header stability (1). No onboarding tests failed. No physical device pairing or full-suite acceptance is claimed.Known follow-up: unify the global Bluetooth guidance sheet and inline failure card so they cannot stack. This PR preserves the existing guidance flow.
Risks: new messages use English fallback for other locales; no physical-device BLE acceptance is claimed; the existing global Bluetooth guidance can appear alongside the inline card; acquisition labels remain localized legacy analytics values; large text deliberately needs scrolling.
Failure-Class: none
🤖 Generated with Claude Code