Skip to content

Improve usage detail layout#2182

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

Improve usage detail layout#2182
jack24254029 wants to merge 4 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 👍 / 👎.

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 20, 2026, 2:35 AM ET / 06:35 UTC.

Summary
The PR adds adaptive one- or two-row usage details, separates pace risk from the ETA, and renders reset-credit expiries as individual compact lines in the menu and Preferences UI.

Reproducibility: not applicable. This PR is a presentation improvement rather than a report of broken established behavior. The supplied screenshot and layout tests provide a concrete way to inspect the proposed UI result.

Review metrics: 1 noteworthy metric.

  • UI and test surface: 13 files affected; 355 additions, 71 deletions. The change consistently updates the menu card, Preferences mirror, height-cache behavior, and focused regression coverage.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Next step before merge

  • [P2] No discrete repair remains; the next action is ordinary maintainer review of the proof-backed layout change.

Security
Cleared: The diff is limited to Swift presentation/model code and tests; it adds no dependency, secret, build, release, or execution-path change.

Review details

Best possible solution:

Merge the shared layout treatment only after a maintainer confirms the menu and Preferences presentation remains desirable at normal and constrained menu widths.

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

Not applicable: this PR is a presentation improvement rather than a report of broken established behavior. The supplied screenshot and layout tests provide a concrete way to inspect the proposed UI result.

Is this the best way to solve the issue?

Yes. Centralizing the adaptive detail behavior in MetricDetailRow while updating the Preferences counterpart and height fingerprints avoids competing layout rules across the two surfaces.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P3: This is a user-facing layout and readability improvement without evidence of a broken core workflow or urgent regression.
  • 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 PR body supplies a directly relevant screenshot of the changed usage-card layout after the UI change; it is appropriate proof for this visible native macOS presentation work.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies a directly relevant screenshot of the changed usage-card layout after the UI change; it is appropriate proof for this visible native macOS presentation work.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body supplies a directly relevant screenshot of the changed usage-card layout after the UI change; it is appropriate proof for this visible native macOS presentation work.
Evidence reviewed

What I checked:

  • Current-main comparison: Current main at f8636cb still models usage pace with one optional right-side detail string; this branch adds riskLabel/detailRightSecondaryText to keep the risk on a stable second row. (Sources/CodexBar/UsagePaceText.swift:6, f8636cb37eb0)
  • Prior review finding addressed: The earlier P2 review identified stale menu-height measurement when adaptive detail text crosses the one-row/two-row boundary. The branch subsequently added text-shape compatibility handling and regression coverage in commits c9ca167 and a959cad. (Sources/CodexBar/MenuCardView+ModelHelpers.swift:178, a959cade8d9f)
  • Real behavior proof: The PR body includes a directly relevant screenshot of the expanded usage card, showing the resulting multi-row detail and reset-credit presentation rather than only test output. (e431be3e3a01)
  • Focused regression coverage: The branch updates pace-text and reset-credit expectations and adds layout/fingerprint coverage for the new row behavior and dynamic-height cache boundary. (Tests/CodexBarTests/UsageMenuCardLayoutTests.swift:85, e431be3e3a01)

Likely related people:

  • steipete: Repository ownership and the concentrated CodexBar menu/Preferences surface make this the best available routing candidate; no stronger feature-history attribution was available in the supplied read-only review context. (role: likely follow-up owner; confidence: low; files: Sources/CodexBar/MenuCardView.swift, Sources/CodexBar/PreferencesProviderDetailView.swift, Sources/CodexBar/UsagePaceText.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.
Review history (1 earlier review cycle)
  • reviewed 2026-07-16T02:16:19.844Z sha a959cad :: needs maintainer review before merge. :: none

…-layout

# Conflicts:
#	Sources/CodexBar/MenuCardHeightFingerprint.swift
#	Sources/CodexBar/MenuCardView.swift
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.

1 participant