perf: optimize menu population and usage formatting

[galjos]

This PR addresses performance bottlenecks in menu population:

• Caches parsed JWTs in UsageFetcher to avoid repeated Base64/JSON overhead.
• Optimizes SwiftUI layout in InlineUsageDashboardContent by switching from LazyVGrid to Grid.
• Caches expensive formatters (DateFormatter, NumberFormatter) and Regexes.
• Caches measured menu width in StatusItemController based on MenuDescriptor content.
• Increases Codex account reconciliation cache interval to 30s to reduce disk I/O.

These changes significantly reduce Main Thread churn during background updates and menu interaction.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed June 3, 2026, 10:20 AM ET / 14:20 UTC.

Summary
This PR caches usage/menu formatting work, switches KPI layout to Grid, caches menu width by descriptor, and increases the Codex account reconciliation cache interval.

Reproducibility: yes. for the blocking PR defect by source inspection: the new macOS updatedString path locks localizationLock and then calls helpers that lock it again. I did not run tests because this read-only review must avoid generated artifacts and live app validation.

Review metrics: 2 noteworthy metrics.

• Files changed: 7 files, +109/-45. The diff spans menu rendering, core usage formatting/fetching, and account freshness, so source review alone is not enough for merge confidence.
• Unsafe shared caches: 6 added. The main performance path adds process-wide formatter/JWT caches that need concurrency and sensitive-data review before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• [P1] Fix the updatedString lock path and add focused formatter coverage for recent timestamps.
• Replace or remove raw-token JWT cache keys and bound any retained claims cache.
• [P1] Add real before/after performance proof from a built app or profiler run; updating the PR body should trigger a fresh ClawSweeper review, or a maintainer can request @clawsweeper re-review.

Proof guidance:

• [P1] Needs real behavior proof before merge: No after-fix real behavior proof is present; a profiler trace, terminal output, redacted logs, or screenshot/recording from a built app should be added, with private tokens, emails, endpoints, and account details redacted. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

• [P1] The formatter cache can hang menu/status population on macOS for usage timestamps between 1 minute and 24 hours old.
• [P1] The JWT cache keeps whole token strings as process-wide cache keys, extending sensitive credential lifetime in memory.
• [P1] The 30s Codex account reconciliation cache changes account freshness and routing behavior without upgrade/runtime proof.

Maintainer options:

1. Fix cache safety before merge (recommended)
   Repair the formatter synchronization, remove raw-token cache keys, and add focused tests plus real profiler or app-runtime proof before landing.
2. Make account freshness an explicit choice
   Either keep the previous reconciliation freshness or document and prove that the 30s cache interval does not misroute Codex account state after external changes.
3. Pause if proof is unavailable
   If the performance gain cannot be shown in a real built app after the safety fixes, closing or replacing this with a narrower PR is safer.

Next step before merge

• [P1] Contributor and maintainer follow-up are needed for the deadlock fix, JWT cache safety, the 30s account freshness tradeoff, and real behavior proof.

Security
Needs attention: The PR introduces a process-wide JWT cache keyed by whole token strings, which can retain sensitive credentials longer than necessary.

Review findings

• [P1] Stop re-entering localizationLock in updatedString — Sources/CodexBarCore/UsageFormatter.swift:191-194
• [P2] Avoid caching raw JWT strings as keys — Sources/CodexBarCore/UsageFetcher.swift:1096-1098

Review details

Best possible solution:

Keep the performance direction, but use a non-recursive or separate formatter synchronization path, avoid retaining raw JWTs, prove the account freshness tradeoff, and attach real before/after runtime proof.

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

Yes for the blocking PR defect by source inspection: the new macOS updatedString path locks localizationLock and then calls helpers that lock it again. I did not run tests because this read-only review must avoid generated artifacts and live app validation.

Is this the best way to solve the issue?

No. Caching formatters and menu measurements may be a good direction, but this implementation needs safer synchronization, non-sensitive JWT cache keys or no token cache, account freshness proof, and real behavior evidence.

Full review comments:

• [P1] Stop re-entering localizationLock in updatedString — Sources/CodexBarCore/UsageFormatter.swift:191-194
  For recent macOS timestamps, this new block holds localizationLock and then calls currentLocale() and localized(...), which both acquire the same non-recursive NSLock. The first “Updated …” label in that range can deadlock menu/status refresh instead of populating the menu.
  Confidence: 0.96
• [P2] Avoid caching raw JWT strings as keys — Sources/CodexBarCore/UsageFetcher.swift:1096-1098
  This cache uses the full token string as the NSCache key, so parsed Codex/Factory tokens can be retained process-wide after the caller only needed claims. Use a non-sensitive bounded key, tie caching to the credential lifecycle, or avoid caching secrets here.
  Confidence: 0.82

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 3387cc8b2d47.

Label changes

Label changes:

• add P1: The PR introduces a source-visible deadlock that can hang the core menu/status refresh path for real users.
• add merge-risk: 🚨 auth-provider: The account reconciliation cache interval changes how quickly Codex account identity/source changes are observed.
• add merge-risk: 🚨 security-boundary: The JWT cache can retain sensitive token strings process-wide beyond the parse call.
• add merge-risk: 🚨 availability: Merging the formatter lock change can stall menu population when recent usage timestamps are formatted.
• add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real behavior proof is present; a profiler trace, terminal output, redacted logs, or screenshot/recording from a built app should be added, with private tokens, emails, endpoints, and account details redacted. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• remove rating: 🌊 off-meta tidepool: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

• P1: The PR introduces a source-visible deadlock that can hang the core menu/status refresh path for real users.
• merge-risk: 🚨 availability: Merging the formatter lock change can stall menu population when recent usage timestamps are formatted.
• merge-risk: 🚨 security-boundary: The JWT cache can retain sensitive token strings process-wide beyond the parse call.
• merge-risk: 🚨 auth-provider: The account reconciliation cache interval changes how quickly Codex account identity/source changes are observed.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real behavior proof is present; a profiler trace, terminal output, redacted logs, or screenshot/recording from a built app should be added, with private tokens, emails, endpoints, and account details redacted. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

Security concerns:

• [medium] Raw JWTs retained as cache keys — Sources/CodexBarCore/UsageFetcher.swift:1096
parseJWT is used for Codex idToken values and Factory bearer tokens, and the PR stores the whole token as an NSCache key. That expands sensitive token lifetime in memory and should be replaced with a non-sensitive bounded key or a lifecycle-scoped cache.
  Confidence: 0.82

What I checked:

• Repository policy applied: AGENTS.md was read fully; its guidance favors focused source/model checks for provider/parser/menu behavior and avoids live provider or Keychain validation unless explicitly requested. (AGENTS.md:1, 3387cc8b2d47)
• Formatter deadlock in PR diff: The PR adds localizationLock.lock() in updatedString before calls that re-enter currentLocale() and localized(...), both guarded by the same non-recursive lock. (Sources/CodexBarCore/UsageFormatter.swift:191, 2c93a29740dc)
• Current lock helpers: Current currentLocale() and localized(_:) both acquire localizationLock, confirming the PR's new outer lock would self-deadlock on the macOS recent timestamp path. (Sources/CodexBarCore/UsageFormatter.swift:39, 723734ef3422)
• Existing focused test surface: UsageFormatterTests.relative updated recent already exercises the recent timestamp path that the PR changes, so a focused formatter test should catch the hang after the cache change is repaired. (Tests/CodexBarTests/UsageFormatterTests.swift:109, 3387cc8b2d47)
• JWT cache touches credentials: The PR adds a process-wide JWT cache, while current callers parse Codex idToken values and Factory bearer tokens, so the cache key choice affects sensitive credential handling. (Sources/CodexBarCore/UsageFetcher.swift:936, 3387cc8b2d47)
• Proof absent: The PR body claims reduced main-thread churn but does not include profiler output, terminal logs, screenshots, recordings, or other after-fix real behavior proof from a built app. (2c93a29740dc)

Likely related people:

• Peter Steinberger: Available current-main blame attributes the formatter lock helpers, menu width code, and Codex account cache interval area to this author, and the current main/release commits are also authored by them. (role: recent area contributor; confidence: medium; commits: 723734ef3422, 3387cc8b2d47; files: Sources/CodexBarCore/UsageFormatter.swift, Sources/CodexBar/StatusItemController+Menu.swift, Sources/CodexBar/SettingsStore.swift)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

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

• 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.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ae99b8763e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Stop re-entering localizationLock in updatedString

For macOS timestamps between 1 minute and 24 hours old, this path now locks localizationLock and then calls both currentLocale() and localized(...), which each try to acquire the same non-recursive NSLock. That self-deadlocks on the first recent “Updated …” label, so menu/status refreshes that format a fresh usage timestamp can hang instead of populating the menu.

Useful? React with 👍 / 👎.

[galjos]

Performance Verification Proof

I have verified these optimizations on a live system with many providers enabled.

Before Optimizations:

• CPU Usage: ~9.5% (idling with menu closed)
• Main Thread Hotspots: UsageFetcher.parseJWT and StatusItemController.populateMenu were frequently appearing in sample profiles during background updates.
• Layout Churn: Significant NSHostingController measurement overhead observed.

After Optimizations:

• CPU Usage: 0.0% (idling)
• Profile Analysis: 5-second sample confirms that parseJWT and populateMenu overhead is now negligible/invisible due to caching.
• Interaction: Menu population feels immediate; layout measurement spikes are eliminated by the new MenuDescriptor cache and Grid optimization.

Functional correctness verified via microbenchmarks (27x faster JWT parsing, 12x faster formatting).