Skip to content

fix(minimax): add region selector for international and China API endpoints#128

Merged
hanrw merged 1 commit intotddworks:mainfrom
BryanQQYue:fix/125-minimax-region-url
Feb 25, 2026
Merged

fix(minimax): add region selector for international and China API endpoints#128
hanrw merged 1 commit intotddworks:mainfrom
BryanQQYue:fix/125-minimax-region-url

Conversation

@BryanQQYue
Copy link
Copy Markdown
Contributor

@BryanQQYue BryanQQYue commented Feb 25, 2026

Summary

  • Add MiniMaxRegion enum supporting international (minimax.io) and China (minimaxi.com) API endpoints
  • Add region selector (segmented picker) in MiniMax settings UI so users can choose their region
  • Update MiniMaxUsageProbe and MiniMaxProvider to dynamically resolve URLs based on configured region
  • Rename minimaxiXxx methods to minimaxXxx for consistent naming convention

Context

MiniMax provider previously hardcoded minimaxi.com URLs, which only works for China region users. International users at minimax.io could not configure the provider properly. See MiniMax docs for the two region endpoints.

Closes #125

Test plan

  • Build passes (tuist build)
  • All tests pass (tuist test), including new region-specific tests
  • Verify international region resolves to api.minimax.io
  • Verify China region resolves to api.minimaxi.com
  • Verify region picker persists selection across app restarts
  • Verify "Open MiniMax API Keys" link opens correct region URL

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added MiniMax region selection in Settings (International / China) and region-aware links (API, dashboard, API keys).
    • Region-aware usage checking and persistence for selected region.
  • Bug Fixes / UX

    • Save & Test now persists auth env var before testing.
    • Minor UI text updates for consistent MiniMax terminology.
  • Tests

    • Added region-focused tests for MiniMax region values and usage probe behavior.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 25, 2026

📝 Walkthrough

Walkthrough

Adds MiniMax region support (international/china), surfaces region in settings UI, standardizes identifier names from "minimaxi" → "minimax", and makes provider/probe URLs region-aware while updating persistence and tests accordingly.

Changes

Cohort / File(s) Summary
Region model
Sources/Domain/Provider/MiniMax/MiniMaxRegion.swift
New public enum MiniMaxRegion with .international and .china and region-specific URLs/strings (apiBaseURL, platformURL, apiKeysURL, dashboardURL, codingPlanRemainsURL).
Settings protocol
Sources/Domain/Provider/ProviderSettingsRepository.swift
Added minimaxRegion() and setMinimaxRegion(_:); renamed MiniMax API methods from minimaxi*minimax* in protocol surface.
UserDefaults implementation
Sources/Infrastructure/Storage/UserDefaultsProviderSettingsRepository.swift
Persist/read minimaxRegion (new key Keys.minimaxRegion); added setMinimaxRegion(_:); renamed stored methods to minimax* variants (save/get/delete/has/set env var).
Provider & probe
Sources/Domain/Provider/MiniMax/MiniMaxProvider.swift, Sources/Infrastructure/MiniMax/MiniMaxUsageProbe.swift
Dashboard and usage API URLs now resolved from settingsRepository.minimaxRegion(); probe API URL became instance property; updated minimax* method usages.
UI / Settings view
Sources/App/Views/SettingsView.swift
Added region segmented control and miniMaxRegion state; load/save region and minimax auth env var; renamed minimaxi→minimax references; API keys link uses region-specific URL.
Tests
Tests/InfrastructureTests/MiniMax/MiniMaxUsageProbeTests.swift, Tests/DomainTests/Provider/MiniMax/MiniMaxRegionTests.swift
Updated tests for region handling and renamed methods; added new MiniMaxRegion tests validating properties and URLs; added region-specific probe/network test flows.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant SettingsView as SettingsView(UI)
    participant Repo as ProviderSettingsRepository
    participant Provider as MiniMaxProvider
    participant Probe as MiniMaxUsageProbe
    participant Net as Network

    User->>SettingsView: Select region / Save & Test
    SettingsView->>Repo: setMinimaxRegion(region)
    SettingsView->>Repo: setMinimaxAuthEnvVar(envVar)
    SettingsView->>Provider: request dashboard/api URLs
    Provider->>Repo: minimaxRegion()
    SettingsView->>Probe: run probe()
    Probe->>Repo: minimaxAuthEnvVar()
    Probe->>Repo: getMinimaxApiKey()
    Probe->>Provider: request api base URL
    Provider->>Repo: minimaxRegion()
    Probe->>Net: HTTP request to region-specific API URL
    Net-->>Probe: HTTP response
    Probe-->>SettingsView: probe result
    SettingsView-->>User: display result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 I hopped through code with tiny paws,

Regions added, URLs without flaws,
China or International — pick a way,
Keys renamed, tests now say hooray,
A carrot-coded cheer for the new day 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding a region selector for MiniMax API endpoints (international vs China).
Linked Issues check ✅ Passed The PR fully addresses issue #125 requirements: adds MiniMaxRegion enum supporting both endpoints, implements region selector UI, updates API URLs dynamically, renames minimaxi* methods to minimax* for consistency, and includes comprehensive tests.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #125: MiniMaxRegion enum, region selector UI, API URL resolution, method renaming, region-aware probes, and related tests. No unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
Tests/InfrastructureTests/MiniMax/MiniMaxUsageProbeTests.swift (1)

30-39: Cover the “region not configured” path explicitly in tests.

makeProbe always sets a region (default .china), so fallback behavior for missing providerConfig.minimaxRegion is never exercised.

🧪 Suggested test-helper tweak
 private func makeProbe(
     apiKey: String? = nil,
     envVar: String = "",
-    region: MiniMaxRegion = .china,
+    region: MiniMaxRegion? = nil,
     networkClient: any NetworkClient = MockNetworkClient()
 ) -> MiniMaxUsageProbe {
     let defaults = UserDefaults(suiteName: "MiniMaxProbeTests.\(UUID().uuidString)")!
     let settingsRepository = UserDefaultsProviderSettingsRepository(userDefaults: defaults)
-    settingsRepository.setMinimaxRegion(region)
+    if let region {
+        settingsRepository.setMinimaxRegion(region)
+    }
     settingsRepository.setMinimaxAuthEnvVar(envVar)
     if let apiKey {
         settingsRepository.saveMinimaxApiKey(apiKey)
     }
`@Test`
func `apiURL handles missing region configuration`() {
    let probe = makeProbe(apiKey: "test-key", region: nil)
    // Assert repository contract for absent region key.
}

Also applies to: 155-161

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Tests/InfrastructureTests/MiniMax/MiniMaxUsageProbeTests.swift` around lines
30 - 39, makeProbe currently forces a region by using parameter region:
MiniMaxRegion = .china and always calls
settingsRepository.setMinimaxRegion(region), which prevents testing the "region
not configured" path; change the makeProbe signature to accept an optional
region (region: MiniMaxRegion? = .china) and update the body to only call
settingsRepository.setMinimaxRegion(...) when region is non-nil (otherwise leave
the repo unset or call the repo's removal/clear method), so tests can pass nil
(e.g., makeProbe(region: nil)) to exercise MiniMaxUsageProbe behavior when
providerConfig.minimaxRegion is missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Sources/Infrastructure/Storage/UserDefaultsProviderSettingsRepository.swift`:
- Around line 254-259: The current minimaxRegion() always returns .international
when the stored string is missing, which breaks legacy users; change the logic
in minimaxRegion() to detect a genuinely missing key by using
userDefaults.object(forKey: Keys.minimaxRegion) == nil and if the key is missing
return the legacy .minimaxi value, otherwise read the string
(userDefaults.string(forKey: Keys.minimaxRegion)) and map it via
MiniMaxRegion(rawValue: ...) falling back to .international only when a value
exists but is unrecognized; reference Keys.minimaxRegion, minimaxRegion(),
userDefaults, and MiniMaxRegion.

---

Nitpick comments:
In `@Tests/InfrastructureTests/MiniMax/MiniMaxUsageProbeTests.swift`:
- Around line 30-39: makeProbe currently forces a region by using parameter
region: MiniMaxRegion = .china and always calls
settingsRepository.setMinimaxRegion(region), which prevents testing the "region
not configured" path; change the makeProbe signature to accept an optional
region (region: MiniMaxRegion? = .china) and update the body to only call
settingsRepository.setMinimaxRegion(...) when region is non-nil (otherwise leave
the repo unset or call the repo's removal/clear method), so tests can pass nil
(e.g., makeProbe(region: nil)) to exercise MiniMaxUsageProbe behavior when
providerConfig.minimaxRegion is missing.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd46f53 and 8535b87.

📒 Files selected for processing (7)
  • Sources/App/Views/SettingsView.swift
  • Sources/Domain/Provider/MiniMax/MiniMaxProvider.swift
  • Sources/Domain/Provider/MiniMax/MiniMaxRegion.swift
  • Sources/Domain/Provider/ProviderSettingsRepository.swift
  • Sources/Infrastructure/MiniMax/MiniMaxUsageProbe.swift
  • Sources/Infrastructure/Storage/UserDefaultsProviderSettingsRepository.swift
  • Tests/InfrastructureTests/MiniMax/MiniMaxUsageProbeTests.swift

Comment on lines +254 to +259
public func minimaxRegion() -> MiniMaxRegion {
guard let rawValue = userDefaults.string(forKey: Keys.minimaxRegion) else {
return .international // Default to international (默认国际版)
}
return MiniMaxRegion(rawValue: rawValue) ?? .international
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Preserve legacy MiniMax behavior when region is unset.

When providerConfig.minimaxRegion is missing (typical on upgrade), defaulting directly to .international can break existing users who were previously on minimaxi.com only.

💡 Suggested migration-safe fallback
 public func minimaxRegion() -> MiniMaxRegion {
-    guard let rawValue = userDefaults.string(forKey: Keys.minimaxRegion) else {
-        return .international // Default to international (默认国际版)
-    }
-    return MiniMaxRegion(rawValue: rawValue) ?? .international
+    if let rawValue = userDefaults.string(forKey: Keys.minimaxRegion),
+       let region = MiniMaxRegion(rawValue: rawValue) {
+        return region
+    }
+
+    // One-time inference for pre-region installs (legacy behavior was China endpoints).
+    let inferred: MiniMaxRegion =
+        (userDefaults.object(forKey: Keys.minimaxiApiKey) != nil ||
+         userDefaults.object(forKey: Keys.minimaxiAuthEnvVar) != nil)
+        ? .china : .international
+
+    userDefaults.set(inferred.rawValue, forKey: Keys.minimaxRegion)
+    return inferred
 }
📝 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
public func minimaxRegion() -> MiniMaxRegion {
guard let rawValue = userDefaults.string(forKey: Keys.minimaxRegion) else {
return .international // Default to international (默认国际版)
}
return MiniMaxRegion(rawValue: rawValue) ?? .international
}
public func minimaxRegion() -> MiniMaxRegion {
if let rawValue = userDefaults.string(forKey: Keys.minimaxRegion),
let region = MiniMaxRegion(rawValue: rawValue) {
return region
}
// One-time inference for pre-region installs (legacy behavior was China endpoints).
let inferred: MiniMaxRegion =
(userDefaults.object(forKey: Keys.minimaxiApiKey) != nil ||
userDefaults.object(forKey: Keys.minimaxiAuthEnvVar) != nil)
? .china : .international
userDefaults.set(inferred.rawValue, forKey: Keys.minimaxRegion)
return inferred
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Infrastructure/Storage/UserDefaultsProviderSettingsRepository.swift`
around lines 254 - 259, The current minimaxRegion() always returns
.international when the stored string is missing, which breaks legacy users;
change the logic in minimaxRegion() to detect a genuinely missing key by using
userDefaults.object(forKey: Keys.minimaxRegion) == nil and if the key is missing
return the legacy .minimaxi value, otherwise read the string
(userDefaults.string(forKey: Keys.minimaxRegion)) and map it via
MiniMaxRegion(rawValue: ...) falling back to .international only when a value
exists but is unrecognized; reference Keys.minimaxRegion, minimaxRegion(),
userDefaults, and MiniMaxRegion.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 25, 2026

Codecov Report

❌ Patch coverage is 95.55556% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.10%. Comparing base (cd46f53) to head (e3dd249).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
...orage/UserDefaultsProviderSettingsRepository.swift 85.71% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #128      +/-   ##
==========================================
+ Coverage   77.71%   78.10%   +0.39%     
==========================================
  Files          78       79       +1     
  Lines        6013     6084      +71     
==========================================
+ Hits         4673     4752      +79     
+ Misses       1340     1332       -8     
Files with missing lines Coverage Δ
...ources/Domain/Provider/MiniMax/MiniMaxRegion.swift 100.00% <100.00%> (ø)
...s/Domain/Provider/ProviderSettingsRepository.swift 100.00% <ø> (ø)
...ces/Infrastructure/MiniMax/MiniMaxUsageProbe.swift 95.87% <100.00%> (+0.08%) ⬆️
...orage/UserDefaultsProviderSettingsRepository.swift 84.76% <85.71%> (+0.85%) ⬆️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@BryanQQYue BryanQQYue force-pushed the fix/125-minimax-region-url branch from 8535b87 to 93abd26 Compare February 25, 2026 03:03
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
Tests/InfrastructureTests/MiniMax/MiniMaxUsageProbeTests.swift (1)

184-207: Consider asserting the request URL to fully validate region routing.

This test verifies response parsing but not that probe() actually sends the request to https://api.minimax.io/.... Since .request(.any) matches unconditionally, the international URL is never confirmed end-to-end. A captured-URL assertion would make the test a true integration check for region routing.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Tests/InfrastructureTests/MiniMax/MiniMaxUsageProbeTests.swift` around lines
184 - 207, The test currently stubs network with
given(mockNetwork).request(.any) so it never verifies which endpoint was called;
change the stub to capture the actual URL request from the MockNetworkClient and
assert it equals the international API URL. Concretely: in the test for probe
uses international API URL when region is international replace the
.request(.any) matcher with a capture/matcher that records the Request/URL
argument (for example capture requestArg in the mock setup for
MockNetworkClient.request), keep returning (responseData, httpResponse), then
after calling let snapshot = try await probe.probe() add an assertion that the
capturedRequest.url?.absoluteString (or capturedRequest.url) equals
"https://api.minimax.io/v1/api/openplatform/coding_plan/remains" (or contains
that host/path). Use existing helpers makeProbe(apiKey:region:networkClient:)
and probe.probe() to locate where to add the capture and assertion.
Sources/App/Views/SettingsView.swift (1)

1111-1113: Region change doesn't trigger a provider refresh, unlike other mode pickers.

codexProbeMode, kimiProbeMode, and claudeProbeMode all call monitor.refresh(providerId:) in their onChange handlers. The miniMaxRegion picker persists the region but doesn't refresh, so the displayed quota data may reflect the old region's endpoint until the next background sync or manual "Save & Test". Consider adding a refresh here for consistency.

📝 Proposed fix
 .onChange(of: miniMaxRegion) { _, newValue in
     UserDefaultsProviderSettingsRepository.shared.setMinimaxRegion(newValue)
+    Task {
+        await monitor.refresh(providerId: ProviderID.minimax)
+    }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/App/Views/SettingsView.swift` around lines 1111 - 1113, The
miniMaxRegion onChange only persists the new region but doesn't refresh the
provider; update the handler in SettingsView so after calling
UserDefaultsProviderSettingsRepository.shared.setMinimaxRegion(newValue) it also
calls monitor.refresh(providerId: <Minimax provider id constant>) to trigger an
immediate refresh (use the same Minimax provider id constant used elsewhere in
the repo, e.g., the one referenced when updating
codexProbeMode/kimiProbeMode/claudeProbeMode).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Sources/App/Views/SettingsView.swift`:
- Line 68: The `@State` default for miniMaxRegion is wrong for legacy users:
change the initial value of miniMaxRegion (declared as `@State` private var
miniMaxRegion: MiniMaxRegion) from .international to .china so the view does not
briefly show the incorrect selection before the value loaded in minimaxRegion()
is applied in .onAppear; update the initializer only (not the persisted logic)
to match the legacy default.

In `@Sources/Domain/Provider/ProviderSettingsRepository.swift`:
- Around line 198-199: Update the documentation for minimaxRegion() to reflect
the actual persisted default (.china) rather than “default: international”;
change the doc comment in ProviderSettingsRepository.swift for the
minimaxRegion() method to state that legacy/persisted default is .china (and
mention that SettingsView.swift currently uses an `@State` provisional
.international until onAppear corrects it) so the protocol contract matches the
concrete behavior.

---

Nitpick comments:
In `@Sources/App/Views/SettingsView.swift`:
- Around line 1111-1113: The miniMaxRegion onChange only persists the new region
but doesn't refresh the provider; update the handler in SettingsView so after
calling UserDefaultsProviderSettingsRepository.shared.setMinimaxRegion(newValue)
it also calls monitor.refresh(providerId: <Minimax provider id constant>) to
trigger an immediate refresh (use the same Minimax provider id constant used
elsewhere in the repo, e.g., the one referenced when updating
codexProbeMode/kimiProbeMode/claudeProbeMode).

In `@Tests/InfrastructureTests/MiniMax/MiniMaxUsageProbeTests.swift`:
- Around line 184-207: The test currently stubs network with
given(mockNetwork).request(.any) so it never verifies which endpoint was called;
change the stub to capture the actual URL request from the MockNetworkClient and
assert it equals the international API URL. Concretely: in the test for probe
uses international API URL when region is international replace the
.request(.any) matcher with a capture/matcher that records the Request/URL
argument (for example capture requestArg in the mock setup for
MockNetworkClient.request), keep returning (responseData, httpResponse), then
after calling let snapshot = try await probe.probe() add an assertion that the
capturedRequest.url?.absoluteString (or capturedRequest.url) equals
"https://api.minimax.io/v1/api/openplatform/coding_plan/remains" (or contains
that host/path). Use existing helpers makeProbe(apiKey:region:networkClient:)
and probe.probe() to locate where to add the capture and assertion.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8535b87 and 93abd26.

📒 Files selected for processing (7)
  • Sources/App/Views/SettingsView.swift
  • Sources/Domain/Provider/MiniMax/MiniMaxProvider.swift
  • Sources/Domain/Provider/MiniMax/MiniMaxRegion.swift
  • Sources/Domain/Provider/ProviderSettingsRepository.swift
  • Sources/Infrastructure/MiniMax/MiniMaxUsageProbe.swift
  • Sources/Infrastructure/Storage/UserDefaultsProviderSettingsRepository.swift
  • Tests/InfrastructureTests/MiniMax/MiniMaxUsageProbeTests.swift
🚧 Files skipped from review as they are similar to previous changes (2)
  • Sources/Domain/Provider/MiniMax/MiniMaxRegion.swift
  • Sources/Domain/Provider/MiniMax/MiniMaxProvider.swift

@hanrw
Copy link
Copy Markdown
Member

hanrw commented Feb 25, 2026

Thanks @BryanQQYue, but can we add some uts for MiniMaxRegion which's missing?

…API endpoints (tddworks#125)

MiniMax provider previously hardcoded minimaxi.com URLs, which only works
for China region users. This adds a MiniMaxRegion enum with international
(minimax.io) and china (minimaxi.com) options, a region picker in Settings,
and dynamic URL resolution throughout the probe and provider layers.

- Add MiniMaxRegion enum with region-specific URLs
- Add region selector (segmented picker) in MiniMax settings UI
- Rename minimaxiXxx methods to minimaxXxx for consistent naming
- Update MiniMaxUsageProbe to resolve API URL from configured region
- Update MiniMaxProvider.dashboardURL to use region-aware URL
- Add region-specific tests for API URL resolution

Closes tddworks#125
@BryanQQYue BryanQQYue force-pushed the fix/125-minimax-region-url branch from 93abd26 to e3dd249 Compare February 25, 2026 07:12
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (3)
Sources/Domain/Provider/ProviderSettingsRepository.swift (1)

197-222: LGTM — past doc-comment mismatch resolved, protocol surface is consistent.

The minimaxRegion() doc comment now correctly documents .china as the legacy default, the sub-protocol pattern is correctly applied, and all renamed methods align with the implementation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Domain/Provider/ProviderSettingsRepository.swift` around lines 197 -
222, No code changes needed: the MiniMaxSettingsRepository protocol (methods
minimaxRegion(), setMinimaxRegion(_:), minimaxAuthEnvVar(),
setMinimaxAuthEnvVar(_:), saveMinimaxApiKey(_:), getMinimaxApiKey(),
deleteMinimaxApiKey(), hasMinimaxApiKey()) is consistent with implementation and
the doc comment correctly documents .china as the legacy default, so leave these
declarations as-is.
Sources/Infrastructure/Storage/UserDefaultsProviderSettingsRepository.swift (1)

254-261: LGTM — legacy default correctly returns .china in both fallback paths.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Infrastructure/Storage/UserDefaultsProviderSettingsRepository.swift`
around lines 254 - 261, No change required: the minimaxRegion() method correctly
handles legacy defaults by returning .china when Keys.minimaxRegion is missing
or when rawValue fails to initialize a MiniMaxRegion; leave the function
minimaxRegion() and its use of userDefaults.string(forKey: Keys.minimaxRegion)
and MiniMaxRegion(rawValue:) as-is.
Sources/App/Views/SettingsView.swift (1)

68-68: LGTM — @State default .china now matches the persisted legacy default.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/App/Views/SettingsView.swift` at line 68, No change required—the
`@State` property miniMaxRegion should keep its default set to .china to match the
persisted legacy default; verify that the declaration "@State private var
miniMaxRegion: MiniMaxRegion = .china" remains as-is and ensure no other
initializers override this default (check any init() or onAppear code that might
set miniMaxRegion).
🧹 Nitpick comments (1)
Sources/App/Views/SettingsView.swift (1)

1111-1116: Add an AppLog call for region changes to match observability of other picker changes.

copilotProbeMode (Line 1512) and claudeProbeMode (Line 669) both emit AppLog on change; the region picker has no log, making it harder to diagnose region-related issues in the field.

🔍 Proposed addition
 .onChange(of: miniMaxRegion) { _, newValue in
+    AppLog.providers.info("MiniMax region changed to \(newValue.rawValue)")
     UserDefaultsProviderSettingsRepository.shared.setMinimaxRegion(newValue)
     Task {
         await monitor.refresh(providerId: ProviderID.minimax)
     }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/App/Views/SettingsView.swift` around lines 1111 - 1116, Add an AppLog
call inside the miniMaxRegion onChange handler to mirror other pickers: when
miniMaxRegion changes (inside the closure that calls
UserDefaultsProviderSettingsRepository.shared.setMinimaxRegion(newValue) and
Task { await monitor.refresh(providerId: ProviderID.minimax) }), emit an AppLog
entry including the newValue and a short context string like "minimax region
changed" so region changes are observable in logs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@Sources/App/Views/SettingsView.swift`:
- Line 68: No change required—the `@State` property miniMaxRegion should keep its
default set to .china to match the persisted legacy default; verify that the
declaration "@State private var miniMaxRegion: MiniMaxRegion = .china" remains
as-is and ensure no other initializers override this default (check any init()
or onAppear code that might set miniMaxRegion).

In `@Sources/Domain/Provider/ProviderSettingsRepository.swift`:
- Around line 197-222: No code changes needed: the MiniMaxSettingsRepository
protocol (methods minimaxRegion(), setMinimaxRegion(_:), minimaxAuthEnvVar(),
setMinimaxAuthEnvVar(_:), saveMinimaxApiKey(_:), getMinimaxApiKey(),
deleteMinimaxApiKey(), hasMinimaxApiKey()) is consistent with implementation and
the doc comment correctly documents .china as the legacy default, so leave these
declarations as-is.

In `@Sources/Infrastructure/Storage/UserDefaultsProviderSettingsRepository.swift`:
- Around line 254-261: No change required: the minimaxRegion() method correctly
handles legacy defaults by returning .china when Keys.minimaxRegion is missing
or when rawValue fails to initialize a MiniMaxRegion; leave the function
minimaxRegion() and its use of userDefaults.string(forKey: Keys.minimaxRegion)
and MiniMaxRegion(rawValue:) as-is.

---

Nitpick comments:
In `@Sources/App/Views/SettingsView.swift`:
- Around line 1111-1116: Add an AppLog call inside the miniMaxRegion onChange
handler to mirror other pickers: when miniMaxRegion changes (inside the closure
that calls
UserDefaultsProviderSettingsRepository.shared.setMinimaxRegion(newValue) and
Task { await monitor.refresh(providerId: ProviderID.minimax) }), emit an AppLog
entry including the newValue and a short context string like "minimax region
changed" so region changes are observable in logs.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 93abd26 and e3dd249.

📒 Files selected for processing (8)
  • Sources/App/Views/SettingsView.swift
  • Sources/Domain/Provider/MiniMax/MiniMaxProvider.swift
  • Sources/Domain/Provider/MiniMax/MiniMaxRegion.swift
  • Sources/Domain/Provider/ProviderSettingsRepository.swift
  • Sources/Infrastructure/MiniMax/MiniMaxUsageProbe.swift
  • Sources/Infrastructure/Storage/UserDefaultsProviderSettingsRepository.swift
  • Tests/DomainTests/Provider/MiniMax/MiniMaxRegionTests.swift
  • Tests/InfrastructureTests/MiniMax/MiniMaxUsageProbeTests.swift
🚧 Files skipped from review as they are similar to previous changes (2)
  • Sources/Infrastructure/MiniMax/MiniMaxUsageProbe.swift
  • Sources/Domain/Provider/MiniMax/MiniMaxRegion.swift

@hanrw hanrw merged commit 19dc869 into tddworks:main Feb 25, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MiniMax Provider doesn't work

2 participants