refactor: use NSStatusItem popover shell#296
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
@coderabbitai review |
|
@codex review |
|
@greptileai review |
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![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: 740e90c1b8
ℹ️ 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".
|
|
||
| runtime.presentQuickAction(.search) | ||
| showMenuBarWindow(nil) | ||
| statusPopoverController?.show(nil) |
There was a problem hiding this comment.
When Cmd-K/search is invoked while the prewarmed popover is closed and the DB is ready, runtime.presentQuickAction(.search) is published before this show call. Because the hosting view was already loaded, BrainBarWindowRootView can consume and clear the request while its text field still has no window; the async focus attempt then sees nsView.window == nil, and the popover opens without the command bar focused. The capture path has the same ordering, so route the request after the popover is visible or defer focus until the popover window exists.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 740e90c. Configure here.
| values: stats.recentEnrichmentBuckets, | ||
| size: NSSize(width: 22, height: 12), | ||
| accentColor: livePresentation.accentColor | ||
| ) |
There was a problem hiding this comment.
Status item brain icon replaced entirely by sparkline
Medium Severity
The renderStatusIcon method overwrites button.image with only the sparkline, completely losing the brain icon set in configureStatusItem. The removed BrainBarMenuBarLabel displayed both a brain icon AND the sparkline side-by-side via an HStack. Once the collector publishes stats, the status item loses its app identifier and shows only a tiny 22×12 sparkline chart, which is a visual regression for the default .menuBarWindow mode.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 740e90c. Configure here.
1 participant
Summary
MenuBarExtrashell with a prewarmed AppKitNSStatusItem+ transientNSPopoverwrapper.BrainBarWindowRootViewinsideNSHostingController; no dashboard/content rewrite.StatsCollectorwired toBrainBusClientfrom item feat(engine): Think/Recall/Sessions intelligence layer #5.Stacking
Stacked on Phase D IPC foundation / Lane B item #5 via
fix/brainbar-hybrid-parity. This assumes PR #295'swatch-brain-buspush path is present.Responsiveness Notes
NSStatusBar.system.statusItem(withLength: NSStatusItem.variableLength).popover.show(...)against already-loaded content.yesworkers around the prewarm XCTest. It passed, but this is not a real pointer-click p95 measurement; a human stage-demo click test is still the right final proof.No UI Polling
MenuBarExtrascene remains inBrainBarApp.BrainBusClient()so dashboard state remains push-driven overwatch-brain-bus.Test Plan
swift test --package-path brain-bar --filter BrainBarStatusPopoverControllerTestsinitially failed becauseBrainBarStatusPopoverControllerandBrainBarAppSupport.makeStatsCollectordid not exist.swift test --package-path brain-bar --filter BrainBarStatusPopoverControllerTests-> 2 tests passed.yes >/dev/nullworkers +swift test --package-path brain-bar --filter BrainBarStatusPopoverControllerTests/testControllerPrewarmsVariableLengthStatusItemPopover-> passed.swift test --package-path brain-bar-> 362 tests passed, 0 failures.Review Caveat
Local
cr review --plaincould not start because CodeRabbit CLI was rate-limited externally.Note
Medium Risk
Moderate risk because it changes the primary menu bar UI surface and toggle/show routing, which could affect app visibility/interaction and lifecycle cleanup despite largely reusing existing SwiftUI content.
Overview
Replaces the SwiftUI
MenuBarExtra-based menu bar shell with an AppKitNSStatusItem+ transientNSPopoverwrapper that is prewarmed at launch and hosts the existingBrainBarWindowRootView.Updates
AppDelegateto create/stop the newBrainBarStatusPopoverControllerin.menuBarWindowmode and route toggle/search/capture/dashboard actions through it, and factorsStatsCollectorconstruction intoBrainBarAppSupport.makeStatsCollectorto allow injection/testing while keepingBrainBusClientwired.Adds focused XCTest coverage to verify popover prewarming/configuration and that the collector factory requests the BrainBus event stream.
Reviewed by Cursor Bugbot for commit 740e90c. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Replace SwiftUI MenuBarExtra with NSStatusItem popover shell in BrainBar
MenuBarExtrascene with an imperativeBrainBarStatusPopoverControllerthat manages anNSStatusItemand a transientNSPopoverhostingBrainBarWindowRootView..menuBarWindowlaunch mode,AppDelegatenow creates aBrainBarStatusPopoverControlleron launch; quick actions (search, capture, dashboard) route to itsshow/toggleAPIs instead of focusing a menu bar window.BrainBarRuntime.$collectorstats.BrainBarAppSupport.makeStatsCollectorfactory and tests verifying popover prewarming and single-subscription behavior of the event stream.MenuBarExtrascene andBrainBarMenuBarLabelview are removed; menu bar integration is now fully imperative.Macroscope summarized 740e90c.