Add notch hover usage overlay - #3164
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed August 24, 2026, 4:22 AM ET / 08:22 UTC. ClawSweeper reviewWhat this changesAdds an opt-in macOS notch-hover dashboard for provider usage and agent sessions, with persisted layout, visibility, sizing, shortcut, localization, documentation, and regression-test support. Merge readinessKeep open for maintainer product-direction review: this is a substantial new opt-in native surface, not a fix already present on main. The previously raised shortcut, lifecycle, accessibility, and credit-fallback concerns are addressed on the current head; no new blocking code defect was found. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherCodexBar collects provider usage snapshots and optional agent-session state, then presents them in native macOS surfaces. This PR adds a non-activating panel anchored to a built-in display’s camera notch and driven by the same data sources and settings store. flowchart LR
A[Provider usage snapshots] --> C[Overlay model]
B[Agent session state] --> C
D[Notch settings and shortcut] --> E[Overlay controller]
F[Built-in display notch] --> E
C --> E
E --> G[Non-activating hover panel]
G --> H[Usage tiles and session band]
Decision needed
Why: The branch introduces a new persistent macOS UI mode and settings surface rather than repairing an established contract. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: If sponsored, retain the default-off design and land this as the supported notch presentation surface with its focused regression coverage and native-display proof. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR proposes a new opt-in capability rather than reporting a broken existing behavior. Is this the best way to solve the issue? Unclear: the implementation is internally coherent and default-off, but only a maintainer can decide whether this notch-specific UI belongs in CodexBar’s supported product scope. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against fa50cf2dcdb6. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (4 earlier review cycles)
|
![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: d2f342ed97
ℹ️ 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".
d2f342e to
74e3b41
Compare
|
Both review findings are fixed in f3b6fec, and the branch is rebased onto current main (merge state is clean now):
|
|
Fixed in ab83688: all three bar factories (
|
clawsweeper
Bot
added
rating: 🐚 platinum hermit
steipete
left a comment
steipete
left a comment
There was a problem hiding this comment.
Really nice work overall — the self-reported sizing approach and the no-global-monitor/no-AX design are exactly right, and the focused suites plus make check all pass on a merge with latest main. Four findings before this can land, two of them blocking:
P1 — Disabled feature still registers a global shortcut. NotchUsageOverlayController.swift:54 installs the KeyboardShortcuts handlers unconditionally, and the dependency registers any saved shortcut system-wide the moment a handler is attached. Disabling the overlay never unregisters it, and PreferencesNotchPane.swift:38 allows recording a shortcut while the feature is off. That breaks the off-by-default guarantee and can steal shortcuts from other apps. Handlers should only exist while the overlay is enabled.
P1 — Any settings change destroys an expanded overlay. The activation observation (NotchUsageOverlayController.swift:123) reads a computed property backed by the whole observable defaults struct, so unrelated preference changes re-fire it; at line 143 an existing panel is preserved only when collapsed — an expanded panel is closed and recreated, losing hover/shortcut state. Please narrow the observation to the fields that actually affect activation, and preserve an expanded panel across benign changes.
P2 — Controller restarts accumulate shortcut handlers. Handlers registered at line 71 are never removed on stop; each stop/start adds another callback, and in Toggle mode two callbacks make one press open-and-immediately-close the panel. The dependency ships KeyboardShortcuts.removeHandler(for:) for exactly this lifecycle.
P2 — Codex credits aren't actually a fallback. NotchUsageOverlayModel.swift:191 appends monthly credits whenever fewer than four bars exist, even when the extra window/spend bar was already chosen — a Codex account with two ordinary windows shows both, contradicting the documented mutually-exclusive fallback. Gate the append on the fallback slot being unfilled.
Happy to merge once these are addressed — the feature itself is a great addition.
4521c61 to
8fb9030
Compare
|
Addressed all four findings in 8fb9030 (rebased onto current main):
Verification on the rebased head:
@clawsweeper please re-review the current head. |
2 participants
Summary
Adds an opt-in notch hover usage overlay with its own Settings → Notch page. Off by default.
Grid), and are drag-to-reorder in settings. Providers are opt-out: newly enabled providers appear without revisiting the pane.Design notes
NSPanelat.statusBarlevel with two frames; hover detection is anNSTrackingAreaon its own content view. The collapsed hit area is exactly the camera housing, so menu extras that sit flush against it stay clickable.NotchGridHeightKey/NotchBandHeightKey); the controller sums the two reports, each clamped by its ceiling. Measuring a detached copy drifted from what SwiftUI actually laid out — that approach is gone. The frame re-applies underwithObservationTracking, so late-arriving snapshots resize the open panel.itemsis typed to provider rows and the session band is a separateSessionsBandvalue, so “the band never joins the grid” is guaranteed by construction rather than by convention.docs/notch-usage-summary.md.Screenshots
Expanded overlay — 3 columns, matched row heights, agent-session band pinned below (real data, debug build on a notched MacBook):
Settings → Notch — overlay, shortcut, agent-session, and provider sections:
Behaviour verified live on a notched display: the panel is content-sized (356×741 at 1 column → 875×283 at 3 columns), the provider cap alone changes the panel (337 → 488 for 200 → 600), a capped grid scrolls while the band stays pinned, and both
above/belowplacements render full width across the configured columns.Commands run
Notes for review
notchUsageSummaryEnabled,notchColumnCount,notchMatchesRowHeights,notchProvidersMaxHeight,notchSessionsMaxHeight,notchItemOrder,notchHiddenProviders,notchShowsAgentSessions,notchSessionsPlacement,notchHotkeyMode, plus theshowNotchOverlayshortcut name.testSettingsStorefixtures andNoop*token stores..onDrag/.onDropwith a plain-text payload (a customTransferableUTI can’t round-trip without an Info.plist declaration, which SwiftPM debug builds don’t have). The reorder logic itself is unit-tested; a manual drag check on the packaged app is welcome.