Skip to content

Preserve stacked menu bar layout line breaks - #3094

Merged
steipete merged 3 commits into
mainfrom
codex-3089
Aug 20, 2026
Merged

Preserve stacked menu bar layout line breaks#3094
steipete merged 3 commits into
mainfrom
codex-3089

Conversation

@steipete

Copy link
Copy Markdown
Owner

Summary

Fixes #3089. Commit 1bb56b8 extracted leading icons into a separately positioned AppKit image (for native rendering/dimming). For stacked two-line layouts whose first row is icon-only, that stripped the row's content entirely — the icon was placed beside the second line's percentages instead of above them, collapsing the user's line break in both the status item and the editor preview.

Stacked layouts now keep icons embedded in the rendered lines; single-line layouts keep the extracted-native-image path (and its dimming behavior) unchanged.

Test plan

  • 101 tests across renderer/layout/editor suites, including new regressions: stacked icon-over-percent layout renders two lines; single-line layouts still extract the native image
  • make check clean

@clawsweeper

clawsweeper Bot commented Aug 20, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 20, 2026
@clawsweeper

clawsweeper Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed August 20, 2026, 4:02 AM ET / 08:02 UTC.

ClawSweeper review

What this changes

This PR keeps provider icons inline in two-line menu-bar layouts while preserving the native extracted-icon path for one-line layouts.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

Ready for maintainer review

Keep open for routine owner review: the focused patch directly restores stacked icon-row rendering and has no actionable correctness finding.

Priority: P2
Reviewed head: 5e6067152f05a5262fdbb8defeac17e062d74f01

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, well-covered regression repair with no correctness finding; routine owner review remains appropriate.
Proof confidence 🌊 off-meta tidepool Not applicable: This owner-authored PR is not subject to the external-contributor real-behavior-proof gate; focused source tests cover the two rendering branches.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This owner-authored PR is not subject to the external-contributor real-behavior-proof gate; focused source tests cover the two rendering branches.
Evidence reviewed 4 items Current-main defect path: On the base branch, a non-high-contrast stacked layout can extract its first icon as the native button image; the render loop then omits that inline token, which leaves the first attributed row empty.
Runtime consumers share the renderer: The status-item controller applies the renderer's optional native image and attributed title, while the editor preview consumes the same rendered title/image split; the change therefore covers both reported surfaces.
Focused regression coverage: New tests assert an inline attachment and preserved newline for icon-over-percent stacked layouts, and retain the extracted image assertion for one-line layouts.
Findings None None.
Security None None.

Live Verification

Command: swift test --filter MenuBarLayoutRendererTests

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

Assertions:

  • FAIL expect_output: Test run with

How this fits together

CodexBar turns a saved menu-bar token layout plus provider usage data into an attributed title and optional native AppKit image. That rendered result is used by both the live status item and the Preferences layout preview.

flowchart LR
A[Saved layout tokens] --> B[Layout renderer]
C[Provider icon and usage data] --> B
B --> D{One line or stacked}
D -->|One line| E[Native status icon]
D -->|Stacked| F[Inline attributed icon]
E --> G[Menu bar and editor preview]
F --> G
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Renderer regression coverage production +5/-6, tests +47 The patch is narrowly scoped and adds direct assertions for both the restored stacked path and preserved one-line behavior.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #3089
Summary: This PR explicitly targets the reported stacked-layout regression and its renderer change covers the reported menu-bar and preview behavior.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Technical review

Best possible solution:

Land the narrow renderer change after routine review so stacked layouts keep their configured row ownership while one-line layouts retain native icon dimming.

Do we have a high-confidence way to reproduce the issue?

Yes—source inspection shows that current main extracts an icon from a stacked first row and skips its inline rendering, while AppKit places the native image beside the whole title rather than above the second row.

Is this the best way to solve the issue?

Yes—the renderer is the shared seam for the live status item and preview, and the branch preserves the existing one-line native-image behavior instead of adding a parallel rendering path.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 84a4ca725d5d.

Labels

Label justifications:

  • P2: This is a bounded regression in custom menu-bar layout rendering and preview behavior.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This owner-authored PR is not subject to the external-contributor real-behavior-proof gate; focused source tests cover the two rendering branches.

Evidence

What I checked:

Likely related people:

  • steipete: Current main history shows repeated ownership of menu-bar layout rendering, vertical adjustment, and the status-item application path. (role: recent area contributor; confidence: high; commits: 7dcfd4143841, 5e6067152f05; files: Sources/CodexBar/MenuBarLayoutRenderer.swift, Sources/CodexBar/StatusItemController+MenuBarLayout.swift)
  • luantu: History attributes the native extracted-leading-icon and follow-up accessibility/preview behavior to this contributor. (role: introduced extracted-icon behavior; confidence: high; commits: 1bb56b8b5caa, de91ea976674; files: Sources/CodexBar/MenuBarLayoutRenderer.swift, Sources/CodexBar/MenuBarLayoutEditor.swift)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)
  • reviewed 2026-08-20T07:06:16.283Z sha 4e7c854 :: needs maintainer review before merge. :: none

@steipete
steipete merged commit 042fc9c into main Aug 20, 2026
9 checks passed
steipete added a commit that referenced this pull request Aug 20, 2026
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The latest update broke line breaks

1 participant