Add Codex Reset Credit Count and Expiry Display - #3
Open
TanjimReza wants to merge 2 commits into
Open
TanjimReza wants to merge 2 commits into
TanjimReza wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughAdds Codex reset credit support end-to-end: a new ChangesCodex Reset Credits Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/CodexMaxx/main.swift (1)
535-544: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear stale reset-credit tooltips when reset info is not rendered.
Line 575 sets a reset tooltip only for stacked/circles layouts. Switching back to inline, losing the snapshot, or hitting the empty-rows path leaves the previous tooltip attached to the status item.
Proposed fix
if rows.isEmpty { self.statusItem.length = NSStatusItem.variableLength self.statusItem.button?.image = nil self.statusItem.button?.title = "codexmaxx" + self.statusItem.button?.toolTip = nil } else { let activeResetCredits = rows.first(where: \.active)?.resetCredits let source = self.settings.source == .active ? rows.first(where: \.active)?.snapshot : UsageMath.combined(rows.map(\.snapshot)) @@ guard let snapshot else { self.statusItem.length = NSStatusItem.variableLength self.statusItem.button?.image = nil self.statusItem.button?.title = "S ? W ?" + self.statusItem.button?.toolTip = nil return } @@ case .inline: self.statusItem.length = NSStatusItem.variableLength self.statusItem.button?.image = nil self.statusItem.button?.title = UsageText.menuBarSummary(snapshot, resetCredits: resetCredits, settings: self.settings) + self.statusItem.button?.toolTip = nilAlso applies to: 557-575
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/CodexMaxx/main.swift` around lines 535 - 544, The status item is leaving a stale reset-credit tooltip behind when reset info is no longer being rendered. Update the logic around the empty-rows path and the `renderStatusItem(snapshot:resetCredits:)` flow so any previously set tooltip is cleared whenever the layout switches away from stacked/circles or no snapshot/reset credits are available. Make sure the tooltip state on `statusItem.button` is reset consistently in the same places that the title/image are updated.
🧹 Nitpick comments (1)
Sources/CodexMaxx/main.swift (1)
3262-3273: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid making reset credits block primary usage rendering.
The reset-credit call is supplemental, but this awaits it before returning the already-fetched usage snapshot. A slow reset endpoint can delay account usage display by the reset request timeout.
Proposed refactor
- let response = try await CodexOAuthUsageFetcher.fetchUsage( + async let usageResponse = CodexOAuthUsageFetcher.fetchUsage( accessToken: credentials.accessToken, accountId: credentials.accountId, env: env) + async let resetCreditsResult: Result<CodexResetCreditsSnapshot, Error> = { + do { + return .success(try await CodexResetCreditsFetcher.fetchResetCredits( + accessToken: credentials.accessToken, + accountId: credentials.accountId, + env: env)) + } catch { + return .failure(error) + } + }() + + let response = try await usageResponse let snapshot = CodexReconciledState .fromOAuth(response: response, credentials: credentials) let resetCredits: CodexResetCreditsSnapshot? let resetCreditsError: String? - do { - resetCredits = try await CodexResetCreditsFetcher.fetchResetCredits( - accessToken: credentials.accessToken, - accountId: credentials.accountId, - env: env) + switch await resetCreditsResult { + case .success(let value): + resetCredits = value resetCreditsError = nil - } catch { + case .failure(let error): resetCredits = nil resetCreditsError = error.localizedDescription }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/CodexMaxx/main.swift` around lines 3262 - 3273, The reset-credits fetch in the usage flow is blocking the already-available usage snapshot, which can delay primary rendering. Update the code around the CodexResetCreditsFetcher.fetchResetCredits call in this async block to run the reset-credits request independently of the main usage response, so account usage can return immediately while the supplemental reset data is resolved later. Keep the existing resetCredits and resetCreditsError handling, but move this work out of the critical path in the main usage rendering logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/CodexMaxx/main.swift`:
- Around line 3530-3543: Update compactCountdown(to:now:) so expired or due-now
dates return the now value instead of a minimum minute; the current fallback in
compactCountdown is causing just-expired timestamps to display as 1m. Adjust the
early/terminal case in compactCountdown(to:now:) to detect date <= now and
return now, while keeping the existing day/hour formatting for future dates.
- Around line 3270-3273: The reset credit fetch failure is being stored in
`resetCreditsError` inside `CodexUsageLoader.load`, but nothing in the UI
consumes it, so the error state is never visible. Update the account/status
rendering path that reads `resetCredits` to also surface `resetCreditsError`, or
remove `resetCreditsError` from `CodexUsageLoader` if you do not intend to show
fetch failures.
---
Outside diff comments:
In `@Sources/CodexMaxx/main.swift`:
- Around line 535-544: The status item is leaving a stale reset-credit tooltip
behind when reset info is no longer being rendered. Update the logic around the
empty-rows path and the `renderStatusItem(snapshot:resetCredits:)` flow so any
previously set tooltip is cleared whenever the layout switches away from
stacked/circles or no snapshot/reset credits are available. Make sure the
tooltip state on `statusItem.button` is reset consistently in the same places
that the title/image are updated.
---
Nitpick comments:
In `@Sources/CodexMaxx/main.swift`:
- Around line 3262-3273: The reset-credits fetch in the usage flow is blocking
the already-available usage snapshot, which can delay primary rendering. Update
the code around the CodexResetCreditsFetcher.fetchResetCredits call in this
async block to run the reset-credits request independently of the main usage
response, so account usage can return immediately while the supplemental reset
data is resolved later. Keep the existing resetCredits and resetCreditsError
handling, but move this work out of the critical path in the main usage
rendering logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 03018df4-cbf4-4a1f-810a-9d6c2fbf4c01
📒 Files selected for processing (2)
README.mdSources/CodexMaxx/main.swift
Comment on lines
+3270
to
+3273
| } catch { | ||
| resetCredits = nil | ||
| resetCreditsError = error.localizedDescription | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether resetCreditsError is rendered or otherwise consumed.
rg -n -C3 '\bresetCreditsError\b' .Repository: kitze/codexmaxx
Length of output: 3439
🏁 Script executed:
#!/bin/bash
rg -n -C4 '\b(CodexAccountUsage|resetCredits|resetCreditsError)\b' Sources/CodexMaxx/main.swift
sed -n '1538,1570p' Sources/CodexMaxx/main.swift
sed -n '2338,2372p' Sources/CodexMaxx/main.swiftRepository: kitze/codexmaxx
Length of output: 22504
Surface resetCreditsError or drop the field. resetCreditsError is set in CodexUsageLoader.load, but the UI only reads resetCredits, so fetch failures never reach the visible account/status views. Either render the error state or remove the unused field.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/CodexMaxx/main.swift` around lines 3270 - 3273, The reset credit
fetch failure is being stored in `resetCreditsError` inside
`CodexUsageLoader.load`, but nothing in the UI consumes it, so the error state
is never visible. Update the account/status rendering path that reads
`resetCredits` to also surface `resetCreditsError`, or remove
`resetCreditsError` from `CodexUsageLoader` if you do not intend to show fetch
failures.
Comment on lines
+3530
to
+3543
| static func compactCountdown(to date: Date, now: Date = Date()) -> String { | ||
| let seconds = max(0, Int(date.timeIntervalSince(now))) | ||
| let days = seconds / 86_400 | ||
| let hours = (seconds % 86_400) / 3_600 | ||
| let minutes = (seconds % 3_600) / 60 | ||
| if days > 0 { | ||
| return hours > 0 ? "\(days)d\(hours)h" : "\(days)d" | ||
| } | ||
| if hours > 0 { | ||
| return minutes > 0 ? "\(hours)h\(minutes)m" : "\(hours)h" | ||
| } | ||
| return "\(max(1, minutes))m" | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Return now for expired compact countdowns.
For dates at or before now, Line 3541 currently returns 1m, which makes just-expired reset credits look like they still have time remaining.
Proposed fix
static func compactCountdown(to date: Date, now: Date = Date()) -> String {
- let seconds = max(0, Int(date.timeIntervalSince(now)))
- let days = seconds / 86_400
- let hours = (seconds % 86_400) / 3_600
- let minutes = (seconds % 3_600) / 60
+ let seconds = date.timeIntervalSince(now)
+ guard seconds > 0 else { return "now" }
+
+ let totalMinutes = max(1, Int(ceil(seconds / 60)))
+ let days = totalMinutes / (24 * 60)
+ let hours = (totalMinutes / 60) % 24
+ let minutes = totalMinutes % 60
if days > 0 {
return hours > 0 ? "\(days)d\(hours)h" : "\(days)d"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| static func compactCountdown(to date: Date, now: Date = Date()) -> String { | |
| let seconds = max(0, Int(date.timeIntervalSince(now))) | |
| let days = seconds / 86_400 | |
| let hours = (seconds % 86_400) / 3_600 | |
| let minutes = (seconds % 3_600) / 60 | |
| if days > 0 { | |
| return hours > 0 ? "\(days)d\(hours)h" : "\(days)d" | |
| } | |
| if hours > 0 { | |
| return minutes > 0 ? "\(hours)h\(minutes)m" : "\(hours)h" | |
| } | |
| return "\(max(1, minutes))m" | |
| } | |
| static func compactCountdown(to date: Date, now: Date = Date()) -> String { | |
| let seconds = date.timeIntervalSince(now) | |
| guard seconds > 0 else { return "now" } | |
| let totalMinutes = max(1, Int(ceil(seconds / 60))) | |
| let days = totalMinutes / (24 * 60) | |
| let hours = (totalMinutes / 60) % 24 | |
| let minutes = totalMinutes % 60 | |
| if days > 0 { | |
| return hours > 0 ? "\(days)d\(hours)h" : "\(days)d" | |
| } | |
| if hours > 0 { | |
| return minutes > 0 ? "\(hours)h\(minutes)m" : "\(hours)h" | |
| } | |
| return "\(max(1, minutes))m" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/CodexMaxx/main.swift` around lines 3530 - 3543, Update
compactCountdown(to:now:) so expired or due-now dates return the now value
instead of a minimum minute; the current fallback in compactCountdown is causing
just-expired timestamps to display as 1m. Adjust the early/terminal case in
compactCountdown(to:now:) to detect date <= now and return now, while keeping
the existing day/hour formatting for future dates.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.