feat: add adaptive refresh frequency option#1861
Conversation
Implements the adaptive refresh decision record proposed in steipete#1739: a pure first-match-wins policy table (2-30 min bounds) driven by menu-open recency, Low Power Mode, and thermal state. Opt-in via a new Refresh frequency choice; fixed/manual behavior unchanged. Also adds the missing post-sleep cancellation check to the fixed interval timer branch, which previously fired one spurious refresh whenever the frequency changed mid-sleep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KmMSEXFQeQHSBZjUyK1hoU
|
Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 5:09 PM ET / 21:09 UTC. Summary Reproducibility: not applicable. this is a feature PR rather than a current bug report. Verification comes from source/proof review against the accepted design and the supplied packaged-app log and screenshot proof. Review metrics: 3 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Merge after maintainer sign-off confirms the opt-in runtime cadence tradeoff and required checks are green; otherwise pause this implementation while keeping the merged design record. Do we have a high-confidence way to reproduce the issue? Not applicable: this is a feature PR rather than a current bug report. Verification comes from source/proof review against the accepted design and the supplied packaged-app log and screenshot proof. Is this the best way to solve the issue? Yes for the code shape: the implementation is narrow, default-off, and follows the accepted bounded design. The remaining question is whether maintainers want to ship this runtime experiment now. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 386de1eaddd9. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
There was a problem hiding this comment.
Pull request overview
Adds an opt-in Adaptive refresh frequency to CodexBar’s refresh loop, implementing a deterministic policy (2–30 min) driven by recent menu interaction plus power/thermal constraints, with logging, documentation, localization, and extensive test coverage.
Changes:
- Introduces
AdaptiveRefreshPolicy(pure input→decision) and wires it intoUsageStore.startTimer()for per-tick adaptive delay computation. - Records an in-memory
lastMenuOpenAtsignal fromStatusItemController.menuWillOpen(_:)and logs each adaptive decision under a new log category. - Adds targeted unit/integration tests, updates refresh-loop documentation, and localizes the new “Adaptive” label across catalogs.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/CodexBarTests/StatusMenuOpenRefreshTests.swift | Adds integration test asserting menuWillOpen records the menu-open signal on the store. |
| Tests/CodexBarTests/AdaptiveRefreshTimerTests.swift | New tests covering timer behavior across manual/fixed/adaptive modes and cancellation correctness. |
| Tests/CodexBarTests/AdaptiveRefreshPolicyTests.swift | New tests covering policy table boundaries, priority ordering, and bounds guarantees. |
| Sources/CodexBarCore/Logging/LogCategories.swift | Adds adaptive-refresh logging category constant. |
| Sources/CodexBar/UsageStore+AdaptiveRefresh.swift | New wiring helpers: compute/log adaptive decision and apply DEBUG sleep override. |
| Sources/CodexBar/UsageStore.swift | Adds adaptive logger, menu-open timestamp signal, DEBUG counters/overrides, and adaptive timer branch; fixes fixed-timer cancellation guard. |
| Sources/CodexBar/StatusItemController+Menu.swift | Records menu-open signal (store.noteMenuOpened()) in menuWillOpen. |
| Sources/CodexBar/SettingsStore.swift | Adds .adaptive refresh frequency option and localized label mapping. |
| Sources/CodexBar/Resources/ar.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/ca.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/de.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/en.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/es.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/fa.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/fr.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/id.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/it.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/ja.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/ko.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/nl.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/pl.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/pt-BR.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/sv.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/th.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/tr.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/uk.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/vi.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/zh-Hans.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/Resources/zh-Hant.lproj/Localizable.strings | Adds refresh_adaptive translation. |
| Sources/CodexBar/AdaptiveRefreshPolicy.swift | New pure policy implementing the acceptance-criteria decision table. |
| docs/refresh-loop.md | Documents Adaptive mode behavior, policy table, signal ownership, and logging constraints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -23,6 +28,7 @@ enum RefreshFrequency: String, CaseIterable, Identifiable { | |||
| case .fiveMinutes: 300 | |||
| case .fifteenMinutes: 900 | |||
| case .thirtyMinutes: 1800 | |||
| case .adaptive: nil | |||
| } | |||
| // Reason and delay only; never provider/account/email/path/credential/response data. | ||
| self.adaptiveRefreshLogger.debug( | ||
| "adaptive refresh: reason=\(decision.reason.rawValue) delay=\(decision.delay.components.seconds)s") |
RefreshFrequency.seconds is nil for adaptive, and four call sites read that nil as "manual": the reset-boundary scheduler disabled itself, the OpenAI web staleness interval fell to its 120s floor, and both persistent-CLI-session idle windows collapsed to 180s. Add UsageStore.normalRefreshIntervalForHeuristics() (manual nil, fixed seconds, adaptive = live policy delay) and route those call sites through it; ProviderRegistry specs are built before a UsageStore exists, so adaptive maps to the policy's nominal 300s there. Drop the "adaptive refresh: " log prefix so the line is exactly "reason=<code> delay=<n>s". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KmMSEXFQeQHSBZjUyK1hoU
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
The stubbed-codex fixture left the live-system codex account unpinned, so the account-scoped apply guard read the real ~/.codex of whatever machine ran the tests: on a developer machine with a codex login the canned snapshot applied, on CI runners (no ~/.codex) both guard identities stayed unresolved and the liveSystem branch dropped the snapshot, leaving no reset-boundary candidate. Pin _test_liveSystemCodexAccount to a fixture account and give the canned snapshot the same email, matching the pattern the existing account-scoped refresh tests use. Verified locally with HOME pointed at an empty directory (simulating the CI runner) and re-verified the call-site mutation still turns the adaptive boundary test red. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KmMSEXFQeQHSBZjUyK1hoU
Implements the adaptive refresh decision record from #1739, as rewritten there. Posting the implementation so it can be judged against the record's acceptance criteria; I understand the merge gate is still your product sign-off on the record itself, so treat this as ready-when-you-are.
What it does
A new "Adaptive" choice in the Refresh cadence picker, listed after the fixed intervals. Defaults are untouched and fixed/manual behave exactly as before.
The decision lives in
AdaptiveRefreshPolicy(new file, 70 lines), a pure function from input to decision with the first-match-wins table from the record: Low Power Mode or thermal serious/critical gives 30 min; a menu opened within the last 5 minutes gives 2 min; within the hour, 5 min; under 4 hours, 15 min; otherwise (or with no menu open recorded) 30 min. Every input lands inside the 2 to 30 minute bounds, including negative/future ages from clock adjustments.The only signal is one in-memory
lastMenuOpenAttimestamp set inmenuWillOpen. It is never persisted, resets on launch, and never triggers a refresh by itself. Each timer tick logsreason=<code> delay=<n>sunder a newadaptive-refreshcategory and nothing else. The record's non-decisions are respected: no history, no per-account state, no prewarming, no scheduler changes, no ML, not the default.One line outside the strictly additive scope: the fixed-interval timer branch was missing the post-sleep
Task.isCancelledcheck, so changing frequency mid-sleep fired one spurious refresh (the cancel wakes the sleep early andtry?swallows it). Reproduced in a test, fixed with the same guard the adaptive branch uses. Happy to split this into its own PR if you prefer.Update (9e497b5): interval-derived heuristics stay active in adaptive mode
Clawsweeper's review caught a real gap here, and Copilot flagged a log nit; both are fixed.
RefreshFrequency.secondsis nil for adaptive, and four call sites read that nil as "manual, no normal poll". The worst one: the reset-boundary scheduler from #1857 disabled itself entirely in adaptive mode. The others: the OpenAI web staleness interval fell to its 120s floor instead of scaling with the cadence, and the persistent-CLI-session idle windows collapsed to their 180s floor, which would churn CLI sessions between adaptive ticks.The fix follows the review's recommended option. A new
UsageStore.normalRefreshIntervalForHeuristics()resolves manual to nil (unchanged), fixed intervals to their seconds, and adaptive to the delay the policy would pick right now from live signals. Three call sites route through it. The fourth,ProviderRegistry.specs, runs before aUsageStoreexists, so adaptive maps there to a named nominal constant (300s, the policy's warm tier) viaProviderRegistry.nominalRefreshInterval(for:).Regression coverage is the new
AdaptiveRefreshHeuristicsTestssuite (9 tests, 13 cases). Each of the four call sites has a test that goes red if it is reverted to.seconds; I verified that by applying each revert as a mutation and running the suite (one focused failure per call site, and mutating the helper's adaptive branch back to nil turns four tests red). The reset-boundary case runs through the realrefresh()pipeline with a stubbed codex fetch, in both adaptive mode (schedules) and manual mode (still doesn't).The log line is now exactly
reason=<code> delay=<n>swith the category carrying the identification, as promised below (the first push had a redundant "adaptive refresh: " prefix; thanks Copilot).Runtime proof
Built this branch with
Scripts/package_app.sh debug(0.38.1 build 95, bundlecom.steipete.codexbar.debug, isolated defaults domain), set the cadence to Adaptive, and ran it:Reading the three lines: launch has no menu-open history, so the first decision is
longIdle(30 min). At 04:32 I opened the status menu and re-selected Adaptive in Settings; the cadence change restarts the timer, and the fresh decision sees the seconds-old menu open, sorecentInteraction(2 min). The 04:34:20 line is the next tick firing on its own after the 120s sleep plus the refresh, recomputing from live signals. Nothing redacted; reason and delay are all these lines ever carry, by design.The picker in Settings, running this build:
The policy-table transitions themselves (warm/idle/constrained boundaries) are covered exhaustively in
AdaptiveRefreshPolicyTestsrather than re-proven against the wall clock.Tests
27 new cases across three files in the original push: the full policy table with boundary ages, priority ordering, and a bounds sweep over every input combination; timer cancellation tested in isolation from the settings-observer refresh path (restart at the same frequency, exact-delta assertions); and an integration test that drives a real
menuWillOpenand checks the signal lands on the store. Mutation-checked: deleting either cancellation guard or thenoteMenuOpened()call turns at least one test red. Plus the 13 heuristics cases described above.Validation
make check: 0 violations in 1239 filesmake test: all 45 shards passswift buildandswift build --build-tests: cleanplutil -lintclean, locale coverage suites passnode Scripts/check-documentation-links.mjs: OK, 142 local linksdocs/refresh-loop.mdgained an "Adaptive mode" section documenting the table, signal ownership, and the heuristics interval contract.Refs #1739.
🤖 Generated with Claude Code
https://claude.ai/code/session_01KmMSEXFQeQHSBZjUyK1hoU