Skip to content

feat(omarchy): mark each provider in the bar with its own logo - #3787

Closed
jsonMartin wants to merge 4 commits into
steipete:mainfrom
jsonMartin:pr4-provider-logos
Closed

jsonMartin wants to merge 4 commits into
steipete:mainfrom
jsonMartin:pr4-provider-logos

Conversation

@jsonMartin

Copy link
Copy Markdown
Contributor

Summary

The bar identified providers with a two-letter tag. This draws the provider's own mark instead, recoloured to the bar's foreground so themes still apply.

Splitting the label is the part that needed care. barLabel built one joined string, and a frontend cannot safely split that back apart on a separator the lane text also uses. barSegments returns one tag-and-text pair per displayed provider and barLabel is built from it, so the two cannot drift — a test asserts that identity rather than trusting it. The pairs travel as barEntries beside barLabel, which the tooltip still uses and which a frontend falls back to when the backend predates this field.

The marks are the ones the Mac app already ships (Sources/CodexBar/Resources/ProviderIcon-*.svg). They are copied by glob, so no provider name is hard-coded anywhere and a provider without one keeps its text tag rather than leaving a gap. Both the installer and the release archive carry them; an archive without them would fall back to text with nothing explaining why. The archive's allowlist test now checks the fixed files plus the mark filenames rather than a member count, so it still fails if a checkout leaks in.

Depends on

This builds on #3786. GitHub's native stacked pull requests do not support
cross-fork stacks, so this is stacked manually: its base is the previous branch and it
will be retargeted to main once that merges. Reviewing in order is easiest.

A question for the maintainer

This puts ~70 brand SVGs (~300KB) into a new distribution path — the Quickshell plugin directory and the release archive. That is your call rather than mine, and I am happy to change the approach: ship a subset, resolve them from the CLI bundle instead, or drop the PR.

Screenshot

◉ 7D 83% · -4% · ✳ 5H 95% · 7D 19% · -11% · ∧ 5H 100% · 7D 94% · Z 5H 97% · 7D 94% · -6%

Commands run

node --test Integrations/Omarchy/test.mjs Integrations/Omarchy/notifications.test.mjs  # 54 pass
python3 Integrations/Linux/tests/test_desktop.py   # 14 tests, OK
python3 Integrations/Linux/tests/test_package.py   # OK
python3 Integrations/Omarchy/test_install.py       # OK
omarchy plugin validate Integrations/Omarchy       # exit 0

Known limitation

CI never loads Panel.qml, so logo rendering, the missing-mark text fallback and older-backend compatibility are verified by screenshot and by inspection, not by test.

…ones

Claude reports a cap scoped to one model beside its general weekly quota, as an
entry in usage.extraRateWindows. The Linux shared model read only primary,
secondary and tertiary, so that lane never reached the snapshot, the Omarchy
popup or any other adapter: an account could sit at 96% of a scoped cap with
nothing on screen saying so, while macOS showed it through
standardSemanticWindows, which already treats extras as candidates.

Read the extras after the three standard lanes. The ordering matters, because a
scoped lane can share the 7-day cadence with the general weekly window and
consumers resolve a cadence by first match; a test pins it rather than leaving it
to the reading.

The same pass fixes what that exposed. A provider also describes windows it
cannot measure: Zed reports an overdue invoice and Antigravity a reset-only pool,
both carrying a full usedPercent with usageKnown false, and Claude emits a
synthetic placeholder when its web API returns no session. Core excludes both.
Read as quota they became a full or exhausted lane, so filter them where the
percentage is first trusted and every consumer inherits the boundary.

Titles are provider-controlled text exported over an IPC contract that excludes
account identity, so they are redacted regardless of the identity display
preference, which governs the native windows rather than this boundary. The count
is bounded like the other provider-controlled lists here, counting lanes that
render so a real one trailing unusable entries is not discarded.
…e bar

The bar showed one quota per provider and no pace, so the weekly burn rate was
only visible by opening a window. Show each provider's session quota, weekly
quota and weekly pace instead, joined by the separator the popup already uses.

Nothing here recomputes pace. The CLI already emits a signed delta per window,
produced by Core, so the shared model carries it through and renders it in the
compact form of MenuBarDisplayText.paceText. The pace shown is the weekly
window's, never whichever lane is most constrained, and a pace Core cannot
compute contributes no segment rather than a placeholder that reads like data.

Lanes are resolved from the cadence a provider reports, the way Core's semantic
windows do, so no provider name is hard-coded and a provider without a session
window emits neither that segment nor its separator. A cadence resolves to the
pool that binds hardest, because a provider can report several at one cadence and
taking the first would hide an exhausted pool behind an idle one. Windows sharing
a duration are not assumed to be the same quota measured twice; a provider lists
its own headline quota first. A provider whose only lane at a cadence is a scoped
one still gets that cadence rather than showing the scoped lane by name.

Percentages run through the existing quota preference, so the bar follows the
used or remaining setting without the adapter inverting anything, and the
existing Show pace preference now governs the bar as well as the native cards.
Caps scoped to a single model are off by default behind a new preference, since
most providers that publish them restate a general lane.

The adapter also stopped reusing a previous poll's body, which let a backend that
had gone away keep reporting the quota it last had.
…reports

Three classification faults an independent review reproduced against real
provider output.

Antigravity emits one window per model family for a cadence, marked with a
quota-summary id, and its positional window is only whichever family it chose to
represent them. Preferring positional windows reported a 40% session while a 20%
one was present. Where a provider marks a summary set that way, the lane resolves
across it. Matching on the provider's marker rather than on percentages matters:
Claude reports several independently valued scoped caps, one can land on the same
rounded percentage as the general weekly window, and reading that coincidence as
a summary replaced the general quota with a cap scoped beneath it and dropped its
pace.

A window promoted to a cadence headline could then render a second time as a
scoped cap, because the scoped pass did not know what had already been shown.
A Kimi subscription-only account read "30D 20% · Total usage 20%".

The tray summary chose the smallest remaining percentage across every pool,
which is not the lane the bar leads with. For Cursor, which bills its total, its
Auto/Composer usage and its API usage over one cycle, the bar reported the
provider's own 75% while the tooltip reported the 10% subquota with no label
saying which. Both now name the same lane.
The bar identified providers with a two-letter tag. Draw the provider's own mark
instead, recoloured to the bar's foreground so themes still apply.

Splitting the label is the part that needed care. barLabel built one joined
string, and a frontend cannot safely split that back apart on a separator the
lane text also uses. barSegments returns one tag-and-text pair per displayed
provider and barLabel is built from it, so the two cannot drift. The cap on how
many providers appear lives inside barSegments rather than in barLabel, or a
frontend reading the entries would show providers the label said were hidden;
a three-provider test pins the providers, their order and the overflow count.
The entries travel as barEntries beside barLabel, which the tooltip still uses
and which a frontend falls back to when the backend predates this field.

Drawing marks hides the button's own label, which is what carried the overflow
count, so the count is drawn beside them, after the repeater rather than as a
second delegate inside it.

The marks are the ones the Mac app already ships. They are copied by glob, so no
provider name is hard-coded and a provider without one keeps its text tag rather
than leaving a gap. Both the installer and the release archive carry them, since
an archive without them would fall back to text with nothing explaining why. The
archive's allowlist test checks the fixed files and the mark filenames instead of
a member count, so it still fails if a checkout leaks in.

The badge sizes itself to whichever child it draws. Taking the larger of the mark
and the hidden text tag reserved the width of the provider id, which left a gap
beside the mark for any provider without a short tag.
@clawsweeper

clawsweeper Bot commented Sep 20, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 20, 2026
@clawsweeper

clawsweeper Bot commented Sep 20, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 20, 2026, 4:53 PM ET / 20:53 UTC.

ClawSweeper review

What this changes

The branch adds provider logos to the Omarchy bar, packages their SVG assets, and includes preceding stack changes for quota lanes, pace, and optional scoped caps.

Merge readiness

Blocked before merge - 5 items remain

This is distinct, useful work that neither current main nor v0.62.0 implements. Its open prerequisite does not supersede the logo feature.

Priority: P2
Reviewed head: 2d25c1d02689be02067f3839bbc074d48ae7022e
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The structured model and tests are useful, but an actual tinting defect and unsubstantiated rendering proof limit readiness.
Proof confidence 🦪 silver shellfish (2/6) Needs stronger real behavior proof before merge: The captured body provides a text illustration and test/validator results, but no observed Panel.qml rendering from Omarchy. Add redacted screenshots or a recording showing light/dark logo rendering and fallback behavior, plus evidence that installation works for fresh and upgraded setups. Redact account details, keys, IPs, and private endpoints. Updating the PR body should trigger review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The captured body provides a text illustration and test/validator results, but no observed Panel.qml rendering from Omarchy. Add redacted screenshots or a recording showing light/dark logo rendering and fallback behavior, plus evidence that installation works for fresh and upgraded setups. Redact account details, keys, IPs, and private endpoints. Updating the PR body should trigger review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 11 items Introduced scope and merge ownership: The pinned merge-base-to-head delta changes 12 files, including the prerequisite stack. The verified test merge has the exact main and PR-head parents and preserves the same 12-file scope; unrelated main-branch changes are not attributed to this PR.
Current main still uses text: The current main adapter renders snapshot.summary through WidgetButton and has no provider-logo renderer.
Release comparison: The supplied latest release, v0.62.0, also contains the text-only adapter.
Findings 1 actionable finding [P2] Tint logos by alpha instead of their original brightness
Security None None.

How this fits together

The Linux desktop backend converts provider CLI output into a private usage snapshot. The Omarchy adapter reads that snapshot and displays provider quotas in the desktop bar.

flowchart LR
  A[Provider CLI output] --> B[Linux usage model]
  C[Display preferences] --> B
  B --> D[Private desktop snapshot]
  D --> E[Omarchy bar adapter]
  F[Packaged provider logos] --> E
  E --> G[Logo or text fallback with quotas]
Loading

Decision needed

Question Recommendation
Should Linux archives and the Omarchy plugin bundle all existing provider SVGs? Bundle the existing marks: Approve the 70 tracked SVGs, about 96 KB uncompressed, to keep the adapter self-contained.

Why: The author explicitly leaves this distribution choice to the maintainer; existing macOS use does not establish approval for the new packaging path.

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: The captured body provides a text illustration and test/validator results, but no observed Panel.qml rendering from Omarchy. Add redacted screenshots or a recording showing light/dark logo rendering and fallback behavior, plus evidence that installation works for fresh and upgraded setups. Redact account details, keys, IPs, and private endpoints. Updating the PR body should trigger review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Tint logos by alpha instead of their original brightness (P2) - For a bundled black logo such as Qoder, MultiEffect.colorization multiplies the foreground colour by zero source luminance, so the mark stays black; dark Kilo and OpenCode assets are similarly dimmed. On a dark bar the provider becomes difficult or impossible to identify, and Image.Ready hides the text fallback anyway. Use an alpha-mask foreground tint or normalize the source luminance before colourization, and verify both black and white assets.
  • Resolve merge risk (P1) - The author’s requested approval for distributing the full logo set through Linux archives and the Omarchy plugin remains unresolved.
  • Complete next step (P2) - Correct the logo tinting and supply real Omarchy proof before merge.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

  • [P2] Tint logos by alpha instead of their original brightness — Integrations/Omarchy/Panel.qml:110-115
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +364/-18; tests +411/-1 The full introduced stack includes quota classification, backend wiring, and logo rendering, with corresponding model and integration coverage.
Packaged logo assets 70 SVGs; 95,997 uncompressed bytes This measures the distribution scope awaiting the maintainer’s decision.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Use an alpha-based foreground tint with reliable text fallback, within a maintainer-approved packaging approach that preserves existing display preferences.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Use an alpha-based foreground tint with reliable text fallback, within a maintainer-approved packaging approach that preserves existing display preferences.

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

Yes, source establishes the introduced rendering defect: select Qoder with a dark bar theme, and its black SVG remains black while the text fallback is hidden. No live QML run was performed.

Is this the best way to solve the issue?

Partly: structured provider segments and reuse of existing assets are sensible, but luminance-based colourization cannot reliably tint every bundled logo; an alpha-based tint is the narrower correction.

Full review comments:

  • [P2] Tint logos by alpha instead of their original brightness — Integrations/Omarchy/Panel.qml:110-115
    For a bundled black logo such as Qoder, MultiEffect.colorization multiplies the foreground colour by zero source luminance, so the mark stays black; dark Kilo and OpenCode assets are similarly dimmed. On a dark bar the provider becomes difficult or impossible to identify, and Image.Ready hides the text fallback anyway. Use an alpha-mask foreground tint or normalize the source luminance before colourization, and verify both black and white assets.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 4b86ea7b3971.

Labels

Label changes:

  • add P2: This is a bounded desktop-display improvement with a concrete logo-visibility defect.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The captured body provides a text illustration and test/validator results, but no observed Panel.qml rendering from Omarchy. Add redacted screenshots or a recording showing light/dark logo rendering and fallback behavior, plus evidence that installation works for fresh and upgraded setups. Redact account details, keys, IPs, and private endpoints. Updating the PR body should trigger review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a bounded desktop-display improvement with a concrete logo-visibility defect.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The captured body provides a text illustration and test/validator results, but no observed Panel.qml rendering from Omarchy. Add redacted screenshots or a recording showing light/dark logo rendering and fallback behavior, plus evidence that installation works for fresh and upgraded setups. Redact account details, keys, IPs, and private endpoints. Updating the PR body should trigger review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Introduced scope and merge ownership: The pinned merge-base-to-head delta changes 12 files, including the prerequisite stack. The verified test merge has the exact main and PR-head parents and preserves the same 12-file scope; unrelated main-branch changes are not attributed to this PR. (2d25c1d02689)
  • Current main still uses text: The current main adapter renders snapshot.summary through WidgetButton and has no provider-logo renderer. (Integrations/Omarchy/Panel.qml:43, 4b86ea7b3971)
  • Release comparison: The supplied latest release, v0.62.0, also contains the text-only adapter. (Integrations/Omarchy/Panel.qml:43, 4b3ed1a2a49a)
  • Introduced tinting defect: The new MultiEffect uses colorization alone and hides the text tag whenever the image loads. The installed assets include black Qoder and dark Kilo logos, so successful loading does not establish a visible foreground-coloured mark. (Integrations/Omarchy/Panel.qml:114, 2d25c1d02689)
  • Concrete black source asset: The Qoder asset explicitly uses fill="black" and is included by the new packaging glob. (Sources/CodexBar/Resources/ProviderIcon-qoder.svg:2, 2d25c1d02689)
  • Authoritative colourization behavior: Panel.qml directly imports QtQuick.Effects and uses MultiEffect, establishing the dependency contract. The Qt shader multiplies the requested colour by source luminance; a black source remains black at default brightness. See the pinned Qt shader. (src/effects/data/shaders/multieffect.frag:84, 35ba89a42a74)

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

Rank-up moves

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

  • Correct tinting for black, dark, and white SVG assets.
  • Provide real Omarchy rendering and fresh/upgrade installation evidence from the changed build.
  • Resolve the explicitly requested logo-distribution decision.

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.

@jsonMartin

Copy link
Copy Markdown
Contributor Author

Reopened as #3795 with a conventional branch name (feat/omarchy-provider-logos). Same commit, no content change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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.

1 participant