Repository navigation
perf(ios): pace capture healthy-state watchdog polls - #21406
Git-on-my-level merged 10 commits into
Conversation
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: 5a06a1681e
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b788d5e7d9
ℹ️ 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".
b788d5e to
409dd0d
Compare
|
Review-fix push
Verification: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 409dd0ddb0
ℹ️ 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".
…uide Codex review P2 on BasedHardware#21406: the phone-mic module guide still defined batch liveness as 1 Hz after watchdog pacing landed. Document the verified per-platform healthy/fault cadence and note that Dart's batch stall deadline is unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Independent pre-merge review — VERDICT: APPROVE (Codex gpt-6-luna, read-only, on head 4405a84; not an author of any commit in this PR) No P1/P2 findings. Verbatim summary:
All review threads now have fix/evidence replies (409dd0d for the P1 and two P2s, 4405a84 for the guide P2). |
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
A silent change to the 5s batch interval, or a dropped snap to 1s on rebuild or a stale beat, now fails the recorder and native replay tests. Co-authored-by: Cursor <cursoragent@cursor.com>
…y's fast beat Address review feedback on the paced batch heartbeat: - Healthy cadence 5s -> 4s (P2 r4259674208): strictly under half the unchanged 10s Dart batch-stall deadline. One lost or coalesced beat now lands the next at ~8.5s (2x4s + 500ms leeway) instead of exactly at the >=10s deadline, so a transient missed callback no longer restarts a healthy batch capture. - Preserve the pending fast first fire when recovery reaches .running (P1 r4259674193): applyProgressCadence previously rescheduled the existing timer to now+5s, so a quick interruption/rebuild recovery could stretch the beat gap past 10s and false-restart a recovered session. It now keeps the ~1s first fire (the shape the foreground path already used) and only switches the repeating cadence. - Align the Pigeon cadence doc across Dart/Swift/Kotlin outputs (P2 r4259674216): onBatchProgress is iOS 4s healthy / 1s fault, Android 1Hz. Verified: flutter test test/unit/native_mic_recorder_service_test.dart (29 passed) and ruby app/ios/test/phone_mic_lifecycle_replay_test.rb (14 assertions), which now also pins the 4s cadence and the preserved fast beat on resume.
…uide Codex review P2 on BasedHardware#21406: the phone-mic module guide still defined batch liveness as 1 Hz after watchdog pacing landed. Document the verified per-platform healthy/fault cadence and note that Dart's batch stall deadline is unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
|
CI flake tracking (not branch-caused): "Dart Analyze & Tests" has now failed 4x on this PR's runners (runs 38446564112 + 3
Attribution evidence:
Continuing green-or-rerun per repo policy (no checks skipped, no bypass). Independent review (gpt-6-luna) on head 4405a84: APPROVE (see review comment above). |
Address review thread PRRT_kwDOLkKqys6r_cxj: the recovery preservation branch re-scheduled the pending fault fire to now+1s with the healthy cadence's 500ms leeway (leeway derived from `repeating`), so a recovery late in the fault window could deliver its beat after Dart's unchanged 10s batch stall deadline and restart a fully recovered session. - applyProgressCadence now leaves the already-armed fast fire untouched; the tick handler retargets to the healthy cadence after it emits. - syncProgressTimer leeway now sized for min(first, repeating) so a fast first fire keeps the 50ms tolerance. - Replay regression: interrupt, resume 0.6s into the fault window, expect the beat on the armed deadline (~0.4s out), not resume+1s; verified it fails on the previous code and passes on the fix.
4405a84 to
2dc8d94
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dc8d94ce3
ℹ️ 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".
Address review thread PRRT_kwDOLkKqys6sETFD: the recovery guard only preserved fast-to-healthy transitions, so a second route/config fault arriving before the preserved 1s recovery beat fell through and re-anchored the fire to now+1s. With one lost healthy beat, repeated quick faults could push the only liveness event past Dart's unchanged 10s batch stall deadline and restart a recovered session. applyProgressCadence now keeps any already-armed deadline — preserved fast fire, second fault before it, or unchanged healthy window — and only a fault pulling a far-off healthy deadline in to the fast cadence re-schedules. Replay regression: interrupt, resume, then a second fault before the preserved fire; verified it fails on the previous code and passes on the fix.
… faults Codex review P2: a second route/config/interruption signal before the preserved 1s recovery beat fell through the healthy-only guard and rescheduled the already-fast deadline later. Keep the armed fast fire when the interval is already fast; only the healthy 4s cadence arms after the beat fires. Replay test pins the repeated-fault case. Co-authored-by: Cursor <cursoragent@cursor.com>
…ne/battery-ios-watchdog Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # app/ios/Runner/PhoneMic/PhoneMicController.swift # app/ios/test/phone_mic_lifecycle_replay_test.swift
|
Independent pre-merge review — VERDICT: APPROVE (Codex gpt-6-luna, read-only, on head 449800d; not an author of any commit in this PR) No P1/P2 findings. Verbatim summary:
Review history on this PR: round 1 (gpt-6-luna) APPROVE on 4405a84; two further Codex P2s ("late recovery deadline", "repeated faults re-anchor") were fixed by 2dc8d94 and 5c3ec61 (+449800dc replay test, verified failing on the old guard). All 6 review threads are resolved with fix/evidence replies. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 449800dc0f
ℹ️ 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".
…21478) ## Summary Answers the one unresolved non-outdated thread on #21406 ("Restore healthy leeway after the fast foreground beat"): `appBecameActive` arms `first: 1s, repeating: 4s`, and a `DispatchSourceTimer` records a single leeway for the whole schedule — 50 ms for the fast first fire. Because `progressIntervalArmed` was recorded as 4 s, the tick handler's `desired == armed` check never retargeted, so every later healthy beat for the rest of the session kept the 50 ms coalescing tolerance the fast beat needed. The pending fast first fire is now tracked explicitly (`progressFastFirePending`). After it emits, the timer retargets to the armed repeating cadence, regaining the 500 ms healthy leeway. If the capture state changed while the beat was pending, the retarget defers to the existing desired-cadence branch instead, preserving the fault-path behaviors pinned by the #21406 review (a preserved fast fire through recovery, a second fault before the fire). `cancelProgressTimer` clears the flag with the rest of the timer state. ## Product invariants - None of the indexed invariants are triggered by this path (no uplink pause, no capture-policy change: the tick still fires at the same deadlines; only the kernel coalescing tolerance after the first fast beat changes). The batch liveness contract is unchanged: healthy 4 s beat under Dart's 10 s stall deadline, one fast beat on foreground return. ## Failure class Failure-Class: none ## Test plan - [x] Behavior reasoning against `syncProgressTimer`/`applyProgressCadence` state machine; flag lifecycle covers arm/retarget/cancel - [x] Pre-push gate (shared PR contract, hygiene, supplied PR body) - [ ] Full CI green on this exact head (iOS build lanes), then independent review before merge <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/BasedHardware/omi/pull/21478?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="View guided diff" src="https://www.cubic.dev/buttons/review-in-cubic-light.svg"></picture></a> <a href="https://www.cubic.dev/action/auto-fix/pr/BasedHardware/omi/21478?returnTo=https%3A%2F%2Fgithub.com%2FBasedHardware%2Fomi%2Fpull%2F21478&source=description" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"><img alt="Turn on auto-fix" src="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img](https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-light.svg"></picture></a](https://www.cubic.dev/buttons/review-in-cubic-light.svg"></picture></a){kind=link}
![https://www.cubic.dev/buttons/turn-on-auto-fix-dark.svg"><source](https://www.cubic.dev/buttons/turn-on-auto-fix-dark.svg"><source){kind=link}
![https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"><img](https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"><img){kind=link}
![https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"></picture></a](https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"></picture></a){kind=link}
1 participant
What changed and why
iOS phone-mic capture was waking once a second while already healthy: the batch heartbeat crossed into Dart at 1Hz, and the Dart stall watchdog polled on the same cadence. Healthy batch capture now heartbeats every 4s (strictly under half the 10s batch deadline, so one lost or coalesced beat still lands ~8.5s in and cannot false-stall), and the iOS Dart watchdog waits that long only while liveness is fresh. Recovery from an interruption/rebuild preserves its pending ~1s fast beat before returning to the healthy cadence. The 3s stream and 10s batch stall deadlines are unchanged, and any interruption, rebuild, stale beat, or foreground return snaps back to the 1s poll. Android keeps the 1s poll.
Product invariants affected
none
How it was verified
flutter test test/unit/native_mic_recorder_service_test.dart— 29 passed, including the pacing cases (3s stream deadline, batch heartbeats without a false stall, stall at 10s, interruption/foreground snap-back, rebuild grace).ruby app/ios/test/phone_mic_lifecycle_replay_test.rb— passed (14 assertions). The production controller stays quiet well inside the healthy 4s cadence, reports storage-full from the write path, snaps to a 1s beat on interruption, emits its preserved ~1s fast beat on resume before returning to the 4s cadence, and emits one fast beat after foreground.Tests
app/test/unit/native_mic_recorder_service_test.dart— iOS pacing group: deadline cap, healthy-cadence progress, interruption and foreground snap-back, rebuild grace. Existing unpaced cases still cover Android's 1s poll.app/ios/test/phone_mic_lifecycle_replay_test.swift— batch heartbeat cadence and immediate storage-full reporting through the production controller.UI evidence
UI-Evidence: none -- capture watchdog timing only; no UI change.
Failure class (fixes)
Failure-Class: none