Skip to content

Menu bar: optional color-coded usage icon (off by default) - #2557

Closed
johnlarkin1 wants to merge 6 commits into
steipete:mainfrom
johnlarkin1:upstream-pr/menu-bar-usage-colors
Closed

Menu bar: optional color-coded usage icon (off by default)#2557
johnlarkin1 wants to merge 6 commits into
steipete:mainfrom
johnlarkin1:upstream-pr/menu-bar-usage-colors

Conversation

@johnlarkin1

Copy link
Copy Markdown
Contributor

Tints the meter icon green → amber → red as usage rises. One toggle in Settings ▸ Menu Bar ▸ Icon, off by default.

This is a narrower, rebuilt version of the idea in #1207, split down to icon rendering only.

Addressing the #1207 review

Prior blocker This branch
changed the existing-user colour default New key menuBarUsageColorsEnabled, absent-means-false. No migration, no registerDefaults. Existing installs render byte-identical icons.
template-image tint path that does not solve the reported macOS behaviour No contentTintColor anywhere. A template image keeps only its alpha mask and is recoloured by AppKit — which is exactly why tinting one does nothing on macOS 26. When tinted, the geometry is drawn in the colour and the bitmap marked non-template, the same approach as the existing quotaWarningFlashImage.
treats every secondary quota as weekly No time window is introduced. The tint is a function of the primary bar value already produced by resolvedMenuBarIconPercents, which is provider-aware today. No weekly, no secondary in this diff.
unlocalized controls Two keys across all 23 catalogs; check-app-locales.mjs clean.

Why there is no appearance observer

The palette is fixed sRGB, not NSColor.system*. Since baseFill becomes the tint outright, no dynamic colour is ever resolved into the bitmap — not the track fill, not the stroke, not the status overlay (hence threading color: into drawStatusOverlay). The render is a pure function of its inputs, so there is no stale-bitmap-on-appearance-flip bug to fix, no KVO observer, and the cache key is honest. A test pins that invariant.

Works identically on macOS 14–26. No #available gates.

Scope boundaries

Known trade-off

A non-template image does not invert under the status-item highlight — same as quotaWarningFlashImage today. Fine against macOS 26's translucent capsule, less crisp on 15's solid fill. Off by default, one toggle to revert.

Verification

swift build and node Scripts/check-app-locales.mjs clean against this branch on current main. Tests cover the tint ramp, the non-dynamic-colour invariant, and the off-by-default no-op.

The ramp's anchors are green 0.14 → orange 0.85 → red 0.80, so red dips slightly on the orange → red leg; the test asserts monotonic green end-to-end and monotonic red only on the green → orange leg. Palette values were picked for legibility against light, dark, and translucent menu bars rather than to form a monotonic ramp — say the word if you'd rather they were tuned differently.

johnlarkin1 and others added 6 commits August 1, 2026 15:39
New `menuBarUsageColorsEnabled` setting, absent-means-false, tints the
meter icon from green through amber to red as usage rises.

A template image keeps only its alpha mask and is recolored by AppKit,
which is why tinting one through `contentTintColor` has no effect on
macOS 26. When a tint is active the geometry is drawn in that color and
the bitmap is marked non-template, matching what `quotaWarningFlashImage`
already does. The palette is fixed sRGB rather than the dynamic system
colors, so nothing appearance-dependent is baked in and the render stays
a pure function of its inputs.

Icon + Percent renders the provider brand logo through
MenuBarLayoutRenderer and is left untouched; the toggle is disabled in
that style.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t no-op

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`tint shifts from green toward red as usage rises` required red to be
non-decreasing at every step, but the ramp's anchors are green (0.14) →
orange (0.85) → red (0.80): red dips 0.05 on the orange → red leg. The test
has never run — MenuBarUsageTintTests.swift has not compiled since it was
added in e3f9c2d, so the assumption was never checked against the palette.
#26 fixed the compile error and CI aborted at group 44/95 before reaching
this suite, so it stayed hidden.

Keeping the per-step green assertion (green is the channel that carries
"toward red" across both segments) and the end-to-end red assertion, and
scoping the per-step red assertion to the green → orange leg. The palette is
deliberate and documented for legibility on light, dark, and translucent
menu bars, so the over-specified test is what gives.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels Aug 1, 2026
@clawsweeper

clawsweeper Bot commented Aug 1, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 1, 2026, 3:46 PM ET / 19:46 UTC.

ClawSweeper review

What this changes

Adds an off-by-default Settings toggle that tints meter-style menu-bar usage icons from green through amber to red as the provider’s primary usage rises.

Merge readiness

Blocked until real behavior proof is added - 5 items remain

Keep this PR open for maintainer review. It is a focused, off-by-default feature rather than an existing-behavior fix; the supplied patch appears to preserve current users’ default rendering, but the added persistent visual mode needs product sponsorship and real native-app proof before merge.

Priority: P3
Reviewed head: 71334e600cfd8d982c1fbd0ae580e54794dba513
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The supplied patch is focused and has targeted test intent, but missing real macOS runtime proof is a merge gate for this UI change.
Proof confidence 🧂 unranked krab (1/6) Needs real behavior proof before merge: The PR reports builds, locale checks, and tests but provides no after-fix native-app screenshot, recording, terminal output, or redacted runtime log. Add redacted built-app proof, update the PR body, and a fresh review should run automatically; otherwise a maintainer can comment @clawsweeper re-review.
Patch quality 🦐 gold shrimp (3/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR reports builds, locale checks, and tests but provides no after-fix native-app screenshot, recording, terminal output, or redacted runtime log. Add redacted built-app proof, update the PR body, and a fresh review should run automatically; otherwise a maintainer can comment @clawsweeper re-review.
Evidence reviewed 6 items Repository UI-proof policy: The fully read repository policy asks PRs with UI changes to include screenshots or GIFs and says menu-bar runtime behavior should be validated against a freshly built bundle.
Current meter versus brand-layout boundary: Current main resolves usage values, takes the provider-brand route when enabled, and otherwise calls IconRenderer for a meter image. The supplied patch limits tinting to that meter path and leaves the brand layout untouched.
Current rendering refresh seam: Current main includes rendering-affecting preferences and resolved usage values in the icon observation signature. The supplied patch adds the new preference to that signature and adds corresponding focused coverage.
Findings None None.
Security None None.

How this fits together

CodexBar turns provider usage snapshots into either a meter icon or a provider-brand menu-bar layout. The status-item controller resolves usage and preferences, then sends the selected rendering inputs to the AppKit icon renderer and status-bar button.

flowchart LR
    A[Provider usage snapshot] --> B[Usage percent resolver]
    C[Menu bar preferences] --> D[Status item controller]
    B --> D
    D --> E{Brand icon layout enabled?}
    E -->|No| F[Meter icon renderer]
    E -->|Yes| G[Provider brand layout]
    F --> H[Status bar image]
    G --> H
Loading

Decision needed

Question Recommendation
Should CodexBar add a persistent, off-by-default color-coded meter-icon preference despite the non-template status-item highlight trade-off? Sponsor the opt-in setting: Accept the added preference once the contributor supplies redacted real-app proof that covers normal and highlighted menu-bar states.

Why: The proposed implementation is deliberately narrow, but deciding whether this additional user preference and its acknowledged visual trade-off fit the menu-bar product direction requires maintainer intent rather than a mechanical code repair.

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR reports builds, locale checks, and tests but provides no after-fix native-app screenshot, recording, terminal output, or redacted runtime log. Add redacted built-app proof, update the PR body, and a fresh review should run automatically; otherwise a maintainer can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - The enabled mode intentionally uses non-template RGB images, so status-item highlight behavior may be less legible than the existing template rendering; the PR describes the trade-off but does not show it in a real app.
  • Resolve merge risk (P1) - The target checkout cannot materialize the PR’s Git objects and GitHub DNS failed during read-only retrieval, so the branch-specific assessment relies on the supplied PR patches and context rather than a complete local three-way diff.
  • Complete next step (P2) - The remaining blocker is maintainer product acceptance of a new persisted visual mode, plus contributor-supplied real app proof before merge.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 35 files affected; 328 added, 12 removed Most changed files localize two new settings strings; the runtime work is concentrated in icon rendering, settings observation, and focused tests.
Live proof 0 artifacts supplied No after-fix screenshot, recording, terminal output, runtime log, or linked artifact is present in the supplied PR context.

Root-cause cluster

Relationship: canonical
Canonical: #2557
Summary: This is the active narrowed successor to the broader closed, unmerged color-icon proposal.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    If maintainers want this optional visual signal, keep the absent-means-false preference and meter-only boundary, then merge after redacted proof from a freshly built bundle demonstrates off/on behavior at representative usage levels and the highlighted status-item state.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

If maintainers want this optional visual signal, keep the absent-means-false preference and meter-only boundary, then merge after redacted proof from a freshly built bundle demonstrates off/on behavior at representative usage levels and the highlighted status-item state.

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

Not applicable: this PR proposes a new optional visual mode instead of reporting broken existing behavior. Current source clearly identifies the meter and brand-layout paths that require manual verification.

Is this the best way to solve the issue?

Unclear: constraining the tint to the meter path and preserving the default is a narrow technical approach, but accepting the new preference and its non-template highlight behavior is a product decision.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 78523f4ad890.

Labels

Label justifications:

  • P3: This is a reversible, off-by-default menu-bar appearance enhancement rather than a regression repair.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR reports builds, locale checks, and tests but provides no after-fix native-app screenshot, recording, terminal output, or redacted runtime log. Add redacted built-app proof, update the PR body, and a fresh review should run automatically; otherwise a maintainer can comment @clawsweeper re-review.

Evidence

What I checked:

  • Repository UI-proof policy: The fully read repository policy asks PRs with UI changes to include screenshots or GIFs and says menu-bar runtime behavior should be validated against a freshly built bundle. (AGENTS.md:20, 78523f4ad890)
  • Current meter versus brand-layout boundary: Current main resolves usage values, takes the provider-brand route when enabled, and otherwise calls IconRenderer for a meter image. The supplied patch limits tinting to that meter path and leaves the brand layout untouched. (Sources/CodexBar/StatusItemController+Animation.swift:303, 78523f4ad890)
  • Current rendering refresh seam: Current main includes rendering-affecting preferences and resolved usage values in the icon observation signature. The supplied patch adds the new preference to that signature and adds corresponding focused coverage. (Sources/CodexBar/StatusItemController+IconObservation.swift:24, 78523f4ad890)
  • Feature-history provenance: The current status-item controller file history includes the menu-bar layout editor feature introduced by the likely owner, establishing the central product surface for this new icon-mode proposal. (Sources/CodexBar/StatusItemController+Animation.swift:235, 0f070a38fa7f)
  • Related proposal: The PR body explicitly identifies this as a narrower, rebuilt successor to the closed unmerged color-icon proposal at Color-coded menu bar icons + separator style + time-window selection (macOS 26 aware) #1207, removing that proposal’s unrelated controls and default-behavior change.
  • Proof and branch-inspection limitation: The supplied PR body reports build, locale-check, and test work but contains no after-fix screenshot, recording, terminal output, runtime log, or linked artifact. The target checkout is clean and on main, but the PR commits are unavailable promisor objects and read-only retrieval failed because github.com DNS could not resolve. (78523f4ad890)

Likely related people:

  • steipete: Peter Steinberger introduced the current menu-bar layout/editor work and remains the strongest current-main history signal for the status-item and icon-rendering surface. (role: feature introducer and recent area contributor; confidence: high; commits: 0f070a38fa7f, 78523f4ad890; files: Sources/CodexBar/StatusItemController+Animation.swift, Sources/CodexBar/IconRenderer.swift, Sources/CodexBar/PreferencesMenuBarPane.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add redacted off/on screenshots or a short recording from a freshly built CodexBar bundle at representative usage levels.
  • Show the known non-template behavior while the status item is highlighted or selected.
  • Obtain maintainer confirmation that the additional preference and visual trade-off fit CodexBar’s menu-bar direction.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

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

@johnlarkin1

Copy link
Copy Markdown
Contributor Author

hey sorry - these are not ready and I will reopen when polished and I've done a review with solid evidence

@johnlarkin1 johnlarkin1 closed this Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v0.45.0 custom menu-bar layout renders provider icons as tiny white dots on a light menu bar

1 participant