Skip to content

Improve usage detail layout - #2182

Closed
jack24254029 wants to merge 5 commits into
steipete:mainfrom
jack24254029:fix/usage-card-detail-layout
Closed

Improve usage detail layout#2182
jack24254029 wants to merge 5 commits into
steipete:mainfrom
jack24254029:fix/usage-card-detail-layout

Conversation

@jack24254029

@jack24254029 jack24254029 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • split usage pace ETA and run-out risk into a stable two-row layout
  • show all reset-credit expiries on separate lines with compact MM/dd HH:mm absolute dates
  • keep Preferences, accessibility, and height-cache fingerprints aligned

Tests

  • make check
  • make test (638 selections, 54 groups)

Screenshots

image

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fcb293fca1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 190 to +192
(current.detailLeftText == nil) == (candidate.detailLeftText == nil) &&
(current.detailRightText == nil) == (candidate.detailRightText == nil) &&
(current.detailRightSecondaryText == nil) == (candidate.detailRightSecondaryText == nil) &&

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Re-measure when metric detail text changes rows

With the new MetricDetailRow, rows without detailRightSecondaryText can change height based on the actual detail strings because ViewThatFits falls back from one row to two rows when the text no longer fits. This compatibility check still only compares whether the detail fields are nil, so an open live card can accept a refresh that changes detailLeftText/detailRightText from short to long as the same layout; the existing NSMenuItem height is then not rebuilt/re-measured and the new second row can be clipped until the menu is reopened. Reject compatibility or compare enough text shape to cover the one-row/two-row transition.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — this was addressed in a959cade. hasCompatibleMetricLayout now compares normalized text shapes for detailLeftText and detailRightText, so wording or shape changes reject compatibility and trigger remeasurement, while numeric-only substitutions remain compatible. UsageMenuCardLayoutTests covers short text, same-shape numeric updates, and longer text that can cross the one-row/two-row boundary. The fix remains present in the current head, 23d748f8.

Split usage pace ETA and run-out risk across two rows.

Show all reset-credit expiries with consistent compact dates.

Keep Preferences, accessibility, and height fingerprints aligned.
@jack24254029
jack24254029 force-pushed the fix/usage-card-detail-layout branch from fcb293f to 6aadf0d Compare July 15, 2026 02:32
Reject live metric updates when adaptive detail text can change row count.

Add regression coverage for short-to-long detail transitions.
Normalize numeric detail changes to stable text shapes so refreshes do not retain stale data.

Preserve remeasurement for wording changes and cover compatible text shapes with a regression test.

Focused tests and make check pass. Full make test remains blocked by existing Claude credentials test failures.
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels Jul 16, 2026
@clawsweeper

clawsweeper Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 31, 2026, 7:02 PM ET / 23:02 UTC.

ClawSweeper review

What this changes

This PR separates pace ETA and run-out risk into stable menu and Preferences rows, and shows every Codex reset-credit expiry as an individual compact line.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

This PR is still necessary: current main and v0.46.0 retain the older combined pace-risk line and truncated reset-credit summary. The prior dynamic-height concern is resolved at the current head, checks are green, and no discrete repair remains; it needs ordinary maintainer review.

Priority: P3
Reviewed head: 23d748f8b81110fd032de5802ff97a36c6df2def

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) This is a focused, cleanly mergeable UI refinement with direct visual proof, targeted regression coverage, and no outstanding correctness finding.
Proof confidence 🦞 diamond lobster (5/6) ✨ media proof bonus Sufficient (screenshot): The contributor included a direct screenshot of the changed native menu layout, which convincingly demonstrates the visual result for this UI-only refinement.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (screenshot): The contributor included a direct screenshot of the changed native menu layout, which convincingly demonstrates the visual result for this UI-only refinement.
Evidence reviewed 5 items Current main does not contain this layout: Current main retains expirySummaryText and combines run-out risk into rightLabel; the PR adds MetricDetailRow, a secondary risk field, and individual compact expiry lines. The PR’s five commits remain unique to its branch.
Prior refresh finding is addressed: The current PR head normalizes numeric portions of adaptive detail text and compares text shapes, so wording changes that can move content from one row to two reject compatibility and trigger remeasurement.
Focused regression coverage: The branch adds model compatibility and height checks for short-to-long adaptive details and a fixed two-row risk detail, alongside targeted pace and reset-credit presentation tests.
Findings None None.
Security None None.

How this fits together

CodexBar turns provider usage snapshots into a shared menu-card model. That model supplies pace, run-out risk, and reset-credit details to both the menu-bar card and the Preferences provider-detail view.

flowchart LR
  A[Provider usage snapshot] --> B[Usage card model]
  B --> C[Pace and expiry formatter]
  C --> D[Menu card layout]
  C --> E[Preferences layout]
  D --> F[Menu bar usage details]
  E --> G[Settings usage details]
Loading

Before merge

  • Complete next step (P2) - No discrete repair remains: the previous remeasurement concern is addressed, current checks are green, and the next action is routine maintainer review by the likely related people above.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 13 files: 8 source, 5 test The patch updates the shared presentation model and its two rendering surfaces with targeted regression coverage.
Validation status 9 of 9 reported checks passed The current clean head has successful build, lint, and sharded macOS test checks.

Technical review

Best possible solution:

Merge the focused shared-model and rendering update after maintainer review, retaining the compatibility guard and regression coverage for live menu-card height changes.

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

Not applicable: this PR refines existing UI presentation rather than repairing a separately reported failure. The supplied screenshot directly shows the intended menu-card result.

Is this the best way to solve the issue?

Yes: the shared presentation-model field keeps the menu and Preferences views aligned, and the revised compatibility check covers the previously identified live remeasurement boundary.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8ef86077e70a.

Labels

Label justifications:

  • P3: This is a focused readability and layout refinement with limited blast radius and no provider, authentication, or data-model behavior change.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The contributor included a direct screenshot of the changed native menu layout, which convincingly demonstrates the visual result for this UI-only refinement.
  • proof: sufficient: Contributor real behavior proof is sufficient. The contributor included a direct screenshot of the changed native menu layout, which convincingly demonstrates the visual result for this UI-only refinement.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The contributor included a direct screenshot of the changed native menu layout, which convincingly demonstrates the visual result for this UI-only refinement.

Evidence

What I checked:

Likely related people:

  • Peter Steinberger: Recent current-main history includes the primary-metric presentation refactor and repeated updates across the menu usage-model surface. (role: recent area contributor and merger; confidence: high; commits: 8ef86077e70a, b82594e2a0c0, 5e36df175e0d; files: Sources/CodexBar/MenuCardView+ModelHelpers.swift, Sources/CodexBar/MenuCardView.swift, Sources/CodexBar/MenuCardView+CodexResetCredits.swift)
  • smkwray: Recent current-main commits cover scoped weekly pace behavior that is rendered by the PR’s changed pace-detail path. (role: recent pace-behavior contributor; confidence: medium; commits: d90282ec76b2, 5e36df175e0d; files: Sources/CodexBar/MenuCardView+ModelHelpers.swift, Sources/CodexBar/UsagePaceText.swift)

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.

History

Review history (4 earlier review cycles)
  • reviewed 2026-07-16T02:16:19.844Z sha a959cad :: needs maintainer review before merge. :: none
  • reviewed 2026-07-20T06:35:04.639Z sha e431be3 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-30T05:02:00.180Z sha e431be3 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-31T02:09:23.213Z sha 23d748f :: needs maintainer review before merge. :: none

…-layout

# Conflicts:
#	Sources/CodexBar/MenuCardHeightFingerprint.swift
#	Sources/CodexBar/MenuCardView.swift
Preserve the adaptive menu-card detail layout and run-out risk presentation while adopting main's refactored primary metric model.
@steipete

steipete commented Aug 3, 2026

Copy link
Copy Markdown
Owner

The layout direction from this PR landed via #2620 in condensed form: used% and reset now live in the title row, and all pace detail folds into a single meta line under the bar — the maintainer wanted one line rather than the stacked block, but your structural cleanups carried over and the changelog credits you. Thanks @jack24254029! Closing in favor of the landed shape; please try it in 0.47.1.

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. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants