Redesign usage widgets around the binding quota - #3137
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
![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: c9feddf3a2
ℹ️ 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".
|
|
||
| var body: some View { | ||
| let color = WidgetColors.color(for: self.entry.provider) | ||
| let allLanes = WidgetTileLane.lanes(for: self.entry, limit: self.laneLimit) |
There was a problem hiding this comment.
For compact Kimi widgets, laneLimit is 3 even though live snapshots can contain four quota rows (UsageStore+WidgetSnapshot.swift appends kimi-code-7d). Applying that limit here before WidgetTilePlan chooses its minimum means a fourth lane at, for example, 1% is discarded while a healthier lane becomes the headline; it is also absent from overflowCount, so the tile gives no indication that the actual binding quota was omitted.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in b94815ce9.
Reproduced exactly as described: KimiProviderDescriptor sets widgetRowLimitResolver: { _, _ in 3 } while UsageStore+WidgetSnapshot.swift:507 appends kimi-monthly and kimi-code-7d on top of the generic primary/secondary pair — four rows against a cap of three. With a kimi-code-7d lane at 1% the tile headlined Session 88% and reported overflowCount == 0.
UsageTile now resolves two sets: allLanes (unlimited) and displayLanes (the provider-curated subset). WidgetTilePlan.make takes both, picks the headline from allLanes, and computes overflowCount against allLanes too. The provider’s intended row count is preserved because the headline occupies one of those rows — capacity = min(maxSecondaryLanes, displayCandidates.count - 1) — and Antigravity’s one-row-per-model-family curation still decides which lanes get listed, since that is real curation rather than a prefix.
The same fixture now headlines the 1% lane in red with +2 more. Covered by a provider row cap never hides the binding lane and the curated row count is honoured once the headline takes a row.
| percentLeft: row.window?.remainingPercent ?? row.percentLeft, | ||
| resetsAt: row.window?.resetsAt, | ||
| resetDescription: row.window?.resetDescription) |
There was a problem hiding this comment.
For normal live snapshots from non-Codex providers, entry.usageRows is populated, but the generic snapshot writer at UsageStore+WidgetSnapshot.swift:463-471 stores only percentLeft while the corresponding reset date remains on entry.primary/secondary. Since the preceding resolver only reconstructs legacy Codex windows, these assignments receive nil, so the redesigned reset caption works in previews and Codex but silently disappears for providers such as Claude, Gemini, and Alibaba.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in b94815ce9.
This one also explained a device screenshot I had already misread — the Claude tile showed no reset line while Codex did, and I had put that down to provider data rather than the writer.
WidgetUsageRow.rows now falls back to the matching entry window when a row carries no window:, via a deliberately narrow resolver:
static func slotWindow(for rowID: String, entry: WidgetSnapshot.ProviderEntry) -> RateWindow? {
switch rowID {
case "primary": entry.primary
case "secondary": entry.secondary
case "tertiary": entry.tertiary
default: nil
}
}Only the reset is taken from it; the row keeps its own percentLeft, so nothing about existing percentage resolution changes. Provider-specific row IDs deliberately do not match, so no reset is invented where the writer has none — slot recovery does not invent resets for provider specific rows pins that.
Fixed widget-side rather than in the writer so snapshots already on disk gain the caption without a snapshot format change. Kimi’s own extra windows still ship without a window:, so their reset stays absent; that needs a writer change and felt out of scope here.
| if let resetsAt { | ||
| let interval = resetsAt.timeIntervalSince(now) | ||
| if interval > 0 { | ||
| return "Resets in \(self.duration(interval))" | ||
| } | ||
| } |
There was a problem hiding this comment.
When a snapshot contains both resetsAt and a cached resetDescription, reaching the reset date skips this return and falls through to the description fallback below. If the app has not yet published a fresh snapshot, the widget therefore continues showing stale copy such as “Resets in 4h” after that reset has already passed; an expired known date should return nil rather than reuse its old wording.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed — GitHub re-anchored this thread onto the newer commit, but the fix is in it. WidgetTilePlan.swift now reads:
if let resetsAt {
let interval = resetsAt.timeIntervalSince(now)
guard interval > 0 else { return nil }
return "Resets in \(self.duration(interval))"
}An expired known date returns nil instead of falling through to the cached wording. My original test only exercised the resetDescription: nil path, which is exactly why it missed this; a known reset that already passed renders nothing, even with cached wording now passes resetDescription: "Resets in 4h" alongside a past resetsAt.
c9feddf to
8f26c5a
Compare
|
Thanks — all three were real. Reproduced each before fixing, and each fix carries a regression test that fails without it. Pushed as P1 · binding lane hidden by a provider row cap — confirmed. The cap now curates only what a tile lists. P2 · reset caption missing for generic rows — confirmed, and it explains something I had already seen on device: the Claude tile showed no reset line while Codex did, and I had put that down to the provider. The generic writer builds Fixed widget-side rather than in the writer so snapshots already on disk get the caption without a format change. Kimi's own extra windows still ship without a P2 · stale reset wording after the reset date — confirmed. A known Widget test count is 43. |
clawsweeper
Bot
added
P3
|
Codex review: needs maintainer review before merge. Reviewed August 27, 2026, 1:04 PM ET / 17:04 UTC. ClawSweeper reviewWhat this changesThis PR consolidates usage-widget layouts into shared responsive tiles, adds provider paging and reset captions, and emphasizes the quota with the least remaining allowance. Merge readinessKeep open: the implementation is technically sound and has sufficient runtime proof, but the owner has explicitly reserved the semantic choice of a dynamic binding-quota headline before merge. Priority: P3 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherCodexBar persists provider usage snapshots, which the WidgetKit extension reads to render compact macOS usage widgets. The tile planner selects quota lanes and the shared views format them into the widget shown to users. flowchart LR
A[Provider usage snapshots] --> B[Widget snapshot reader]
B --> C[Quota lane planner]
D[Widget size and appearance] --> E[Shared usage tile]
C --> E
F[Provider selection] --> G[Pager header]
G --> E
E --> H[WidgetKit display]
Decision needed
Why: Both choices are technically viable; the first surfaces the quota most likely to block work, while the second preserves stable positional meaning across refreshes. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the shared responsive tile architecture and land it only after the owner explicitly accepts the urgency-first headline or requests the small primary-lane variant. Do we have a high-confidence way to reproduce the issue? Not applicable: this is a widget redesign rather than a report of one failing current behavior; supplied native WidgetKit captures show the proposed behavior after the change. Is this the best way to solve the issue? Unclear: the implementation is cohesive and validated, but the owner must choose between the more urgent dynamic headline and a stable primary-lane convention. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against b4694bd7edbf. LabelsLabel changes:
Label 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
History |
The tiles spent their space on a provider switcher that could not render
provider names, and buried the one number a glance is for.
Switcher: a row of chips never fit. Names wrapped mid-word on large,
truncated to indistinguishable stems on medium ("Co…" is both Codex and
Copilot), and collapsed to a single letter on small, where four of five
common providers are a "C". Widgets cannot host a menu or a picker, so the
replacement pages one provider at a time: one line instead of a row, and the
provider whose numbers are on screen is always spelled out.
Layout: every tile now leads with the binding quota — the lane with the least
left — as one large figure with its lane, its reset, and a bar. Percentages
say "left" or "used" so the display preference is no longer silent. Lanes that
do not fit are counted in a "+N more" line rather than clipped off the edge.
Lane capacity is derived from whether the tile also draws costs and a chart,
so a busy provider does not clip and a sparse one does not page for nothing.
Also fixes, all reproduced before being fixed:
- WidgetKit's default content margins applied on top of the tile's own
padding, insetting every tile roughly twice as far as intended
- Small tiles truncated the provider name because the timestamp took the width
- Tinted and clear appearances render through a luminance mask, which turned
the brand-filled provider mark into an empty white square
- The percentage was drawn dimmer than its own label
- Reset times were present in the snapshot and never rendered
Six near-duplicate size views collapse into one `UsageTile`, so the usage and
switcher widgets cannot drift apart.
Verified by rendering every layout headless at the exact macOS tile sizes
(155x155, 329x155, 329x345) across light, dark and clear, including the
gallery placeholder path, plus 37 tests over the pure layout logic.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
8f26c5a to
b94815c
Compare
|
Thanks — noting the two items and one correction. Headline policy (the decision you flagged). Your recommendation matches what's implemented: the headline follows the binding lane. I'd already surfaced this in the PR body as the first reviewer note, and I agree it's a product call rather than a correctness one, so I won't pre-empt it. To make the alternative cheap to evaluate, pinning it is a one-function change — private static func bindingLane(in lanes: [WidgetTileLane]) -> WidgetTileLane? {
lanes.first // instead of min(by: remainingPercent)
}Everything downstream — lane listing, overflow counting, severity, the reset caption — is independent of which lane becomes the headline, and Worth adding to the decision: the binding lane is what the P1 fix in this PR exists to protect. A provider row cap was hiding a 1%-left Kimi lane behind a healthier one, so a pinned headline would have shown Live verification. The That's Corepack blocking on a pnpm download, so the Swift build never ran. For the record, on
One note on the diff stat. The |
|
Maintainer review: technically this is in good shape — merges cleanly onto current main, builds, all 134 widget-suite tests pass, CI is green, and the before/after screenshots make a persuasive case (the Holding the merge for an owner decision since this reshapes the shipped appearance of all four usage widgets — @steipete, the screenshots in the description are worth the look. Two things to note when deciding:
No code changes requested from my side. |
|
Thank you and especially for running the full 134-test widget suite. That gives confidence across the broader provider surface I couldn't fully validate myself. On point 2: fair, and this was a deliberate tradeoff. There are two clean fixes for the doubled inset:
I chose the first because 14pt is about 2pt tighter per side. On the 155pt small tile, that extra 4pt is where the truncation fix lives: If you prefer the system-owned approach, I'm happy to switch it. It's a small diff, followed by a character-budget retune and a re-render to confirm nothing regresses. As shipped, that inset is single-sourced through On point 1: "it varies per refresh as lanes drain" is the strongest form of the objection. Naming the lane — such as "Weekly left" — keeps the figure accurate, but it doesn't preserve positional habit. The pinned variant is a one-function change from I'll keep the current binding-lane behavior unless @steipete prefers the pinned variant. |
steipete
left a comment
steipete
left a comment
There was a problem hiding this comment.
Rechecked b94815ce94f5fb82bbedc5e840a4afe53dbc0a96. The existing hold on the widget appearance/headline decision remains; this review does not approve that redesign.
There is also a concrete compact-row budget edge case in WidgetTilePlan.make: overflow reservation checks remainder.count from the curated candidates, but the final overflow count includes every lane. With four lanes, three display candidates, the hero inside those candidates, maxSecondaryLanes: 2, and reservesOverflowRow: true, the remainder has two entries, so no slot is reserved. The result contains two secondary rows plus +1 more—three rows for a two-row budget. UsageTile renders all three.
Please add this combined curation/reservation regression (for example the four-lane Kimi shape with the primary lane most constrained) and reserve space based on total omitted lanes, not only the curated remainder. The existing tests exercise curation and reservation separately, so they miss this intersection.
This is a source-proven budget violation, not a reproduced claim of WidgetKit clipping. After correction and the appearance decision, current-head packaged extension/pager/timeline/tinted-margin proof is still needed; static render images and older green test reports do not establish that runtime behavior. No tests or UI probes were run in this pass.
The overflow-row reservation asked whether the curated remainder exceeded capacity, but the overflow count is computed against every lane the provider reports. With four lanes, three display candidates, the hero inside them and a two-row budget, the curated remainder fit exactly, no slot was reserved, and the tile drew two lanes plus "+1 more" — three rows in a two-row budget. Reservation now derives the overflow line from all lanes and gives up a slot only when that line would actually exceed the budget, so a medium tile with room for lanes plus the line keeps its density. Adds the combined curation/reservation regression, a density guard, and a sweep across lane count, curation, budget and binding-lane position — the intersections the focused tests missed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Real WidgetKit desktop captures from the packaged extension: pager paging through all five seeded providers in place with wrap-around, the four-lane Kimi shape holding its row budget on small and medium, and the vibrant rendering keeping every mark legible. All frames use a synthetic seeded snapshot; no live account data appears. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
clawsweeper
Bot
added
the
merge-risk: 🚨 compatibility
|
Thanks for the follow-up. I rechecked the delta through 746318c and inspected both committed runtime contact sheets. The row-reservation change addresses the specific source defect from my previous review: it counts omitted lanes from the full set, while retaining the extra lane when the overflow row already fits. The new combined regression, density guard and combination sweep cover that distinction. The submitted images show the small Kimi tile with Session plus The packaged-extension log and runtime images are meaningful new evidence; the earlier “static renders only” limitation is no longer an accurate description of what you have supplied. I have inspected those artifacts, not independently rerun the build, tests or WidgetKit interactions. The recorded dim/vibrant appearance is also narrower than proof of every tinted/clear mode, and the seeded snapshot with the app stopped does not by itself establish repeated app-to-timeline updates. The existing owner appearance/headline decision remains open, so this is acknowledgment of the correction and new proof, not approval to merge or a request to add a settings toggle. Once that decision is settled, we can verify the final integrated variant and any remaining runtime gaps without repeating the already-addressed row-budget request. |
fix: let WidgetKit own usage content margins Remove redundant outer padding from Usage, Switcher, History, and Metric views while preserving system margins and the existing layout semantics. Extracted from #3137; the broader redesign remains separate. Native signed WidgetKit before/after proof uses the same synthetic snapshot. Full 1,031-selection suite, make check, and independent P2 review pass. Production -9 lines; docs and changelog updated. Co-authored-by: iamenahs <xianheng.ma@gmail.com>
|
Landed the isolated redundant-padding fix in #3480, using the system-owned margin option discussed here. Usage, Switcher, History, and Metric views now keep WidgetKit content margins without adding another outer inset; the patch removes nine production lines. The PR includes native signed WidgetKit before/after captures from the same synthetic snapshot, plus the full 1,031-selection suite and green final CI. Existing five-provider chip wrapping remains visible in the proof and is not claimed fixed. The broader appearance and binding-quota decision in this PR remains open. Thanks @iamenahs for identifying the inset problem and explaining both ownership options. |
2 participants
Summary
The usage widgets spend their space on a provider switcher that cannot render provider names,
and bury the one number a glance is for.
The switcher lays every provider out as a chip in a single row. That never fits: on large the
names wrap mid-word (
Cod/ex,Clau/de,Cur/sor), on medium they truncate toindistinguishable stems (
Co…is both Codex and Copilot), and on small they collapse to asingle letter, where four of five common providers are a
C. Meanwhile the percentage is drawn.secondary— dimmer than its own label — and never says whether it means remaining or used.Widgets cannot host a menu or a picker, so the chip row is replaced by a pager:
[CL] Claude · 4m ago … ‹ 2/5 ›. One line instead of a row, and the provider whose numbers areon screen is always spelled out in full.
Changes
figure with its lane name, its reset, and a bar.
WidgetTilePlanpicks it; ties keep theprovider's own ordering.
usageBarsShowUsedisno longer silent.
RateWindow.resetsAtis rendered. It was already in the snapshot and dropped on the floor.Counts down compactly ("Resets in 47m"); provider wording is the fallback and gets labelled, so
Codex's bare
tomorrow, 12:28 PMno longer appears without context.+N moreline, and oncompact tiles that line takes a lane slot so it cannot push content off the edge.
and a history chart, so a seven-lane provider does not clip and a one-lane provider does not
page for nothing. Medium drops to a single column when there is no second lane.
.contentMarginsDisabled()on the four redesigned widgets. WidgetKit was applying its owncontent margins on top of the tile's padding, insetting every tile roughly twice as far as
intended.
mask, which turned a brand fill carrying a dark label into an empty white square. Outside
.fullColorthe mark inverts to a faint plate with a bright label.lane is not an empty track.
UsageTile, so the usage and switcher widgetscannot drift apart. Size differences are values on
WidgetTileSize.BurnDown widgets are deliberately untouched.
Screenshots
Rendered headless at true macOS tile sizes with synthetic fixtures — no account identity, spend,
or token counts.
Before / after — same fixture, same framing, rendered from the pre- and post-redesign views
Light
Dark
Clear / tinted desktop appearance
Test plan
make check— 0 violationsmake test— green apart from a pre-existing failure, see caveatxcodebuild CodexBarWidgetExtension— Release, arm64 + x86_64reservation, reset rounding, pager wrap-around, monogram uniqueness, tinted-mode mark
styling, WCAG contrast for every provider colour)
placeholder(in:)gallerypath that nothing previously covered
inverted, longest provider name
Group container (frames not attached — they contain live usage)
Full proof artifact:
.github/pr-proof/widget-readability-redesign.logCommands run
Verification caveat
StatusMenuSwitcherRefreshTests→ "merged provider switch updates live tab rows in place" failswith 2 issues. It fails identically on a clean checkout of the same base (27c7f33) with this
branch absent — verified in a separate worktree. It is a headless AppKit menu-identity assertion;
this diff touches only
Sources/CodexBarWidget.Runtime proof (on-device)
Follow-up to the maintainer re-check: real WidgetKit desktop captures from the packaged
extension (
CodexBarWidget.appex, Developer ID signed, debug-config app group), running withthe app process dead — timelines and button intents are the extension's own. All frames carry a
synthetic seeded snapshot validated against the real decoder; no live account data appears.
Full steps in
.github/pr-proof/widget-readability-redesign.log.The pager paging in place through five providers, with wrap-around at the end:
The four-lane Kimi shape holding its row budget (this review round's fix), and the vibrant
rendering keeping marks legible:
Automated review follow-up
All three Codex findings on
c9feddf3a2were reproduced and fixed:(
primary,secondary,kimi-monthly,kimi-code-7d) while its resolver caps compactfamilies at 3, and the cap was applied before the headline was chosen. A 1%-left
kimi-code-7dlane was discarded, a healthier lane became the headline, andoverflowCountstayed 0 so nothing hinted at the omission. The cap now curates only what a tile lists;
headline and overflow are computed against every lane the provider reports, and the provider's
intended row count is still honoured because the headline occupies one of those rows.
screenshot where the Claude tile showed no reset line. The generic writer emits
primary/secondary/tertiaryrows with a percentage only and leaves the reset on theentry's own windows; only legacy Codex windows were reconstructed. Reset is now recovered from
the matching entry slot, with the row's own percentage left authoritative. Fixed widget-side so
snapshots already on disk benefit without a format change.
resetsAtin the past fellthrough to the cached description, so a stale snapshot kept reading "Resets in 4h" after the
reset had happened. An expired known date now renders nothing.
Each carries a regression test that fails without the fix.
The maintainer re-check on
b94815ce9found one more, also real and fixed in2cdfe0734:remainder exceeded capacity, while
overflowCountcounts every reported lane. Four lanes,three display candidates, the hero inside them and a two-row budget meant no slot was reserved,
yet "+1 more" still rendered — three rows in a two-row budget. Reservation now derives the
overflow line from all lanes and gives up a slot only when the line would actually exceed the
budget, so a medium tile with room for lanes plus the line keeps its density (the literal
"reserve whenever anything is omitted" would have dropped a lane the tile has room for — the
runtime captures above show both paths). Covered by the combined regression, a density guard,
and a sweep across lane count × curation × budget × binding-lane position. Widget test count is
now 46.
Reviewer notes
it can change identity between refreshes (Weekly → Session) as values move. It is always
labelled. Pinning it instead is a one-function change (
WidgetTilePlan.bindingLane(in:)→lanes.first); everything downstream is independent of which lane wins. Say the word and I willpush that variant or a settings toggle. Worth weighing: the P1 fix above exists precisely to
protect this signal — a provider row cap was hiding a 1%-left lane behind a healthier one, and a
pinned headline would have read "Session 88%" while the account was actually blocked.
is unchanged; this matches how first-party widgets degrade.
widgetRenderingModeOverrideis a snapshot-rendering seam and is always nil in the shippingwidget. WidgetKit owns
widgetRenderingModeand no host sets it outside a widget, so thetinted variants are otherwise unreachable from a preview.