Avoid closed menu rebuilds during data refresh#1291
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 3, 2026, 12:21 PM ET / 16:21 UTC. Summary Reproducibility: yes. there is a high-confidence source reproduction path: current main routes stale store observations into invalidateMenus with allowStaleContentDuringDataRefresh: true, then still prepares closed attached menus when no menu is open. I did not run a live macOS profiler in this read-only review. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the narrow menu-refresh change after CI or a full-Xcode run confirms the focused StatusMenu tests on the exact PR source. Do we have a high-confidence way to reproduce the issue? Yes, there is a high-confidence source reproduction path: current main routes stale store observations into invalidateMenus with allowStaleContentDuringDataRefresh: true, then still prepares closed attached menus when no menu is open. I did not run a live macOS profiler in this read-only review. Is this the best way to solve the issue? Yes, the PR is the narrowest maintainable shape I found: skip closed preparation only for stale data-refresh invalidations and compare delayed tasks against the captured required rebuild version. The remaining validation need is exact-source CI/full-Xcode test confirmation, not a different implementation. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 3387cc8b2d47. Label changesLabel changes:
Label 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
|
85f952a to
c0d5952
Compare
|
Added after-fix idle @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Summary
Evidence
sampleof CodexBar 0.32.4 showed CPU underStatusItemController.rebuildClosedMenuIfNeeded->populateMenu-> SwiftUI menu card height measurement while the menu was closed.After-fix profiler proof
c0d5952a04eb60860747436724b84481c89cf98blocally withswift build --product CodexBar.#Previewdeclarations fromKeyboardShortcutsand replaced SwiftUI@Entrywith the equivalentEnvironmentKeyimplementation. The app/menu refresh code under review was unchanged, and the working tree was restored afterward..build/arm64-apple-macosx/debug/CodexBar -debugDisableKeychainAccess YES -openAIWebAccessEnabled NO -refreshFrequency oneMinute -debugLogLevel debug.oneMinuterefresh cadence with the menu closed, then capturedsample <pid> 20 -file pr1291-afterfix-idle-after-refresh.sample.txt.0.0%before and after the sample window, and the sample contained zero occurrences of the pre-fix hot-path symbols:rebuildClosedMenuIfNeeded: 0populateMenu: 0NSHostingController: 0sizeThatFits: 0closed menu rebuild completed: 0Tests
git diff --checkswift test --filter StatusMenuTestsattempted, but local Command Line Tools environment failed while compiling dependencyKeyboardShortcutsbecausePreviewsMacrosfor#Previewwas unavailable.swift build --product CodexBarpassed for the local proof build with the macro-only compile workaround described above.Local limitations
xcode-select -pis/Library/Developer/CommandLineTools; this machine has no/Applications/Xcode*.app, andhome-mac-mainalso has no full Xcode install.