Skip to content

fix(minimax): filter modelRemains to coding-plan-relevant models#1285

Closed
LeoLin990405 wants to merge 1 commit into
steipete:mainfrom
LeoLin990405:fix/minimax-filter-auxiliary-services-clean
Closed

fix(minimax): filter modelRemains to coding-plan-relevant models#1285
LeoLin990405 wants to merge 1 commit into
steipete:mainfrom
LeoLin990405:fix/minimax-filter-auxiliary-services-clean

Conversation

@LeoLin990405
Copy link
Copy Markdown
Contributor

Summary

  • Filter model_remains returned by the MiniMax coding-plan API to coding-plan-relevant models only (general + text-generation models), dropping auxiliary services like video / image / music / speech before service construction.
  • Backward-compatible: when filtering leaves nothing (e.g. an account with only auxiliary services), the original list is used as fallback so the card still surfaces something.
  • Single-file change in MiniMaxUsageFetcher.parseMultiService.

Why

The MiniMax coding_plan/remains API currently returns one model_remains entry per accessible model:

{
  "model_remains": [
    {"model_name": "general", "current_interval_total_count": 0, "current_interval_usage_count": 0, "current_interval_remaining_percent": 99},
    {"model_name": "video",   "current_interval_total_count": 3, "current_interval_usage_count": 3, "current_interval_remaining_percent": 100}
  ]
}

On a coding-plan account that's idle in the current window (general is 0/0) but has used the free-tier video quota (3/3), the menu card currently surfaces only video 0/3 because:

  1. general has current_interval_total_count: 0, so makeServiceUsage skips it (guard total > 0).
  2. video has a non-zero allowance, so it survives makeServiceUsage and becomes the only visible service.

The user picked MiniMax for the coding plan, not video generation — surfacing video 0/3 · 0% used as the headline number is confusing and unhelpful.

Fix

Filter modelRemains to coding-plan-relevant models before service construction:

let codingPlanRemains = payload.data.modelRemains.filter { item in
    guard let name = item.modelName?.lowercased() else { return false }
    if name == "general" { return true }
    if let raw = item.modelName, Self.isTextGenerationModelName(raw) { return true }
    return false
}
let effectiveRemains = codingPlanRemains.isEmpty ? payload.data.modelRemains : codingPlanRemains

Then the rest of parseMultiService operates on effectiveRemains. The fallback to payload.data.modelRemains when filtering empties out preserves behavior for accounts that legitimately have only auxiliary services.

Tests

  • Existing MiniMax parsing tests remain green.

Validation

  • swift build --target CodexBarCore passes.
  • swift build --target CodexBar passes.
  • Real-behavior probe on a coding-plan account that returns [general(0/0), video(3/3)]: menu card now shows the coding plan window as primary (100% remaining placeholder for the idle window) instead of video 0/3 · 0% used.
  • Local swift test --filter MiniMaxUsageFetcher is blocked before reaching focused tests by the local Command Line Tools environment (KeyboardShortcuts #Preview macro plugins are unavailable without full Xcode); CI should be the test signal.

Scope

Single-file change in MiniMaxUsageFetcher.parseMultiService. No changes to API endpoint, auth, struct decoding, mapModelNameToServiceType, settings UI, or non-MiniMax providers.

The MiniMax API returns `model_remains` entries for every model the account
has access to (`general` coding plan + auxiliary services like `video`,
`image`, `music`, `speech`, etc.). When the user only intends to use the
coding plan, the menu card surfaces e.g. `video 0/3` because:

1. `general` (main coding plan) has `current_interval_total_count: 0` in idle
   windows, so `makeServiceUsage` skips it (`guard total > 0`).
2. `video` has a non-zero allowance from the free tier, so it survives and
   becomes the only visible service.

This is confusing — the user picked MiniMax for coding, not for video.

Fix: filter `modelRemains` down to "general" + text-generation models before
service construction. Other models (video/image/music/speech/etc.) are
dropped so they no longer clutter the card.

If filtering leaves nothing (e.g. an account with only auxiliary services),
the original list is used as fallback so the card still surfaces *something*
rather than going dark.

## Tests
- Existing MiniMax parsing tests remain green.

## Validation
- `swift build --target CodexBarCore` passes.
- `swift build --target CodexBar` passes.
- Real-behavior probe against a coding-plan account that returns
  `[general(0/0), video(3/3)]`: menu now shows the coding plan as primary
  (idle, 100% remaining placeholder) instead of `video 0/3 0% used`.
- Local `swift test --filter MiniMaxUsageFetcher` is blocked before reaching
  the new tests by the local Command Line Tools environment
  (`KeyboardShortcuts` `#Preview` macro plugins are unavailable without full
  Xcode); CI should be the test signal.

## Scope
Single-file change in `MiniMaxUsageFetcher.parseMultiService`. No changes
to API endpoint, auth, struct decoding, settings, or non-MiniMax providers.
@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented Jun 3, 2026

Codex review: needs changes before merge. Reviewed June 4, 2026, 3:01 AM ET / 07:01 UTC.

Summary
The PR filters MiniMax coding-plan model_remains to general and text-generation entries before building service usage, with an unfiltered fallback when no coding-plan entries remain.

Reproducibility: yes. From source inspection, a payload ordered [MiniMax-M1, general, video] passes both text and general through the new filter while leaving the text model first, so the summary fields still come from the text model.

Review metrics: 2 noteworthy metrics.

  • Patch surface: 1 Swift source file changed, 15 added / 2 deleted. The behavioral change is localized to MiniMax parser selection, so the remaining blocker is focused and repairable.
  • Regression coverage: 0 test files changed. The repository policy asks parser changes to be mirrored with focused tests, and the uncovered ordering case is the current blocker.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🐚 platinum hermit
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:

  • Partition or sort filtered MiniMax remains so general is first whenever present.
  • [P2] Add focused parser tests for mixed coding-plan/auxiliary payloads, text-before-general ordering, and auxiliary-only fallback.

Risk before merge

  • [P1] No parser regression fixture currently covers general mixed with auxiliary quotas or general appearing after a text model, so the ordering bug could pass CI unnoticed.
  • [P1] I did not run Swift tests because this read-only review must not create build artifacts; validation is from source inspection, git history, and contributor-provided live output.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the auxiliary-service filter, but explicitly put general first when present and add MiniMax parser tests for mixed coding-plan, text-before-general, and auxiliary-only fallback payloads.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] A narrow automated repair can preserve the filter while fixing general ordering and adding focused MiniMax parser tests.

Security
Cleared: The diff only changes in-memory MiniMax usage parsing and does not alter auth, credential storage, dependencies, scripts, or network endpoints.

Review findings

  • [P2] Prioritize general before text models — Sources/CodexBarCore/Providers/MiniMax/MiniMaxUsageFetcher.swift:795-801
Review details

Best possible solution:

Keep the auxiliary-service filter, but explicitly put general first when present and add MiniMax parser tests for mixed coding-plan, text-before-general, and auxiliary-only fallback payloads.

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

Yes. From source inspection, a payload ordered [MiniMax-M1, general, video] passes both text and general through the new filter while leaving the text model first, so the summary fields still come from the text model.

Is this the best way to solve the issue?

No. Filtering auxiliary entries is the right direction, but the implementation should partition or sort the filtered list so general is first and should add focused parser coverage for the mixed and fallback cases.

Full review comments:

  • [P2] Prioritize general before text models — Sources/CodexBarCore/Providers/MiniMax/MiniMaxUsageFetcher.swift:795-801
    The new filter keeps MiniMax's server order, despite the comment saying general should be first. If a recognized text model appears before general, effectiveRemains.first still drives the legacy summary fields from that model quota instead of the main coding-plan quota; partition or sort so general is first and cover that payload in parser tests.
    Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This is a normal-priority MiniMax provider display bug with limited blast radius and a clear parser-level repair.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR discussion now includes copied after-fix CLI output from a real MiniMax coding-plan account showing the prompt quota surfaced instead of video.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR discussion now includes copied after-fix CLI output from a real MiniMax coding-plan account showing the prompt quota surfaced instead of video.
  • remove status: 📣 needs proof: Current PR status label is status: ⏳ waiting on author.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P2: This is a normal-priority MiniMax provider display bug with limited blast radius and a clear parser-level repair.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR discussion now includes copied after-fix CLI output from a real MiniMax coding-plan account showing the prompt quota surfaced instead of video.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR discussion now includes copied after-fix CLI output from a real MiniMax coding-plan account showing the prompt quota surfaced instead of video.
Evidence reviewed

Acceptance criteria:

  • [P1] swift test --filter MiniMaxUsageParserTests.
  • [P1] make check.

What I checked:

Likely related people:

  • Peter Steinberger: Current-main blame for the MiniMax parser snapshot and release integration points to commit 723734e, and history shows multiple recent MiniMax parser/auth follow-ups. (role: recent area contributor; confidence: high; commits: 723734ef3422, 1cd1641b9cea, b4f8af1e4315; files: Sources/CodexBarCore/Providers/MiniMax/MiniMaxUsageFetcher.swift, Tests/CodexBarTests/MiniMaxProviderTests.swift, docs/minimax.md)
  • Yuxin Qiao: Commit 645ca83 changed the MiniMax remains-to-used mapping, quota-style cards, weekly text window behavior, and related parser tests central to this PR. (role: quota parser contributor; confidence: high; commits: 645ca833df31, 668f6be861d1, cea20ca18f27; files: Sources/CodexBarCore/Providers/MiniMax/MiniMaxUsageFetcher.swift, Tests/CodexBarTests/MiniMaxProviderTests.swift)
  • XWind: Commit 77b7367 introduced MiniMax multi-service usage parsing and tests, which is the behavior this PR narrows for coding-plan displays. (role: feature introducer; confidence: medium; commits: 77b7367c9bd7; files: Sources/CodexBarCore/Providers/MiniMax/MiniMaxUsageFetcher.swift, Tests/CodexBarTests/MiniMaxProviderTests.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: 732c55bb28

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

Comment on lines +795 to +801
let codingPlanRemains = payload.data.modelRemains.filter { item in
guard let name = item.modelName?.lowercased() else { return false }
if name == "general" { return true }
if let raw = item.modelName, Self.isTextGenerationModelName(raw) { return true }
return false
}
let effectiveRemains = codingPlanRemains.isEmpty ? payload.data.modelRemains : codingPlanRemains
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 Prioritize general quota after filtering

When MiniMax returns both a recognized text model and general but the text model appears first, this filtered array preserves the server order, so effectiveRemains.first below still uses the model-specific quota for the legacy summary fields instead of the main coding-plan quota described in the comment. That keeps the menu on the wrong totals for those payloads; reorder the filtered results so general is selected first before falling back to other text-generation models.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 3, 2026
@LeoLin990405
Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Real-behavior proof on a coding-plan account where the MiniMax API returns general (idle this window, total=0) + video (auxiliary free tier):

Before this PR (master): the menu card displayed video 0/3 · 0% used as the headline number because makeServiceUsage skipped general for total == 0 and video was the only surviving service, even though the user signed up for the coding plan, not video generation.

After this PR (commit 732c55bb):

$ codexbar usage --provider minimax
== MiniMax (api) ==
Prompts: 100% left [============]
Resets in 1h 38m

The menu now surfaces the coding plan window (general model) as the primary metric and drops the auxiliary video from the displayed service list. When filtering returns nothing (account with only auxiliary services), the original modelRemains list is the fallback so the card still surfaces something rather than going dark.

Menu screenshot will be attached in a follow-up comment from the PR author (gh CLI doesn't support image upload).

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented Jun 4, 2026

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added 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. P2 Normal priority bug or improvement with limited blast radius. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 4, 2026
@LeoLin990405
Copy link
Copy Markdown
Contributor Author

Superseded by #1266, which takes a more comprehensive approach to the underlying MiniMax model_remains handling — explicit general / video model naming, shouldRenderWeeklyWindow instead of the broader isTextGenerationModelName, and most-constrained-window resolution for the auto menu metric. That's the proper architectural fix; this PR was a narrow filter-only workaround.

Closing to consolidate review effort. Thanks @XWind18 @Yuxin-Qiao @coygeek for the deeper work in #1266 — happy to test against #1266's branch once it lands.

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. 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.

1 participant