Skip to content

Fix MiniMax token plan usage display#1266

Open
XWind18 wants to merge 10 commits into
steipete:mainfrom
XWind18:codex/minimax-token-plan
Open

Fix MiniMax token plan usage display#1266
XWind18 wants to merge 10 commits into
steipete:mainfrom
XWind18:codex/minimax-token-plan

Conversation

@XWind18
Copy link
Copy Markdown
Contributor

@XWind18 XWind18 commented Jun 1, 2026

Summary

  • Handle MiniMax token plan quota responses with general and video buckets.
  • Preserve bucket-style display labels (General, Video) in the MiniMax menu card.
  • Support boosted quota limits from interval_boost_permill / weekly_boost_permill.
  • Treat Plus general weekly status-3 quota as unlimited, while hiding unavailable Plus video placeholders.
  • Keep MiniMax subscription metadata merged into the displayed plan name.

Validation

  • swift test --filter MiniMaxTokenPlanChangeTests
  • swift test --filter MiniMaxMenuCardModelPlanTests
  • make check
  • CODEXBAR_SIGNING=adhoc ./Scripts/package_app.sh

Notes

  • Full swift test currently fails in unrelated ProviderStorageFootprintTests.swift:401 with storageRefreshGeneration expected 41, actual 42.
  • The GitHub Actions failure shown earlier occurs while installing Swift 6.2.1 via swiftly, before project build/test starts.
image image

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented Jun 1, 2026

Codex review: needs changes before merge. Reviewed June 1, 2026, 9:59 PM ET / 01:59 UTC.

Summary
Updates MiniMax token-plan parsing and menu display for general/video quota buckets, boosted and unlimited quotas, points balance, subscription metadata, warning markers, and focused tests.

Reproducibility: yes. for the review finding from source: current main sends API-token fetches to apiRemainsURL, while the PR head sends only to tokenPlanRemainsURL. I did not run live MiniMax probes because AGENTS.md requires avoiding real credential checks unless explicitly requested.

Review metrics: 2 noteworthy metrics.

  • Current-main diff: 19 files, +1659/-147. The current-main diff is broad enough that endpoint compatibility and provider-state tests need explicit review before merge.
  • API-token endpoint: 1 request target changed. The MiniMax API-token fetch path switches from apiRemainsURL to tokenPlanRemainsURL, which is the compatibility blocker.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

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

Rank-up moves:

  • [P2] Add the old MiniMax coding-plan remains endpoint fallback for API-token fetches.
  • [P2] Add focused regression tests proving both the new token-plan endpoint and old endpoint fallback paths.

Risk before merge

  • [P1] The API-token path now only calls /v1/token_plan/remains; existing MiniMax API-token users whose credentials still work only against /v1/api/openplatform/coding_plan/remains may lose usage display after upgrade.

Maintainer options:

  1. Keep both MiniMax API-token endpoints (recommended)
    Try the new token-plan endpoint first, then fall back to the existing coding-plan remains endpoint on compatible failure cases, with tests for both paths.
  2. Accept new-endpoint-only behavior
    Maintainers can intentionally require API-token users to move to the token-plan endpoint, but that needs explicit upgrade proof and docs because old explicit-token setups may stop showing usage.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Preserve MiniMax API-token compatibility by trying the existing `apiRemainsURL` coding-plan endpoint when `/v1/token_plan/remains` cannot provide usage, and add focused tests covering both new endpoint success and old endpoint fallback without changing web-session behavior.

Next step before merge

  • [P2] A narrow automated repair can preserve MiniMax API-token endpoint compatibility and add regression coverage without changing the main token-plan parser/UI direction.

Security
Cleared: No concrete security or supply-chain issue found; the diff does not add dependencies or scripts, and the new metadata helper rejects non-HTTPS host overrides before sending cookies.

Review findings

  • [P1] Preserve the existing MiniMax API-token endpoint fallback — Sources/CodexBarCore/Providers/MiniMax/MiniMaxUsageFetcher.swift:122
Review details

Best possible solution:

Keep the new token-plan parser and UI direction, but preserve the existing MiniMax API-token remains endpoint as a fallback with focused regression coverage before merge.

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

Yes for the review finding from source: current main sends API-token fetches to apiRemainsURL, while the PR head sends only to tokenPlanRemainsURL. I did not run live MiniMax probes because AGENTS.md requires avoiding real credential checks unless explicitly requested.

Is this the best way to solve the issue?

No, not yet: the new token-plan parser/UI is the right direction, but the narrowest maintainable solution preserves existing API-token endpoint compatibility while adding the new endpoint support.

Full review comments:

  • [P1] Preserve the existing MiniMax API-token endpoint fallback — Sources/CodexBarCore/Providers/MiniMax/MiniMaxUsageFetcher.swift:122
    This changes API-token fetches to only call /v1/token_plan/remains, while current main calls apiRemainsURL (/v1/api/openplatform/coding_plan/remains). Existing MINIMAX_CODING_API_KEY or explicit API-source users whose tokens still work only on the old endpoint would fail instead of falling back; try the new endpoint first but retain the old endpoint fallback with a regression test.
    Confidence: 0.89

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against dc4e4835bc6e.

Label changes

Label justifications:

  • P2: This is a bounded MiniMax provider display fix with a concrete merge blocker, not an urgent app-wide regression.
  • merge-risk: 🚨 compatibility: Merging as-is may break existing MiniMax API-token setups that still rely on the current coding-plan remains endpoint.
  • merge-risk: 🚨 auth-provider: The diff changes MiniMax token/cookie request routing and provider metadata fetch behavior, so credential-path compatibility needs proof before merge.
  • 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. Sufficient (screenshot): The PR body includes screenshots that directly show the after-change MiniMax menu/settings behavior, and the attached PNGs were inspected locally.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes screenshots that directly show the after-change MiniMax menu/settings behavior, and the attached PNGs were inspected locally.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body includes screenshots that directly show the after-change MiniMax menu/settings behavior, and the attached PNGs were inspected locally.
Evidence reviewed

Acceptance criteria:

  • [P1] swift test --filter MiniMaxTokenPlanChangeTests.
  • [P1] swift test --filter MiniMaxProviderTests.
  • [P1] swift test --filter MiniMaxMenuCardModelPlanTests.
  • [P1] make check.

What I checked:

Likely related people:

  • Peter Steinberger: Local blame and history show the current MiniMax API-token fetch path, provider introduction, token/cookie auth support, and billing-summary work primarily in Peter Steinberger commits. (role: feature owner / recent area contributor; confidence: high; commits: 17cdc54b26fa, 0bced475365d, da7acd03c03c; files: Sources/CodexBarCore/Providers/MiniMax/MiniMaxUsageFetcher.swift, Sources/CodexBarCore/Providers/MiniMax/MiniMaxAPIRegion.swift, Tests/CodexBarTests/MiniMaxProviderTests.swift)
  • Yuxin-Qiao: Recent merged MiniMax quota/parser commits and the related closed token-plan PR show strong adjacent context for the same remains/menu behavior. (role: recent adjacent contributor; confidence: medium; commits: 645ca833df31, 668f6be861d1, cea20ca18f27; files: Sources/CodexBarCore/Providers/MiniMax/MiniMaxUsageFetcher.swift, Tests/CodexBarTests/MiniMaxProviderTests.swift)
  • XWind: Beyond authoring this PR, XWind also appears in merged MiniMax multi-service usage history that this patch extends. (role: prior merged MiniMax contributor; confidence: medium; commits: 77b7367c9bd7; files: Sources/CodexBarCore/Providers/MiniMax/MiniMaxUsageFetcher.swift, Sources/CodexBar/MenuCardView+MiniMax.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.

Copy link
Copy Markdown

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

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: c30b7ee387

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

@@ -0,0 +1,43 @@
extension MiniMaxUsageSnapshot {
func withPlanNameIfAvailable(_ planName: String?) -> MiniMaxUsageSnapshot {
let cleaned = planName?.trimmingCharacters(in: .whitespacesAndNewlines)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Import Foundation before trimming plan names

This new file does not import Foundation, but trimmingCharacters(in:) and .whitespacesAndNewlines are Foundation APIs; Swift imports are file-scoped, so any target compiling this file fails before the MiniMax tests can run. Add import Foundation before using these helpers.

Useful? React with 👍 / 👎.

Comment on lines +92 to +95
if let tokenPlan = tokenPlans.min(by: { lhs, rhs in
lhs.rank == rhs.rank ? lhs.value.count < rhs.value.count : lhs.rank < rhs.rank
}) {
return tokenPlan.value
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 Prefer current plan over global token-plan strings

When the combo response contains strings for multiple token plans (for example a packages list with Plus/Max/Ultra alongside current_subscribe_title for the actual subscription), this global min ranking always chooses Plus over Max/Ultra regardless of the current plan. That mislabels Max or Ultra users as Plus; restrict the search to the current subscription fields first, or only fall back to all collected strings when no current-plan value is present.

Useful? React with 👍 / 👎.

@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. labels Jun 1, 2026
@XWind18 XWind18 force-pushed the codex/minimax-token-plan branch from c30b7ee to 485bd89 Compare June 1, 2026 11:44
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. and removed 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. labels Jun 1, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. 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.

4 participants