Skip to content

Compact reset credit expiries#1902

Merged
steipete merged 1 commit into
mainfrom
codex/compact-reset-credit-expiries
Jul 4, 2026
Merged

Compact reset credit expiries#1902
steipete merged 1 commit into
mainfrom
codex/compact-reset-credit-expiries

Conversation

@steipete

@steipete steipete commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • replace one reset-credit expiry row per credit with a single clock-prefixed timeline
  • keep full numbered expiry details in the tooltip and accessibility label
  • mirror the compact treatment in provider settings and collapse long inventories with +N

Before / after

Before

Reset-credit expiries before: one numbered row per credit

After

Reset-credit expiries after: one compact clock-prefixed timeline

Proof

  • swift test --filter CodexResetCreditsMenuCardTests
  • swift test --filter MenuCardHeightFingerprintTests
  • make check
  • make test — 47/47 shards passed
  • ./Scripts/compile_and_run.sh — packaged app launched and remained running
  • source-blind behavior validation — 5/5 clauses passed, including menu close/reopen
  • autoreview — clean; no accepted/actionable findings

@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: 5eb70d2413

ℹ️ 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".

HStack(alignment: .firstTextBaseline, spacing: 4) {
Image(systemName: "clock")
.font(.caption2)
Text(self.presentation.expirySummaryText)

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 Add a tooltip for hidden reset-credit expiries

When a Codex account has more than four available reset credits, expirySummaryText collapses the remaining entries into +N; in the provider settings row this is now the only visible expiry text, and unlike the menu card it does not attach .help(self.presentation.helpText). That means the expiry dates that used to be displayed in settings are no longer discoverable to sighted users from this view, so add the same tooltip/full-detail affordance here before replacing the per-credit rows.

Useful? React with 👍 / 👎.

@clawsweeper

clawsweeper Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed July 4, 2026, 3:21 PM ET / 19:21 UTC.

Summary
The PR compacts Codex reset-credit expiry rows into a clock-prefixed summary while preserving full details in menu help/accessibility and adding matching settings-row presentation.

Reproducibility: yes. from source inspection: a reset-credit inventory with more than four credits produces +N, and the settings row has no .help modifier to expose the hidden expiry list to sighted users.

Review metrics: 1 noteworthy metric.

  • Changed surface: 6 files, +63/-53. The PR is narrow enough for a targeted settings-row correction rather than broad redesign.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
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:

  • [P2] Add settings-row tooltip/help for presentation.helpText.

Risk before merge

  • [P1] Settings users with five or more available reset credits would see +N without a sighted path to inspect the hidden expiry dates in that view.

Maintainer options:

  1. Decide the mitigation before merge
    Add the same full-detail tooltip/help affordance to the provider settings reset-credit row before landing the compact settings summary.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] The remaining blocker is a narrow SwiftUI affordance fix that automation can attempt on the PR branch.

Security
Cleared: The diff only changes local SwiftUI presentation, tests, and a changelog entry; no security or supply-chain surface is introduced.

Review findings

  • [P2] Restore full expiry help in settings — Sources/CodexBar/PreferencesProviderDetailView.swift:493-494
Review details

Best possible solution:

Add the same full-detail tooltip/help affordance to the provider settings reset-credit row before landing the compact settings summary.

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

Yes from source inspection: a reset-credit inventory with more than four credits produces +N, and the settings row has no .help modifier to expose the hidden expiry list to sighted users.

Is this the best way to solve the issue?

No: the compact UI direction is fine, but the settings row should keep the full expiry list discoverable with the same tooltip/help affordance used by the menu card.

Full review comments:

  • [P2] Restore full expiry help in settings — Sources/CodexBar/PreferencesProviderDetailView.swift:493-494
    When there are more than four reset credits, this row now collapses the rest to +N, but unlike the menu card it never attaches presentation.helpText as a tooltip. The full expiry dates that used to be visible in Settings are only present in the accessibility label, so sighted users cannot inspect the hidden credits from this view.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This is a bounded UI correctness regression in provider settings discoverability, not an urgent runtime or data-loss issue.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The contributor proof gate does not apply to this owner-authored PR, though the body includes before/after screenshots and validation commands.

Label justifications:

  • P2: This is a bounded UI correctness regression in provider settings discoverability, not an urgent runtime or data-loss issue.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The contributor proof gate does not apply to this owner-authored PR, though the body includes before/after screenshots and validation commands.
Evidence reviewed

Acceptance criteria:

  • [P1] swift test --filter CodexResetCreditsMenuCardTests.
  • [P1] swift test --filter MenuCardHeightFingerprintTests.
  • [P1] make check.

What I checked:

Likely related people:

  • Brahim HAMICHAN: Introduced the current read-only reset-credit inventory UI and provider settings row in the merged reset-credit inventory commit. (role: recent feature contributor; confidence: high; commits: a1da184c6397; files: Sources/CodexBar/MenuCardView+CodexResetCredits.swift, Sources/CodexBar/PreferencesProviderDetailView.swift, Tests/CodexBarTests/CodexResetCreditsMenuCardTests.swift)
  • Peter Steinberger: Authored this PR and is co-authored on merged reset-credit inventory and reset-credit visibility commits, with current-main ownership signals in the affected SwiftUI paths. (role: recent adjacent contributor; confidence: high; commits: 5eb70d2413a9, a1da184c6397, 945d4ba1264c; files: Sources/CodexBar/MenuCardView+CodexResetCredits.swift, Sources/CodexBar/PreferencesProviderDetailView.swift, Tests/CodexBarTests/CodexResetCreditsMenuCardTests.swift)
  • Abu Talha: Added the initial Codex reset-credit menu-card presentation and tests that the current compact UI refactors. (role: original feature contributor; confidence: medium; commits: a3ea16743560; files: Sources/CodexBar/MenuCardView+CodexResetCredits.swift, Sources/CodexBarCore/CreditsModels.swift, Tests/CodexBarTests/CodexResetCreditsMenuCardTests.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.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. labels Jul 4, 2026
@steipete steipete merged commit c5cf41c into main Jul 4, 2026
9 checks passed
@steipete steipete deleted the codex/compact-reset-credit-expiries branch July 4, 2026 19:22
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: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant