MiniMax: fix remains-to-used mapping and add quota-style usage cards with weekly text window#970
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e2d0832b4
ℹ️ 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".
| let usageLabel = "Usage: \(used.formatted()) / \(service.limit.formatted())" | ||
| let usedLabel = "Used \(String(format: "%.0f%%", displayPercent))" | ||
| let title = if service.displayName == "Text Generation", textGenerationCount > 1 { | ||
| "Text Generation · \(service.windowType == "Weekly" ? "Weekly" : "5h")" |
There was a problem hiding this comment.
Derive text-generation subtitle from actual window type
The title logic hard-codes every non-weekly Text Generation row to · 5h, so any plan whose interval window is not 5 hours (for example Today or other hour lengths produced by parseWindowInfo) will be mislabeled in the menu. This is a user-visible regression introduced by the new dual-window labeling, and it should use the real service.windowType (or a deliberate mapping) instead of forcing 5h.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed both Codex Review P2 comments with minimal-scope fixes in follow-up commit 9c61d87:
- Text Generation badge now derives from the actual
windowTypeinstead of hardcoding every non-weekly row as5h. - Weekly rows now format
timeRangeas a date-based UTC+8 range instead of HH:mm-only output.
Also added regression coverage for both cases and re-ran:
swift build✅swift test --filter MiniMax✅git diff --check✅
| var (windowType, timeRange) = self.parseWindowInfo(startTime: startTime, endTime: endTime, now: now) | ||
| if let windowTypeOverride { windowType = windowTypeOverride } |
There was a problem hiding this comment.
Format weekly time ranges with dates, not HH:mm-HH:mm
Weekly service rows only override windowType but keep timeRange from parseWindowInfo, which always formats as HH:mm-HH:mm(UTC+8). Because the new MiniMax card now displays service.timeRange directly, weekly windows can render as misleading same-time ranges (e.g., 10:00-10:00(UTC+8)) instead of a 7-day span, making the quota window look effectively zero-length.
Useful? React with 👍 / 👎.
9c61d87 to
aaa3576
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aaa3576fe1
ℹ️ 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".
| let weeklyService = self.makeServiceUsage( | ||
| ServiceUsageInput( | ||
| serviceType: serviceTypeIdentifier, | ||
| windowTypeOverride: "Weekly", |
There was a problem hiding this comment.
Keep weekly windows parseable as 10080-minute lanes
Setting windowTypeOverride to the literal "Weekly" creates MiniMax weekly rows that cannot be converted into a duration by MiniMaxUsageSnapshot.windowMinutes(for:) (it only recognizes today or numeric unit strings), so the resulting RateWindow gets windowMinutes == nil. That breaks downstream weekly-lane consumers that key off 10080 minutes (for example UsageStore+PlanUtilization only records weekly samples when window.windowMinutes == 10080), so MiniMax weekly history/reset tracking features silently stop working even though a weekly card is shown.
Useful? React with 👍 / 👎.
| remainsTime: item.weeklyRemainsTime), | ||
| now: now) | ||
| { | ||
| services.append(weeklyService) |
There was a problem hiding this comment.
Ensure weekly MiniMax lane is always emitted as secondary
Appending weekly services inline per model_remains item makes the lane order depend on API array order, and MiniMaxUsageSnapshot.secondaryService treats secondary as the weekly slot for many downstream paths (quota warnings/widgets). If a non-text model appears before the text model, the produced order becomes non-text first, text weekly later, so the weekly lane is pushed to tertiary and the app reports the wrong window as weekly in those flows.
Useful? React with 👍 / 👎.
Summary
This PR improves MiniMax token plan usage presentation and fixes remains-field semantics.
What changed
Corrected MiniMax remains semantics:
current_interval_usage_countis treated as interval remainingcurrent_weekly_usage_countis treated as weekly remainingmax(0, total - remaining)used / totalAdded text-generation dual window support:
5handWeeklyentries when weekly quota is valid.Filtered invalid placeholder rows:
total <= 0) so 0/0 placeholders do not render.Updated MiniMax menu metric presentation within the existing SwiftUI menu layout:
Usage: used / totalUsed xx%Expanded MiniMax service display-name normalization for common service labels.
Scope and safety
Tests
Ran and passed:
swift buildswift test --filter MiniMaxMiniMax-related tests now cover:
current_interval_usage_count/current_weekly_usage_counthandled as remaining.used = total - remainingcalculation.