Skip to content

feat: expand MiniMax provider registry - #762

Open
octo-patch wants to merge 2 commits into
dataelement:mainfrom
octo-patch:octo/20260708-add-target-provider-model-to-existing-provider-registry-recvoNoEohPuXr
Open

feat: expand MiniMax provider registry#762
octo-patch wants to merge 2 commits into
dataelement:mainfrom
octo-patch:octo/20260708-add-target-provider-model-to-existing-provider-registry-recvoNoEohPuXr

Conversation

@octo-patch

Copy link
Copy Markdown

Reason: add target provider/model to existing provider registry.

  • Register MiniMax-M3 and MiniMax-M2.7 with the configured model metadata.
  • Expose global_en and cn_zh OpenAI-compatible and Anthropic-compatible endpoint choices in the provider manifest and LLM settings form.
  • Route configured MiniMax Anthropic-compatible bases through the native Anthropic client while preserving the OpenAI-compatible path.
  • Add focused registry and client-routing regression tests.

Checks:

  • git diff --check
  • ruff check backend/app/services/llm/client.py backend/tests/test_llm_provider_registry.py
  • PYTHONPATH=. pytest tests/test_llm_provider_registry.py
  • npm run build --prefix frontend

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

Copy link
Copy Markdown

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: 336d63b6d5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread backend/app/services/llm/client.py Outdated
Comment on lines +2275 to +2278
normalized_base_url = base_url.rstrip("/")
return any(
normalized_base_url == endpoint.anthropic_base_url.rstrip("/")
for endpoint in spec.endpoints

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 Normalize MiniMax Anthropic URLs before client selection

When an admin enters the full MiniMax Anthropic endpoint ending in /v1/messages (or /v1), this exact comparison returns false even though AnthropicClient._normalize_base_url() already supports those forms. The model then falls through to OpenAICompatibleClient, which appends /chat/completions and sends the wrong protocol to a URL like .../anthropic/v1/messages/chat/completions; normalize the candidate URL the same way before matching, or accept the documented full endpoint variants.

Useful? React with 👍 / 👎.

Strip trailing /v1/messages and /v1 suffixes before matching configured
Anthropic endpoints, so admin-entered full endpoint variants route to
AnthropicClient instead of falling through to OpenAICompatibleClient.
@octo-patch

Copy link
Copy Markdown
Author

Thanks for the review. I normalized the MiniMax Anthropic base URL before endpoint matching in backend/app/services/llm/client.py (_normalize_anthropic_base_url), so admin-entered full endpoints like .../anthropic/v1/messages and .../anthropic/v1 now route to AnthropicClient instead of falling through to the OpenAI-compatible client. I added parametrized routing tests in backend/tests/test_llm_provider_registry.py. I ran py_compile, ruff check (target files clean), and an isolated import of the registry/routing tests — all pass. The Drone CI step is failing on the git clone/git checkout step (fatal: unable to read tree) for both the original and the new commit, which looks like a runner/clone infrastructure issue rather than a build or test failure.

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.

1 participant