Extend menu bar conditionals beyond usage percentages - #3088
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbeffc237f
ℹ️ 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".
|
Codex review: found issues before merge. Reviewed August 20, 2026, 5:36 AM ET / 09:36 UTC. ClawSweeper reviewWhat this changesThis PR expands menu-bar conditional rules from four usage percentages to usage, time, pace, run-out, balance, and cost metrics, with editor, persistence, refresh, localization, and test updates. Merge readinessKeep open: the new run-out predicate scheduler can leave a conditional branch stale for up to a minute, and VISION.md requires owner sign-off for this new persisted feature. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherCodexBar converts provider snapshots and clock-derived usage estimates into a configurable menu-bar title. Conditional layout rules consume those values to choose tokens, while persistence retains each user’s rule library across launches and upgrades. flowchart LR
A[Provider snapshots] --> B[Menu bar render data]
C[Clock and reset times] --> B
B --> D[Conditional rule evaluation]
E[Saved rule library] --> D
D --> F[Refresh scheduling and title cache]
F --> G[Menu bar title]
E --> H[Legacy compatibility projection]
Decision needed
Why: VISION.md requires sign-off for new features and behavior changes affecting data storage; code review cannot determine the acceptable downgrade product contract. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Schedule run-out predicates at the actual rounded-ETA transition, retain the explicit non-inverting downgrade projection, and merge only after the owner approves the expanded persisted-rule surface. Do we have a high-confidence way to reproduce the issue? Yes, from source: a run-out ETA changes continuously with Is this the best way to solve the issue? No: the refresh plan must calculate the next rounded ETA transition rather than use a generic minute boundary; the persistence approach is otherwise a reasonable safety trade-off pending owner approval. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a3457dafbe63. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (10 earlier review cycles; latest 8 shown)
|
P2 — Schedule predicate-only pace and run-out refreshes → fixed in f2b4673You were right, and my reasoning for excluding Two changes:
A minute tick is the right granularity rather than a hedge: both numbers are pre-rounded to what the menu bar actually shows — The gate stays precise: money predicates ( Regression coverage in
P1 — persisted conditional surface needs maintainer sign-offAgreed, and not something I can resolve from here — flagging for @steipete. Two notes that may help the decision:
Note on the live-verification step
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
CI note:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
P1 — Preserve older conditional libraries on downgrade → fixed in 3b53b91You're right, and you caught the exact hole in my reasoning: element-wise decoding only helps a build that already has the lenient decoder. The build a user actually downgrades to decodes Fixed by dual-writing the way layouts already do, so the mechanism matches
The projection drops an entry when any clause uses a metric outside the original four or a non- Coverage in
Remaining itemThe VISION.md feature sign-off is the one thing left, and it needs @steipete rather than a code change. Everything actionable from both reviews is now addressed:
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
CI: the two red jobs are identical to
|
main @ 61f54225 (run 32318564053) |
this PR @ 3b53b91 (run 32336983376) |
|
|---|---|---|
| Failing jobs | swift-test-macos (0, 2), lint-build-test |
swift-test-macos (0, 2), lint-build-test |
| Failing tests | generic provider details keep canonical labels alongside localized core metrics — and only that one |
same, and only that one |
lint-build-test is not an independent failure: it is the aggregate gate, failing with macOS test gate/result mismatch: required=true deferred=false result=failure because shard 0 is red.
That main run predates this PR, and it is the only recent main run that actually executed the macOS suite — the others show swift-test-macos: skipped, so they carry no signal either way.
The test builds an OpenRouterUsageSnapshot and asserts its "API key" detail section under a zh-Hant override. Nothing here touches OpenRouterUsageSnapshot, UsageMenuCardView.Model.make, or the OpenRouter detail sections; MenuBarLayoutBalanceResolver only reads detail rows. Shard 1, which carries the conditional suites, is green.
Locally the exact CI group passes on this head:
$ swift test --skip-build --no-parallel --filter \
'^CodexBarTests\.PopupLocalizationTests/|^CodexBarTests\.PredictivePaceWarningTests/|^CodexBarTests\.PreferencesPaneSmokeTests/|^CodexBarTests\.PreferencesSelectionTests/'
✔ Test run with 53 tests in 4 suites passed
Happy to chase it in a separate PR if wanted — it looks like a CI-environment difference in how that provider's detail sections resolve, which is a different area from this change.
Conditional predicates could only compare four percent-used windows. They now compare 18 metrics across four units: percent windows, the direct primary/secondary/tertiary lanes, four reset countdowns, three pace deltas, run-out, credit balance, and today/30-day cost. Metrics with two readings (percent windows, lanes, balance) gain a used/remaining select, so "session > 50% used and session resets in < 2h" is expressible. Pace, run-out, balance and cost were only carried as display strings, which cannot be compared, so MenuBarLayoutRenderMetrics carries their numeric twins pre-rounded to the same granularity as the text they mirror. Three refresh gates needed widening for the new data dependencies: - The title cache key had no component that moves with the clock, so a countdown predicate would have served its pre-flip title indefinitely. It now keys on the per-conditional outcome, evaluated once per render. - The four observation signatures gated on display tokens; a predicate on cost or balance has no token. They now also read the conditionals' metrics, which additionally fixes lane tokens inside conditional branches being invisible to the lane signature. - A reset-countdown predicate flips at an instant nothing else ticks on, so the countdown scheduler wakes at `resetsAt - threshold`. The conditional library is now decoded element-wise: this change makes forward-incompatible metric values possible for the first time, and one unknown value would otherwise have wiped the whole library on a downgrade. Ships an "Auto % / Resets in" default that renders the automatic percentage while the lane has headroom and the reset countdown once it is spent.
Three observation-signature gaps let a predicate flip without a redraw: - Cost signatures recorded only the currency-formatted string, so two token-cost updates could cross a threshold while both formatted to the same cent. A referenced cost metric now signs the unrounded amount losslessly. - The balance signature recorded only the rendered "Remaining" row, so a `balance used` predicate — which reads the "Used" row no token surfaces — was entirely unsigned. Both amounts are now signed. - The lane signature recorded the displayed reading, which follows `usageBarsShowUsed` and clamps remaining at zero, while `RateWindow.usedPercent` deliberately preserves over-quota values. A used-direction predicate such as `primaryLane > 105%` could move 104% -> 106% against a constant `0.000`. The lane signature is now scoped to what the layout renders, and a new conditional-window signature covers what conditionals read: the raw used percent (which remaining derives from, so it covers both directions) plus `resetsAt`, which countdown predicates depend on and no display token contributes.
`menuBarWeeklyPaceRefreshDelays` is gated on a placed `.pace(.weekly)` token and only wakes once, at the pace-eligibility boundary. Excluding `runsOutIn` from the conditional reset schedule on the assumption that scheduler covered it therefore left a hole: a layout whose only pace or run-out reference is a predicate got no clock wake-up at all, so it kept rendering the branch that was true when the value last moved. Referenced weekly-pace predicates now also trigger the eligibility wake-up, and any referenced pace or run-out predicate schedules a minute tick. Both numbers are pre-rounded to the granularity the menu bar shows -- whole percentage points and whole minutes -- so a minute tick is exactly enough, and it is the cadence a `.resetCountdown` token already costs. Money predicates deliberately schedule nothing: they move only when new provider data arrives.
Decoding the library element-wise only helps builds that already have the lenient decoder. The build a user actually downgrades to decodes `menuBarLayoutConditionals` strictly and falls back to `[]`, so one saved rule using a new metric would empty the entire library there. The conditional library now dual-writes the way layouts already do: `menuBarLayoutConditionalsV2` keeps full fidelity, and the original key keeps an older-readable projection. `loadLibrary` mirrors `preferredLayout` — the current key wins unless the legacy key disagrees with its own projection, which only happens when an older release wrote it, and that edit must survive. The projection drops an entry when any clause uses a metric outside the original four, and also when any clause uses a non-`.used` direction. The second case is the subtler one: an older release's synthesized decoder ignores the unknown `direction` key, so `session remaining > 80` would come back as `session used > 80` and render the opposite branch. A missing rule is visibly missing; an inverted one is not.
3b53b91 to
8f62fba
Compare
P1 "Restore provider-detail localization" → branch was stale; rebased onto
|
| Finding | Status |
|---|---|
| P2 Include numeric costs in conditional signatures (codex) | Fixed |
| P2 Track used lane values for used-direction predicates (codex) | Fixed |
| P2 Schedule predicate-only pace and run-out refreshes | Fixed |
| P1 Preserve older conditional libraries on downgrade | Fixed |
| P1 Restore provider-detail localization | Not a real deletion — stale branch, now rebased |
| Expanded persisted surface needs VISION sign-off | Needs @steipete |
make check clean on the rebase (0 SwiftLint violations, 23/23 locale catalogs); 226 tests green across the 8 affected suites.
On the downgrade contract you flagged for owner acceptance: the projection deliberately omits expanded rules on an older build rather than handing over rules it would misread. A dropped rule is visibly absent; a silently inverted one (session remaining > 80 decoding as session used > 80 on 0.54.0, whose synthesized decoder ignores the unknown key) would be worse than absent. Happy to change that trade if you'd rather older builds see a best-effort translation.
@clawsweeper re-review
|
🦞🧹 I asked ClawSweeper to review this item again. |
`UsageFormatter.convertedCost` returns the source amount unchanged when it has no rate for the provider's currency, and both cost producers passed that value straight through as `costTodayUSD`/`cost30dUSD`. A `Cost today > 5 USD` rule would then compare, say, €6 against a $5 threshold and pick the wrong branch. Both producers now keep the amount only when the conversion actually landed in USD. Otherwise the predicate sees no value and evaluates false, which is the existing contract for a metric the provider does not report. The rendered text is untouched and still shows the provider's own currency.
P1 — Reject unconvertible cost values for USD predicates → fixed in 659a813Real bug, and my own code comment was the tell — I wrote "without a rate Both producers now keep the amount only when the conversion actually landed in USD: let toUSD = { (value: Double) -> Double? in
let converted = UsageFormatter.convertedCost(
value,
preferredCurrency: "USD",
providerCurrency: sourceCurrencyCode)
return converted.currencyCode == "USD" ? converted.value : nil
}Guard mirrored in Balance needs no equivalent guard: Coverage:
Remaining item is the VISION sign-off for @steipete — no actionable code findings left from either reviewer. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
* feat(kiro): show overage credits against their cap kiro-cli /usage states plan credits alone and omits the overage section for organization accounts, so a spent plan looks like the account is out. Read GetUsageLimits with the CLI's own token (read-only) and surface overage as a second credit window plus charges against the overage budget. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(kiro): reject impossible usage counters from GetUsageLimits An overage larger than total usage would clamp planUsed to zero and overwrite valid CLI numbers. Honor API-disabled overage over a stale CLI Enabled line so enrichment cannot resurrect a cap the API says is off. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(kiro): reject GetUsageLimits payloads that exceed the plan Drop enrichment when plan usage is above the plan ceiling, and treat an unrecognized overage status as unknown so the CLI overage line can still stand. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: import FoundationNetworking for Linux URLSession types * fix(kiro): resolve the CLI state database on Linux GetUsageLimits enrichment always looked under macOS Application Support, so Linux refreshes never found data.sqlite3. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(kiro): reject overage above cap and honor API currency Best-effort GetUsageLimits data should not present a spend window above its ceiling or format non-USD charges as dollars. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(kiro): propagate cancellation from usage-limits enrichment A cancelled GetUsageLimits call now fails the refresh instead of publishing a CLI-only snapshot as success. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(kiro): drop CLI USD overage estimate when the API currency is not USD A missing overageCharges field no longer lets a dollar CLI fallback render in the API's non-USD currency. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(kiro): add locale entries for overage copy Translate the new Overage window title and Overage credits left detail row in every app catalog so non-English UIs no longer fall back to English. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(kiro): honor API-disabled overage and localize cap phrases GetUsageLimits DISABLED now replaces a stale CLI Enabled status, and Kiro "of N" detail values go through the of %@ localization key. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(i18n): expect localized OpenRouter API key details in zh-Hant Provider detail titles and catalogued labels go through L after #3084, so the popup test must assert the Traditional Chinese strings. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(kiro): clamp overage that runs slightly past the cap Rejecting over-cap counters dropped the whole GetUsageLimits payload and hid overage on organization accounts, so the gauge now clamps to the cap instead. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(kiro): keep over-cap overage and localize credit units Clamping hid the amount Kiro actually billed, and overage usage still rendered the English "credits" suffix in other locales. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(kiro): keep CLI overage when API omits the cap ENABLED without overageCapWithPrecision is incomplete enrichment, not a disabled account. * fix(kiro): mark API-enabled overage when the CLI omits it Organization accounts skip the CLI overage section, so the menu-bar modes never saw an Enabled row. * fix(kiro): keep CLI plan usage when API bonuses are present GetUsageLimits folds bonus spend into currentUsage, so overwriting the plan gauge would double-count bonus credits. * fix(kiro): parse bonus-inclusive usage above the plan limit GetUsageLimits folds bonus spend into currentUsage, so rejecting planUsed > planLimit dropped overage enrichment for those accounts. * fix: preserve stacked menu bar layout line breaks (#3094) * Preserve card menu-item subclass during cached swaps (#3093) * Preserve menu item subclasses during cached swaps * Add native menu proof for cached shell swaps --------- Co-authored-by: Kiran Magic <262980978+kiranmagic7@users.noreply.github.com> Co-authored-by: Peter Steinberger <steipete@gmail.com> * docs: changelog for #3093 * Extend menu bar conditionals beyond usage percentages (#3088) * Extend menu bar conditionals beyond usage percentages Conditional predicates could only compare four percent-used windows. They now compare 18 metrics across four units: percent windows, the direct primary/secondary/tertiary lanes, four reset countdowns, three pace deltas, run-out, credit balance, and today/30-day cost. Metrics with two readings (percent windows, lanes, balance) gain a used/remaining select, so "session > 50% used and session resets in < 2h" is expressible. Pace, run-out, balance and cost were only carried as display strings, which cannot be compared, so MenuBarLayoutRenderMetrics carries their numeric twins pre-rounded to the same granularity as the text they mirror. Three refresh gates needed widening for the new data dependencies: - The title cache key had no component that moves with the clock, so a countdown predicate would have served its pre-flip title indefinitely. It now keys on the per-conditional outcome, evaluated once per render. - The four observation signatures gated on display tokens; a predicate on cost or balance has no token. They now also read the conditionals' metrics, which additionally fixes lane tokens inside conditional branches being invisible to the lane signature. - A reset-countdown predicate flips at an instant nothing else ticks on, so the countdown scheduler wakes at `resetsAt - threshold`. The conditional library is now decoded element-wise: this change makes forward-incompatible metric values possible for the first time, and one unknown value would otherwise have wiped the whole library on a downgrade. Ships an "Auto % / Resets in" default that renders the automatic percentage while the lane has headroom and the reset countdown once it is spent. * Sign the readings conditional predicates actually compare Three observation-signature gaps let a predicate flip without a redraw: - Cost signatures recorded only the currency-formatted string, so two token-cost updates could cross a threshold while both formatted to the same cent. A referenced cost metric now signs the unrounded amount losslessly. - The balance signature recorded only the rendered "Remaining" row, so a `balance used` predicate — which reads the "Used" row no token surfaces — was entirely unsigned. Both amounts are now signed. - The lane signature recorded the displayed reading, which follows `usageBarsShowUsed` and clamps remaining at zero, while `RateWindow.usedPercent` deliberately preserves over-quota values. A used-direction predicate such as `primaryLane > 105%` could move 104% -> 106% against a constant `0.000`. The lane signature is now scoped to what the layout renders, and a new conditional-window signature covers what conditionals read: the raw used percent (which remaining derives from, so it covers both directions) plus `resetsAt`, which countdown predicates depend on and no display token contributes. * Tick clock-derived predicates that no token schedules `menuBarWeeklyPaceRefreshDelays` is gated on a placed `.pace(.weekly)` token and only wakes once, at the pace-eligibility boundary. Excluding `runsOutIn` from the conditional reset schedule on the assumption that scheduler covered it therefore left a hole: a layout whose only pace or run-out reference is a predicate got no clock wake-up at all, so it kept rendering the branch that was true when the value last moved. Referenced weekly-pace predicates now also trigger the eligibility wake-up, and any referenced pace or run-out predicate schedules a minute tick. Both numbers are pre-rounded to the granularity the menu bar shows -- whole percentage points and whole minutes -- so a minute tick is exactly enough, and it is the cadence a `.resetCountdown` token already costs. Money predicates deliberately schedule nothing: they move only when new provider data arrives. * Keep older releases' conditional libraries readable on downgrade Decoding the library element-wise only helps builds that already have the lenient decoder. The build a user actually downgrades to decodes `menuBarLayoutConditionals` strictly and falls back to `[]`, so one saved rule using a new metric would empty the entire library there. The conditional library now dual-writes the way layouts already do: `menuBarLayoutConditionalsV2` keeps full fidelity, and the original key keeps an older-readable projection. `loadLibrary` mirrors `preferredLayout` — the current key wins unless the legacy key disagrees with its own projection, which only happens when an older release wrote it, and that edit must survive. The projection drops an entry when any clause uses a metric outside the original four, and also when any clause uses a non-`.used` direction. The second case is the subtler one: an older release's synthesized decoder ignores the unknown `direction` key, so `session remaining > 80` would come back as `session used > 80` and render the opposite branch. A missing rule is visibly missing; an inverted one is not. * Drop cost metrics that could not be converted to USD `UsageFormatter.convertedCost` returns the source amount unchanged when it has no rate for the provider's currency, and both cost producers passed that value straight through as `costTodayUSD`/`cost30dUSD`. A `Cost today > 5 USD` rule would then compare, say, €6 against a $5 threshold and pick the wrong branch. Both producers now keep the amount only when the conversion actually landed in USD. Otherwise the predicate sees no value and evaluates false, which is the existing contract for a metric the provider does not report. The rendered text is untouched and still shows the provider's own currency. --------- Co-authored-by: Peter Steinberger <steipete@gmail.com> * Prevent RPC pipe writes from aborting after child exit (#3095) * fix: prevent RPC pipe writes from aborting after child exit * test: repin UsageFetcher codex identity anchor after pipe-write refactor * Publish live Grok tokens and xAI spend into Usage & Spend (#3085) * Publish live Grok tokens and xAI spend into Usage & Spend Enabled Grok and xAI now join the shared spend catalog instead of only inflating the unavailable denominator. xAI contributes vendor-metered daily USD from the Management API chart; Grok contributes local session tokens. SuperGrok credits and xAI prepaid balance stay quotas, not spend. * docs: add #3085 to changelog * Fix lint on Grok and xAI spend messages Wrap no-data copy under 120 characters and drop a redundant throws on the prepaid-balance mapping test. * Fix Grok/xAI spend publication, Today, and coverage Preserve xAI analytics failures as unavailable instead of known-zero spend, publish local Grok tokens when remote billing fails, pin Today to the current UTC/local day, and keep xAI history as a 30-day source. * Fix Grok and xAI spend edge cases * Fix OpenRouter localization test after #3086 #3086 scoped localizedProviderDetails to DeepSeek and z.ai, so generic OpenRouter details keep canonical English. The merge test still expected zh-Hant "API 金鑰" and failed macOS shard 0. * Align OpenRouter localization test with main Peter restored generic title/row L() localization in 84a4ca7 after #3086 scoped it away. The merge kept the canonical-English assertion from the earlier CI fix; match the restored shared catalog instead. * test: reconcile gatekeeper anchors and fingerprints with Grok/xAI spend clusters * test: include Grok and xAI in the cost-capable dashboard source contract --------- Co-authored-by: Peter Steinberger <steipete@gmail.com> * docs: credit #3085 and #3088 changelog entries * chore: finalize 0.54.0 changelog and bump build to 127 * docs: update appcast for 0.54.0 * chore: open 0.54.1 unreleased changelog section * Clarify five-hour quota wording in Simplified Chinese (#3070) * Clarify Simplified Chinese five-hour quota label * Derive Simplified Chinese session quota labels from duration --------- Co-authored-by: UNGETSU <ungetsu@UNGETSUdeMacBook-Air.local> * Fix agent session menu width (#3096) * fix(alibaba): resolve mainland Personal/Solo sec_token from the console shell (#3098) Mainland Personal/Solo Token Plan (cn-personal) fails with a 200 `BailianGateway.Login.NotLogined` body ("Alibaba Token Plan login required") even with fresh, valid aliyun cookies, because the request lacks the `sec_token` the OneConsole gateway requires. #2533 already forwards the token when present, but it was never resolvable for this path for two reasons: - The console shell only server-renders `window.ALIYUN_CONSOLE_CONFIG.SEC_TOKEN` for a genuine same-origin document navigation; a bare GET receives a token-less shell. Send the browser-navigation headers (Referer, Sec-Fetch-*, Accept-Language) so the shell includes the token. - The shell embeds it as an upper-case, unquoted key (`SEC_TOKEN: "..."`), but `extractSECToken` only matched the lower-case `secToken`/`sec_token` shapes. Add the `SEC_TOKEN` pattern. With both, the scraper resolves the Personal `sec_token`, the gateway returns real usage, and the mainland Personal/Solo card renders. Verified end-to-end on a real cn-personal account: `secTokenSource=resolved`, body `message=Success`, 5-hour/weekly windows populated (was "login required"). Adds AlibabaTokenPlanSECTokenScrapeTests covering the upper-case shell format, the existing lower-case shapes, and the no-token case. Refs #2500, #2349, #2370, #2533. Co-authored-by: LeoLin <leolin990405@gmail.com> * Add BigModel CN account balance to z.ai provider (#3109) * Add BigModel CN account balance to z.ai provider * Bound balance timeout, region-aware override validation, null-safe amounts --------- Co-authored-by: RunhuaHuang <RunhuaHuang@users.noreply.github.com> * docs: credit #3070 #3096 #3098 #3109 changelog entries * test: repair gatekeeper anchors after #3096 menu refactor --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Peter Steinberger <steipete@gmail.com> Co-authored-by: kiranmagic7 <kiranmagic@proton.me> Co-authored-by: Kiran Magic <262980978+kiranmagic7@users.noreply.github.com> Co-authored-by: William Mitchell <13799071+wdmitchelluk@users.noreply.github.com> Co-authored-by: Alec Gutman, Chip <44984861+Chipagosfinest@users.noreply.github.com> Co-authored-by: UNGETSU <44204222+YunyueLi@users.noreply.github.com> Co-authored-by: UNGETSU <ungetsu@UNGETSUdeMacBook-Air.local> Co-authored-by: KaranocaVe <39995482+KaranocaVe@users.noreply.github.com> Co-authored-by: Zhongyue Lin <101193087+LeoLin990405@users.noreply.github.com> Co-authored-by: LeoLin <leolin990405@gmail.com> Co-authored-by: Xigua <rh2897@columbia.edu> Co-authored-by: RunhuaHuang <RunhuaHuang@users.noreply.github.com>
Follow-up to #3076. Conditional predicates could only compare four percent-used windows (
session | weekly | scopedWeekly | automatic) against a0...100threshold. They now compare 18 metrics across four units, and metrics with two meaningful readings gain a used / remaining select — sosession > 50% used AND session resets in < 2horbalance remaining >= 5are expressible.Metrics
Session % · Weekly % · Scoped weekly % · Auto %%Primary % · Secondary % · Tertiary %%Session · Weekly · Scoped weekly · Auto resets inhSession · Weekly · Auto pace%(signed)Runs outhBalance · Cost today · Cost 30dUSDBalanceonlyTime thresholds are hours as a
Double, so "resets in < 2h" is literally2and 30 minutes is0.5— no extra unit picker. Currency thresholds are USD; a provider reporting another cost currency is converted throughUsageFormatter.convertedCost, so a threshold does not shift when the user changes their display currency. Pace is already signed, and a reset countdown or cost total has no complement, so those hide the direction picker.Numeric data behind the display strings
MenuBarLayoutRenderDatacarried pace, run-out, balance and cost only as formatted strings ("+11%","Runs out in 1d 16h","$12.34"), which cannot be compared. NewMenuBarLayoutRenderMetricscarries their numeric twins, pre-rounded to the same granularity as the text they mirror — an unrounded value drifts on every clock tick and would defeatMenuBarLayoutTitleCache, which keys on this struct.Sources: a
menuBarLayoutPaceDeltatwin ofmenuBarLayoutPaceText(identical parameter list, so text and number can never disagree), theUsagePacealready computed forrunsOut(now bound instead of discarded),menuBarLayoutCostsreplacingmenuBarLayoutCostStringswith strings and USD amounts from one snapshot read, andMenuBarLayoutBalanceResolver.balanceAmountsUSD.Three refresh gates the new metrics exposed
These are the load-bearing part of the change — without them a predicate can be correct and still render stale:
MenuBarLayoutRenderKeyhad no component that moves with the clock (resetTextderives only from the automatic window), so a countdown predicate would have served its pre-flip title indefinitely. The key now carries[UUID: Bool]conditional outcomes, evaluated once per render instead of once per placement.storedMenuBarLayout*Signaturebuilders gated on which display tokens the layout contains; a predicate on cost, balance, pace or run-out has no matching token. They now also readMenuBarLayout.referencedConditionalPredicates. This incidentally fixes a pre-existing gap whereselectedLanesnever walked conditional branches, so alanePercenttoken inside a branch was invisible to the lane signature.…ResetsInpredicate flips at an instant no display token ticks on, somenuBarConditionalResetDelayswakes atresetsAt - threshold. Run-out predicates are deliberately excluded: their estimate drifts with usage rather than crossing a fixed instant, and the existing pace delays already cover that lane.Persistence
MenuBarConditionalPredicategains an explicitinit(from:):directiondecodes as.usedwhen absent, so every already-persisted predicate keeps its original meaning. The synthesized decoder would have rejected all of them.normalized()clamps the threshold into the metric's unit range and drops a direction the metric cannot use, so switching metric families in the editor can never leave an out-of-range value.try? decode([MenuBarLayoutConditional].self) ?? []would have silently wiped the user's entire library on a downgrade. One unrecognized entry is now dropped on its own; layouts referencing it already render the dangling-conditional placeholder.New shipped default
Auto % / Resets in— renders the automatic percentage while the lane has headroom, and the reset countdown once it is spent (Auto % remaining >= 1 → Auto %, else Resets in). Its name is composed from the two palette token labels it switches between, so the chip reads in the same words as the tokens themselves in every language. Shipped defaults only seed on a fresh install, so an existing library is never reseeded.Localization
Four new keys (
..._used,..._remaining,..._metric_resets_in,..._metric_scoped_weekly) translated across all 23 catalogs. Every other metric label reuses an existing palette token string, so a metric and the block it measures always read the same.check-app-localespasses: 22 catalogs against 1476 English keys.Evidence
All 18 metrics in the picker
Percent metric — used/remaining select present,
%unitCountdown metric — direction select hidden, unit switches to
hThe headline case: two clauses, mixed units, live summary
If Session % used > 50% and Session resets in < 2h then Resets in else HideFresh install seeds the new
Auto % / Resets indefaultBoth branches of that default, same rule, live data
Automatic lane has headroom → percentage:
Automatic lane spent → reset countdown:
Tests
make checkclean (0 SwiftLint violations, 23/23 locale catalogs). New coverage inMenuBarLayoutRendererTests(55 tests) andMenuBarLayoutTests(54 tests):resets-in predicate picks the then branch inside the thresholdsession percent and resets-in combine with and— the headline caseremaining direction inverts the percent readingbalance direction selects used or remaining amountpace run-out and cost predicates read numeric metricspredicate on a metric with no datum evaluates falsetime based conditional flips when only the clock advances— regression guard for the cache keyshipped auto default swaps percent for the countdown once the quota is spentpredicate without direction decodes as usedthreshold clamps to the metric unit rangedirection is dropped for metrics without a complementreferenced conditional predicates include nested branchesunrecognized conditional metric drops only its own entrymetric drives the editor row controls and unitssummary spells out direction and unit for a mixed-unit conditionFull suite run in 8 shards. Remaining failures (
StatusMenuSwitcherRefreshTests,MenuCardViewRecyclingTests,AdaptiveRefreshTimerTestsload timeouts,CostUsageFetcher*hangs) were verified against a pristine-HEAD worktree on the same volume and reproduce byte-identically — the headless AppKit brittlenessAGENTS.mdwarns about, not this change.Review round-up
All actionable findings from both reviewers are fixed; ClawSweeper is at Findings: None on
659a8134.6dbabfc— a referenced cost metric signs the unrounded amount losslessly. The same gap existed for balance, which recorded only the rendered "Remaining" row while abalance usedpredicate reads "Used"; both are now signed.usageBarsShowUsedand clamps remaining at zero, soprimaryLane > 105%could move 104% → 106% against a constant0.0006dbabfc— the lane signature now covers only what the layout renders; a new conditional-window signature covers what conditionals read: the rawusedPercent(which remaining derives from, so it covers both directions) plusresetsAt, which countdown predicates depend on and no token contributes..pace(.weekly)token and only fires oncef2b4673— referenced weekly-pace predicates now trigger the eligibility wake-up, and any referenced pace/run-out predicate schedules a minute tick. Money predicates deliberately schedule nothing.[]8f62fbad— the library dual-writes like layouts already do:menuBarLayoutConditionalsV2keeps full fidelity, the original key keeps an older-readable projection, andloadLibrarymirrorspreferredLayout.convertedCostreturns the source amount when it has no rate, and both cost producers passed it through as USD659a8134— the amount is kept only when the conversion actually landed in USD; otherwise the predicate sees no value and evaluates false.Downgrade contract (flagged for owner acceptance)
The older-readable projection drops an entry when any clause uses a metric outside the original four or a non-
.useddirection. The second case is not a decode failure: 0.54.0's predicate uses synthesizedCodable, which silently ignores the unknowndirectionkey, sosession remaining > 80would come back assession used > 80and render the opposite branch. A dropped rule is visibly absent; an inverted one is not. Happy to change that trade if a best-effort translation is preferred.Added since the first review
StatusItemConditionalSignatureTests— over-quota used-direction lane, moved reset timestamp, sub-cent cost, unconvertible currencyMenuBarCountdownRefreshTests— predicate-only clock-derived scheduling (parameterised overrunsOutIn/weeklyPace/sessionPace/automaticPace), plus the negative cost case and the reset-instant caseMenuBarLayoutTests— four downgrade tests using aPreExpandedConditionalfixture that reproduces the 0.54.0 surface exactly, including that the old decoder throws on the current blobRebased onto
5bbf1773. The redswift-test-macos (0, 2)shard isgeneric provider details keep canonical labels alongside localized core metrics, which fails on pristinemainat HEAD — verified in a cleanorigin/mainworktree, zero changes applied.build-linux-musl-clifails in "Install Swift Static Linux SDK", before any source is compiled.