Make merged-Overview provider limit user-configurable#2339
Conversation
Was hardcoded to 3 (docs/ui.md, issue steipete#2107). Now backed by UserDefaults (static var, since it's used as a default-parameter value elsewhere and Swift disallows self in those) with a Settings stepper (1-12, default 3 for no behavior change).
![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: 3017da8f53
ℹ️ 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".
| private static let maxOverviewProviders = SettingsStore.mergedOverviewProviderLimit | ||
| private static var maxOverviewProviders: Int { SettingsStore.mergedOverviewProviderLimit } | ||
|
|
||
| @AppStorage("mergedOverviewProviderLimit") private var overviewProviderLimitSetting = 6 |
There was a problem hiding this comment.
When this setting changes after the merged Overview menu has already been rendered, only UserDefaults.standard is updated. StatusItemController invalidates/rebuilds menu content via observation of settings.menuObservationToken, and that token never reads this new key, so menuNeedsUpdate can consider the existing menu fresh and keep showing the old number of Overview rows until an unrelated observed setting/store change or app restart. Please route the limit through SettingsStore/defaultsState and include it in menu observation, or otherwise invalidate the merged menu when the value changes.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Codex review: needs real behavior proof before merge. Reviewed July 19, 2026, 12:45 PM ET / 16:45 UTC. Summary Reproducibility: yes. at source level: open a merged Overview menu, change the new stepper, and reopen or refresh the menu without another observed settings change; the added Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: If a maintainer sponsors a configurable cap, model it in Do we have a high-confidence way to reproduce the issue? Yes, at source level: open a merged Overview menu, change the new stepper, and reopen or refresh the menu without another observed settings change; the added Is this the best way to solve the issue? No. The direct Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3b4d9a85cbec. 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
|
1 participant
Summary
docs/ui.md, issue Allow more than 3 providers in the Overview tab (compact density mode) #2107). This makes it user-configurable via a Settings stepper (1-12), backed byUserDefaults.main, which already raised the hardcoded default from 3 to 6 — kept that as the new configurable default so existing users see no behavior change.Test plan
swift build --product CodexBar -c debugsucceeds