feat(bailian): add native support for Alibaba Cloud Bailian (百炼)#392
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Alibaba Cloud Bailian (百炼 / DashScope) as a new LLM provider to GoModel. The implementation wraps an OpenAI-compatible provider with a ChangesAlibaba Cloud Bailian Provider
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR adds Alibaba Cloud Bailian and Xiaomi MiMo provider support. The main changes are:
Confidence Score: 4/5This is close, but the already noted provider-edge fixes should land before merging.
internal/providers/bailian/bailian.go, internal/providers/xiaomi/audio.go, and internal/core/chat_content.go already have the relevant follow-up comments. Important Files Changed
Reviews (2): Last reviewed commit: "feat(bailian): add native support for Al..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/gomodel/docs/docs.go`:
- Line 7012: The Description field on line 7012 of cmd/gomodel/docs/docs.go
lists the supported LLM providers but is missing the newly added Bailian
provider. Add Bailian to the comma-separated list of providers in the
Description string to ensure the Swagger documentation accurately reflects all
supported backends available through the AI gateway.
In `@internal/cache/redis.go`:
- Line 32: The NewRedisStore function now has a side effect by calling
installRedisLogger(), but there is no test verifying this function is actually
invoked. Add a test in the redis package that temporarily substitutes
installRedisLogger with a test stub or spy function to verify that NewRedisStore
calls it during initialization. This ensures the constructor-level behavior
change is properly covered by tests as per coding guidelines.
In `@internal/core/chat_content.go`:
- Around line 53-64: The validation in the dataURIHasMediaType function is too
permissive. The current check using strings.Contains(mediaType, "/") accepts
malformed media types like empty types or subtypes (e.g., data:/;base64,... or
data:audio/;base64,...). Replace the strings.Contains check with a proper
validation that splits mediaType by "/" and verifies that both the type part
(before the slash) and subtype part (after the slash) are non-empty strings
before returning true.
In `@internal/providers/bailian/bailian_test.go`:
- Around line 289-294: The tests TestDefaultBaseURL (lines 289-294) and
TestNew_UsesRegistrationAndDefaultBaseURL (lines 352-361) currently only verify
that the provider constructor returns a non-nil value, which is insufficient to
catch default URL regressions. Strengthen both tests by adding assertions that
verify the actual default base URL value is correctly set on the provider
object. For TestDefaultBaseURL, after constructing the provider with
NewWithHTTPClient, assert that the provider's BaseURL field or accessor method
equals the expected default base URL. Apply the same fix to
TestNew_UsesRegistrationAndDefaultBaseURL, ensuring it validates the default
base URL is properly configured when using registration and default URL
initialization.
In `@internal/providers/xiaomi/audio.go`:
- Around line 203-204: The MIME type validation using strings.HasPrefix with
"audio/" is case-sensitive, causing valid MIME types like "Audio/WAV" to be
incorrectly rejected. In the conditional block checking the FileContentType,
normalize the contentType value to lowercase using strings.ToLower before
performing the strings.HasPrefix check against "audio/". This ensures that MIME
types with any casing variation are properly recognized as audio types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d68c65a9-dbd9-4349-8676-6c4710b5f21f
📒 Files selected for processing (27)
.env.template.gitignoreCLAUDE.mdREADME.mdcmd/gomodel/docs/docs.gocmd/gomodel/main.goconfig/config.example.yamldocs/docs.jsondocs/providers/bailian.mdxdocs/providers/overview.mdxdocs/providers/xiaomi.mdxinternal/cache/redis.gointernal/cache/redis_logger.gointernal/cache/redis_logger_test.gointernal/core/chat_content.gointernal/core/chat_content_test.gointernal/providers/bailian/bailian.gointernal/providers/bailian/bailian_test.gointernal/providers/minimax/minimax.gointernal/providers/openai/chat_compatible.gointernal/providers/responses_adapter_test.gointernal/providers/responses_content.gointernal/providers/xiaomi/audio.gointernal/providers/xiaomi/audio_test.gointernal/providers/xiaomi/xiaomi.gointernal/providers/xiaomi/xiaomi_test.gointernal/providers/zai/zai.go
|
@SantiagoDePolonia This PR (#392) replaces #390. Both review items from your feedback on #390 have been addressed:
Thanks! |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/config.example.yaml`:
- Around line 202-210: Add an explicit commented-out `base_url` configuration
key in the bailian block in config/config.example.yaml that shows the default
Beijing URL value. This will make the override path copy-pasteable and clearer
for operators, complementing the region guidance comments that are already
present. Place this `base_url` key after the `api_key` line to provide a
concrete example of how to override the base URL for different regions.
In `@internal/providers/bailian/bailian_test.go`:
- Around line 59-102: Add a negative test case to verify that the max_tokens to
max_completion_tokens mapping only applies to Qwen models and not to other
models. Create a new test function similar to
TestChatCompletion_MaxTokensMapping that uses a non-Qwen model (e.g., a generic
model name) in the ChatRequest and verify that the sent request body contains
max_tokens (not max_completion_tokens), ensuring the mapping logic is properly
scoped to Qwen providers only. This test should follow the same pattern as the
existing Qwen test but assert the opposite behavior to prevent accidental
mapping drift to unintended models.
In `@internal/providers/bailian/bailian.go`:
- Around line 189-207: The adaptBailianRequest function is ignoring the error
returned by core.MergeUnknownJSONFields on line 199-200. Instead of discarding
the error with blank identifier assignment, capture the error returned by
MergeUnknownJSONFields and add proper error handling logic, such as returning an
error or logging and returning the original request unchanged. Refer to how
other callers in responses_adapter.go handle this function's error return value
for consistency with the codebase pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d59faccf-c175-43d0-a878-c90200e82a84
📒 Files selected for processing (11)
.env.template.gitignoreCLAUDE.mdREADME.mdcmd/gomodel/main.goconfig/config.example.yamldocs/docs.jsondocs/providers/bailian.mdxdocs/providers/overview.mdxinternal/providers/bailian/bailian.gointernal/providers/bailian/bailian_test.go
|
@coderabbitai Thanks for the review. Re: the 5 actionable comments:
|
|
Tip For best results, initiate chat on the files or code changes.
The Bailian-specific changes look good overall. The 🐇 ✨ ✏️ Learnings added
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
config/config.example.yaml (1)
16-16:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnable Bailian in the shipped passthrough defaults.
bailianis now registered and documented as supporting/p/bailian/..., but the sample passthrough allowlists still omit it. That leaves the new route disabled for operators who copy the provided defaults.
config/config.example.yaml#L16-L16: addbailiantoenabled_passthrough_providers..env.template#L28-L29: addbailiantoENABLED_PASSTHROUGH_PROVIDERS.As per coding guidelines, choose safe defaults and document when users should override them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@config/config.example.yaml` at line 16, The bailian provider is registered and documented but not included in the default passthrough provider allowlists, leaving it disabled for operators using the shipped defaults. Add bailian to the enabled_passthrough_providers list in config/config.example.yaml at line 16-16 by including it in the array alongside the existing providers (openai, anthropic, openrouter, zai, vllm, deepseek). Similarly, add bailian to the ENABLED_PASSTHROUGH_PROVIDERS value in .env.template at line 28-29 in the same provider list format used there. This ensures bailian is enabled by default per safe defaults principles while remaining documented and overridable by operators.Source: Coding guidelines
♻️ Duplicate comments (1)
internal/providers/bailian/bailian.go (1)
199-200:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle merge/serialization errors instead of discarding them.
Line 199 and Line 200 drop errors, but
core.MergeUnknownJSONFieldscan fail. In that case, the code still clearsMaxTokens(Line 205), which can silently lose the user’s token limit in the forwarded request. Please propagate or explicitly handle the error path before mutating the cloned request.Suggested minimal fix pattern
-func adaptBailianRequest(req *core.ChatRequest) *core.ChatRequest { +func adaptBailianRequest(req *core.ChatRequest) (*core.ChatRequest, error) { if req == nil || req.MaxTokens == nil { - return req + return req, nil } // If the caller already set max_completion_tokens explicitly, respect it. if existing := req.ExtraFields.Lookup("max_completion_tokens"); existing != nil { cloned := *req cloned.MaxTokens = nil - return &cloned + return &cloned, nil } - maxTokensJSON, _ := json.Marshal(*req.MaxTokens) - extra, _ := core.MergeUnknownJSONFields(req.ExtraFields, map[string]json.RawMessage{ + maxTokensJSON, err := json.Marshal(*req.MaxTokens) + if err != nil { + return nil, err + } + extra, err := core.MergeUnknownJSONFields(req.ExtraFields, map[string]json.RawMessage{ "max_completion_tokens": maxTokensJSON, }) + if err != nil { + return nil, err + } cloned := *req cloned.ExtraFields = extra cloned.MaxTokens = nil - return &cloned + return &cloned, nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/providers/bailian/bailian.go` around lines 199 - 200, The code discards errors from both json.Marshal on line 199 and core.MergeUnknownJSONFields on line 200 by using blank identifiers, but if either of these operations fail, the code continues to execute line 205 which clears the MaxTokens field, silently losing the user's token limit in the forwarded request. Capture the errors returned by both json.Marshal and core.MergeUnknownJSONFields, check if either error is non-nil, and either propagate the error or explicitly handle the error case before proceeding to mutate any fields of the cloned request (such as clearing MaxTokens).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.template:
- Around line 355-357: The Bailian configuration section in the environment
template is missing the BAILIAN_MODELS variable that the provider page uses as
the embedding allowlist knob. Add a new commented-out line with
BAILIAN_MODELS=... to the Bailian section after BAILIAN_BASE_URL to complete the
template and ensure users have all necessary configuration options available for
copy/paste setup.
In `@internal/providers/bailian/bailian_test.go`:
- Line 201: The stream read operations in
internal/providers/bailian/bailian_test.go are discarding errors from
io.ReadAll, which can hide broken stream behavior. At line 201 and line 395,
replace the blank identifier in the io.ReadAll calls with proper error variable
capture and validation. After reading the stream, check if the error is non-nil
and fail the test appropriately (using t.Fatalf or t.Errorf) to ensure stream
errors are detected during testing rather than producing false positives.
- Around line 479-483: The TestChatCompletion_TransportFailure test relies on
the assumption that port 127.0.0.1:1 is always closed, which can be flaky in
different environments. Replace this approach by creating a stub RoundTripper
implementation that always returns an error, and then pass it to the http.Client
(via the Transport field) that is provided to NewWithHTTPClient. This makes the
transport failure deterministic and independent of local port availability.
---
Outside diff comments:
In `@config/config.example.yaml`:
- Line 16: The bailian provider is registered and documented but not included in
the default passthrough provider allowlists, leaving it disabled for operators
using the shipped defaults. Add bailian to the enabled_passthrough_providers
list in config/config.example.yaml at line 16-16 by including it in the array
alongside the existing providers (openai, anthropic, openrouter, zai, vllm,
deepseek). Similarly, add bailian to the ENABLED_PASSTHROUGH_PROVIDERS value in
.env.template at line 28-29 in the same provider list format used there. This
ensures bailian is enabled by default per safe defaults principles while
remaining documented and overridable by operators.
---
Duplicate comments:
In `@internal/providers/bailian/bailian.go`:
- Around line 199-200: The code discards errors from both json.Marshal on line
199 and core.MergeUnknownJSONFields on line 200 by using blank identifiers, but
if either of these operations fail, the code continues to execute line 205 which
clears the MaxTokens field, silently losing the user's token limit in the
forwarded request. Capture the errors returned by both json.Marshal and
core.MergeUnknownJSONFields, check if either error is non-nil, and either
propagate the error or explicitly handle the error case before proceeding to
mutate any fields of the cloned request (such as clearing MaxTokens).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5dd2dabc-8ba3-4d67-b500-213d4f7b9b25
📒 Files selected for processing (11)
.env.template.gitignoreCLAUDE.mdREADME.mdcmd/gomodel/main.goconfig/config.example.yamldocs/docs.jsondocs/providers/bailian.mdxdocs/providers/overview.mdxinternal/providers/bailian/bailian.gointernal/providers/bailian/bailian_test.go
|
@coderabbitai Thanks for the follow-up review. All actionable items have been addressed:
|
|
Tip For best results, initiate chat on the files or code changes.
The Bailian provider implementation looks solid. Happy to trigger a fresh review pass if you'd like a final check before merge. 🐇 ✨ |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/config.example.yaml`:
- Around line 202-210: In the bailian configuration block in
config/config.example.yaml, add a commented-out base_url configuration line
after the api_key line. The line should be formatted as "# base_url:
"${BAILIAN_BASE_URL}"" with a brief comment explaining it's optional for
overriding the default Beijing endpoint or switching regions. This commented-out
key makes the configuration override path clear and copy-pasteable for
operators, appearing before the existing region guidance comments.
In `@internal/providers/bailian/bailian_test.go`:
- Line 63: Errors from `io.ReadAll()` calls in test assertions are being
discarded using blank identifiers, which causes test failures to be silent and
hard to diagnose. For each occurrence of `io.ReadAll()` where the error is
ignored, capture and handle the error properly by checking if it is not nil and
failing the test (using t.Fatalf or similar) with the actual error details
before proceeding with assertions. This applies to the ReadAll call at the
initial marked location as well as the similar patterns at the other affected
lines in the test file, ensuring all error returns from read operations are
properly checked rather than discarded.
In `@internal/providers/bailian/bailian.go`:
- Around line 188-212: The adaptBailianRequest function silently returns the
original request when json.Marshal or core.MergeUnknownJSONFields fails, masking
critical adaptation failures. Refactor the function signature to return an error
as the second return value (making it func adaptBailianRequest(req
*core.ChatRequest) (*core.ChatRequest, error)), then replace the silent return
statements with proper error returns when either json.Marshal or
core.MergeUnknownJSONFields fails. Update all callers of adaptBailianRequest (in
ChatCompletion and StreamChatCompletion methods) to check and handle the
returned error, propagating it upward so adaptation failures are visible and
actionable rather than hidden from users.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 70685d0d-b7e6-4f29-b365-b677dcbbab49
📒 Files selected for processing (11)
.env.template.gitignoreCLAUDE.mdREADME.mdcmd/gomodel/main.goconfig/config.example.yamldocs/docs.jsondocs/providers/bailian.mdxdocs/providers/overview.mdxinternal/providers/bailian/bailian.gointernal/providers/bailian/bailian_test.go
Add Bailian provider with: - Chat completions translation (max_tokens → max_completion_tokens) - Full Passthrough router support - Streaming and non-streaming responses - Configurable base URL for region switching Merge upstream/main (Xiaomi MiMo provider, cache logging fixes). Add local pre-commit lint hook (git config core.hooksPath .githooks). Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 131-132: The provider reference in CLAUDE.md is missing the
BAILIAN_MODELS environment variable configuration, which should be documented
alongside the existing BAILIAN_API_KEY and BAILIAN_BASE_URL entries. Add
BAILIAN_MODELS to the provider environment variables list to complete the
Bailian provider documentation, following the same pattern used for other
provider model lists like OPENAI_MODELS, ANTHROPIC_MODELS, etc.
In `@config/config_test.go`:
- Around line 1127-1130: The clearProviderEnvVars function is not clearing the
Bailian-related environment variables (BAILIAN_API_KEY, BAILIAN_BASE_URL, and
BAILIAN_MODELS), which can cause these env vars to leak into the zero-config and
provider-discovery tests. Update the clearProviderEnvVars function to also clear
these three Bailian environment variables along with the other provider-specific
env vars it currently clears, ensuring test isolation is maintained.
In `@docs/providers/bailian.mdx`:
- Around line 93-98: The current note in the Bailian provider documentation
presents `BAILIAN_MODELS` and `CONFIGURED_PROVIDER_MODELS_MODE=allowlist` as
alternative solutions, which is misleading. Rephrase the note to clarify that
`BAILIAN_MODELS` must be set to make embedding models available, and that
`CONFIGURED_PROVIDER_MODELS_MODE=allowlist` is a separate configuration that
only controls which pre-configured models are exposed. Make the dependency
explicit by explaining that setting
`BAILIAN_MODELS=text-embedding-v3,text-embedding-v4` is the required step, and
`CONFIGURED_PROVIDER_MODELS_MODE=allowlist` can be additionally used if you want
to restrict model visibility to only configured models.
In `@internal/providers/bailian/bailian_test.go`:
- Around line 644-826: Add HTTP method and endpoint path assertions to each
batch and file delegation test handler to verify correct request routing. In
each test function (TestCreateBatch_Delegates, TestGetBatch_Delegates,
TestListBatches_Delegates, TestCancelBatch_Delegates, TestCreateFile_Delegates,
TestDeleteFile_Delegates, TestListFiles_Delegates, TestGetFile_Delegates,
TestGetFileContent_Delegates, TestGetBatchResults_Delegates), add assertions
inside the http.HandlerFunc to check that r.Method matches the expected HTTP
verb (POST for create operations, GET for retrieve operations, DELETE for delete
operations, etc.) and that r.URL.Path matches the expected endpoint path for
that operation. This ensures the provider is correctly routing requests to the
right methods and endpoints, not just that the response decoding works.
In `@internal/providers/bailian/bailian.go`:
- Around line 110-126: The Passthrough method calls adaptPassthroughBody which
consumes the request body. When adaptPassthroughBody returns nil (indicating no
adaptation was needed), the body has already been read/closed but is not
restored before being forwarded at line 124. This results in an empty body being
sent for passthrough requests. Fix this by rewinding the body using the
rewindBody function when adapted is nil, similar to how the error case is
handled at lines 115-116, ensuring the original body is available when
forwarding the request to p.compatible.Passthrough.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 21c25b63-bdf0-45d5-8a12-0d8f69016d1c
📒 Files selected for processing (14)
.env.template.gitignoreCLAUDE.mdREADME.mdcmd/gomodel/docs/docs.gocmd/gomodel/main.goconfig/config.example.yamlconfig/config_test.godocs/docs.jsondocs/openapi.jsondocs/providers/bailian.mdxdocs/providers/overview.mdxinternal/providers/bailian/bailian.gointernal/providers/bailian/bailian_test.go
| - **Providers:** `OPENAI_API_KEY`, `ANTHROPIC_API_KEY`, `GEMINI_API_KEY`, `USE_GOOGLE_GEMINI_NATIVE_API` (true by default; false uses Gemini's OpenAI-compatible chat API), `XAI_API_KEY`, `GROQ_API_KEY`, `OPENROUTER_API_KEY`, `ZAI_API_KEY`, `ZAI_BASE_URL` (optional Z.ai endpoint override), `MINIMAX_API_KEY`, `MINIMAX_BASE_URL` (optional MiniMax endpoint override), `XIAOMI_API_KEY`, `XIAOMI_BASE_URL` (optional Xiaomi MiMo endpoint override), `BAILIAN_API_KEY`, `BAILIAN_BASE_URL` (optional Bailian base URL for region switching; default `https://dashscope.aliyuncs.com/compatible-mode/v1`), `AZURE_API_KEY`, `AZURE_BASE_URL` (Azure OpenAI deployment base URL), `AZURE_API_VERSION` (optional Azure API version), `ORACLE_API_KEY` (Oracle API key), `ORACLE_BASE_URL` (Oracle OpenAI-compatible base URL), `<PROVIDER>[_SUFFIX]_MODELS` (comma-separated configured model list for any provider type), `OLLAMA_BASE_URL`, `VLLM_BASE_URL`, `VLLM_API_KEY` (optional upstream vLLM bearer token) | ||
| - **Provider model metadata:** `providers.<name>.models` accepts either model IDs (strings) or `{id, metadata}` objects. When `metadata` is supplied (`display_name`, `context_window`, `max_output_tokens`, `modes`, `capabilities`, `pricing`, …) it is merged onto the remote ai-model-list entry during enrichment, with operator values winning per-field. Primary use case: advertising context windows, capabilities, and pricing for local models (Ollama) and other custom endpoints whose IDs are not in the upstream registry. |
There was a problem hiding this comment.
Add BAILIAN_MODELS to the provider reference.
This list is still incomplete without the embedding allowlist variable that the new Bailian docs and env template already advertise.
✍️ Suggested update
- - `BAILIAN_BASE_URL` (optional Bailian base URL for region switching; default `https://dashscope.aliyuncs.com/compatible-mode/v1`), `AZURE_API_KEY`, ...
+ - `BAILIAN_BASE_URL` (optional Bailian base URL for region switching; default `https://dashscope.aliyuncs.com/compatible-mode/v1`),
+ `BAILIAN_MODELS` (optional model allowlist for embeddings), `AZURE_API_KEY`, ...📝 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.
| - **Providers:** `OPENAI_API_KEY`, `ANTHROPIC_API_KEY`, `GEMINI_API_KEY`, `USE_GOOGLE_GEMINI_NATIVE_API` (true by default; false uses Gemini's OpenAI-compatible chat API), `XAI_API_KEY`, `GROQ_API_KEY`, `OPENROUTER_API_KEY`, `ZAI_API_KEY`, `ZAI_BASE_URL` (optional Z.ai endpoint override), `MINIMAX_API_KEY`, `MINIMAX_BASE_URL` (optional MiniMax endpoint override), `XIAOMI_API_KEY`, `XIAOMI_BASE_URL` (optional Xiaomi MiMo endpoint override), `BAILIAN_API_KEY`, `BAILIAN_BASE_URL` (optional Bailian base URL for region switching; default `https://dashscope.aliyuncs.com/compatible-mode/v1`), `AZURE_API_KEY`, `AZURE_BASE_URL` (Azure OpenAI deployment base URL), `AZURE_API_VERSION` (optional Azure API version), `ORACLE_API_KEY` (Oracle API key), `ORACLE_BASE_URL` (Oracle OpenAI-compatible base URL), `<PROVIDER>[_SUFFIX]_MODELS` (comma-separated configured model list for any provider type), `OLLAMA_BASE_URL`, `VLLM_BASE_URL`, `VLLM_API_KEY` (optional upstream vLLM bearer token) | |
| - **Provider model metadata:** `providers.<name>.models` accepts either model IDs (strings) or `{id, metadata}` objects. When `metadata` is supplied (`display_name`, `context_window`, `max_output_tokens`, `modes`, `capabilities`, `pricing`, …) it is merged onto the remote ai-model-list entry during enrichment, with operator values winning per-field. Primary use case: advertising context windows, capabilities, and pricing for local models (Ollama) and other custom endpoints whose IDs are not in the upstream registry. | |
| - **Providers:** `OPENAI_API_KEY`, `ANTHROPIC_API_KEY`, `GEMINI_API_KEY`, `USE_GOOGLE_GEMINI_NATIVE_API` (true by default; false uses Gemini's OpenAI-compatible chat API), `XAI_API_KEY`, `GROQ_API_KEY`, `OPENROUTER_API_KEY`, `ZAI_API_KEY`, `ZAI_BASE_URL` (optional Z.ai endpoint override), `MINIMAX_API_KEY`, `MINIMAX_BASE_URL` (optional MiniMax endpoint override), `XIAOMI_API_KEY`, `XIAOMI_BASE_URL` (optional Xiaomi MiMo endpoint override), `BAILIAN_API_KEY`, `BAILIAN_BASE_URL` (optional Bailian base URL for region switching; default `https://dashscope.aliyuncs.com/compatible-mode/v1`), `BAILIAN_MODELS` (optional model allowlist for embeddings), `AZURE_API_KEY`, `AZURE_BASE_URL` (Azure OpenAI deployment base URL), `AZURE_API_VERSION` (optional Azure API version), `ORACLE_API_KEY` (Oracle API key), `ORACLE_BASE_URL` (Oracle OpenAI-compatible base URL), `<PROVIDER>[_SUFFIX]_MODELS` (comma-separated configured model list for any provider type), `OLLAMA_BASE_URL`, `VLLM_BASE_URL`, `VLLM_API_KEY` (optional upstream vLLM bearer token) | |
| - **Provider model metadata:** `providers.<name>.models` accepts either model IDs (strings) or `{id, metadata}` objects. When `metadata` is supplied (`display_name`, `context_window`, `max_output_tokens`, `modes`, `capabilities`, `pricing`, …) it is merged onto the remote ai-model-list entry during enrichment, with operator values winning per-field. Primary use case: advertising context windows, capabilities, and pricing for local models (Ollama) and other custom endpoints whose IDs are not in the upstream registry. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CLAUDE.md` around lines 131 - 132, The provider reference in CLAUDE.md is
missing the BAILIAN_MODELS environment variable configuration, which should be
documented alongside the existing BAILIAN_API_KEY and BAILIAN_BASE_URL entries.
Add BAILIAN_MODELS to the provider environment variables list to complete the
Bailian provider documentation, following the same pattern used for other
provider model lists like OPENAI_MODELS, ANTHROPIC_MODELS, etc.
| gotProviders := result.Config.Server.EnabledPassthroughProviders | ||
| wantProviders := []string{"openai", "anthropic", "openrouter", "zai", "vllm", "deepseek"} | ||
| wantProviders := []string{"openai", "anthropic", "openrouter", "zai", "vllm", "deepseek", "bailian"} | ||
| if !reflect.DeepEqual(gotProviders, wantProviders) { | ||
| t.Fatalf("Server.EnabledPassthroughProviders = %v, want %v", gotProviders, wantProviders) |
There was a problem hiding this comment.
Keep the config tests isolated from Bailian env leakage.
clearProviderEnvVars still does not clear BAILIAN_API_KEY, BAILIAN_BASE_URL, or BAILIAN_MODELS. Once Bailian is part of the expected provider set, any ambient Bailian env vars can leak into the zero-config and provider-discovery tests.
🔧 Suggested fix
"OLLAMA_API_KEY", "OLLAMA_BASE_URL", "OLLAMA_MODELS",
+ "BAILIAN_API_KEY", "BAILIAN_BASE_URL", "BAILIAN_MODELS",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@config/config_test.go` around lines 1127 - 1130, The clearProviderEnvVars
function is not clearing the Bailian-related environment variables
(BAILIAN_API_KEY, BAILIAN_BASE_URL, and BAILIAN_MODELS), which can cause these
env vars to leak into the zero-config and provider-discovery tests. Update the
clearProviderEnvVars function to also clear these three Bailian environment
variables along with the other provider-specific env vars it currently clears,
ensuring test isolation is maintained.
| <Note> | ||
| Embedding models (`text-embedding-v3`, `text-embedding-v4`) are served by | ||
| the compatible-mode API but are **not** auto-discovered from the upstream | ||
| `/v1/models` endpoint. Set `BAILIAN_MODELS=text-embedding-v3,text-embedding-v4` | ||
| or use `CONFIGURED_PROVIDER_MODELS_MODE=allowlist` to make them available. | ||
| </Note> |
There was a problem hiding this comment.
Don't make allowlist sound like a substitute for BAILIAN_MODELS.
CONFIGURED_PROVIDER_MODELS_MODE=allowlist only changes which configured models are exposed; it doesn't surface embeddings unless BAILIAN_MODELS is set. Please rephrase this note so the dependency stays explicit.
✍️ Suggested wording
- Set `BAILIAN_MODELS=text-embedding-v3,text-embedding-v4`
- or use `CONFIGURED_PROVIDER_MODELS_MODE=allowlist` to make them available.
+ Set `BAILIAN_MODELS=text-embedding-v3,text-embedding-v4`.
+ Optionally use `CONFIGURED_PROVIDER_MODELS_MODE=allowlist` if you want
+ only the configured models exposed.📝 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.
| <Note> | |
| Embedding models (`text-embedding-v3`, `text-embedding-v4`) are served by | |
| the compatible-mode API but are **not** auto-discovered from the upstream | |
| `/v1/models` endpoint. Set `BAILIAN_MODELS=text-embedding-v3,text-embedding-v4` | |
| or use `CONFIGURED_PROVIDER_MODELS_MODE=allowlist` to make them available. | |
| </Note> | |
| <Note> | |
| Embedding models (`text-embedding-v3`, `text-embedding-v4`) are served by | |
| the compatible-mode API but are **not** auto-discovered from the upstream | |
| `/v1/models` endpoint. Set `BAILIAN_MODELS=text-embedding-v3,text-embedding-v4`. | |
| Optionally use `CONFIGURED_PROVIDER_MODELS_MODE=allowlist` if you want | |
| only the configured models exposed. | |
| </Note> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/providers/bailian.mdx` around lines 93 - 98, The current note in the
Bailian provider documentation presents `BAILIAN_MODELS` and
`CONFIGURED_PROVIDER_MODELS_MODE=allowlist` as alternative solutions, which is
misleading. Rephrase the note to clarify that `BAILIAN_MODELS` must be set to
make embedding models available, and that
`CONFIGURED_PROVIDER_MODELS_MODE=allowlist` is a separate configuration that
only controls which pre-configured models are exposed. Make the dependency
explicit by explaining that setting
`BAILIAN_MODELS=text-embedding-v3,text-embedding-v4` is the required step, and
`CONFIGURED_PROVIDER_MODELS_MODE=allowlist` can be additionally used if you want
to restrict model visibility to only configured models.
| func TestCreateBatch_Delegates(t *testing.T) { | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = w.Write([]byte(`{"id":"batch-bailian-1","object":"batch","status":"validating"}`)) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| provider := NewWithHTTPClient("key", server.Client(), llmclient.Hooks{}) | ||
| provider.SetBaseURL(server.URL) | ||
|
|
||
| resp, err := provider.CreateBatch(context.Background(), &core.BatchRequest{ | ||
| InputFileID: "file-1", | ||
| Endpoint: "/v1/chat/completions", | ||
| }) | ||
| if err != nil { | ||
| t.Fatalf("CreateBatch() error = %v", err) | ||
| } | ||
| if resp.ID != "batch-bailian-1" { | ||
| t.Fatalf("batch id = %q", resp.ID) | ||
| } | ||
| } | ||
|
|
||
| func TestGetBatch_Delegates(t *testing.T) { | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = w.Write([]byte(`{"id":"batch-bailian-1","object":"batch","status":"completed"}`)) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| provider := NewWithHTTPClient("key", server.Client(), llmclient.Hooks{}) | ||
| provider.SetBaseURL(server.URL) | ||
|
|
||
| resp, err := provider.GetBatch(context.Background(), "batch-bailian-1") | ||
| if err != nil { | ||
| t.Fatalf("GetBatch() error = %v", err) | ||
| } | ||
| if resp.ID != "batch-bailian-1" { | ||
| t.Fatalf("batch id = %q", resp.ID) | ||
| } | ||
| } | ||
|
|
||
| func TestListBatches_Delegates(t *testing.T) { | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = w.Write([]byte(`{"object":"list","data":[]}`)) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| provider := NewWithHTTPClient("key", server.Client(), llmclient.Hooks{}) | ||
| provider.SetBaseURL(server.URL) | ||
|
|
||
| _, err := provider.ListBatches(context.Background(), 10, "") | ||
| if err != nil { | ||
| t.Fatalf("ListBatches() error = %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestCancelBatch_Delegates(t *testing.T) { | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = w.Write([]byte(`{"id":"batch-bailian-1","object":"batch","status":"cancelling"}`)) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| provider := NewWithHTTPClient("key", server.Client(), llmclient.Hooks{}) | ||
| provider.SetBaseURL(server.URL) | ||
|
|
||
| resp, err := provider.CancelBatch(context.Background(), "batch-bailian-1") | ||
| if err != nil { | ||
| t.Fatalf("CancelBatch() error = %v", err) | ||
| } | ||
| if resp.ID != "batch-bailian-1" { | ||
| t.Fatalf("batch id = %q", resp.ID) | ||
| } | ||
| } | ||
|
|
||
| func TestCreateFile_Delegates(t *testing.T) { | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = w.Write([]byte(`{"id":"file-1","object":"file","purpose":"batch","bytes":100}`)) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| provider := NewWithHTTPClient("key", server.Client(), llmclient.Hooks{}) | ||
| provider.SetBaseURL(server.URL) | ||
|
|
||
| resp, err := provider.CreateFile(context.Background(), &core.FileCreateRequest{ | ||
| Content: []byte("data"), | ||
| Purpose: "batch", | ||
| }) | ||
| if err != nil { | ||
| t.Fatalf("CreateFile() error = %v", err) | ||
| } | ||
| if resp.Provider != "bailian" { | ||
| t.Fatalf("provider = %q, want bailian", resp.Provider) | ||
| } | ||
| } | ||
|
|
||
| func TestDeleteFile_Delegates(t *testing.T) { | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = w.Write([]byte(`{"id":"file-1","object":"file","deleted":true}`)) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| provider := NewWithHTTPClient("key", server.Client(), llmclient.Hooks{}) | ||
| provider.SetBaseURL(server.URL) | ||
|
|
||
| resp, err := provider.DeleteFile(context.Background(), "file-1") | ||
| if err != nil { | ||
| t.Fatalf("DeleteFile() error = %v", err) | ||
| } | ||
| if !resp.Deleted { | ||
| t.Fatal("expected deleted=true") | ||
| } | ||
| } | ||
|
|
||
| func TestListFiles_Delegates(t *testing.T) { | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = w.Write([]byte(`{"object":"list","data":[]}`)) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| provider := NewWithHTTPClient("key", server.Client(), llmclient.Hooks{}) | ||
| provider.SetBaseURL(server.URL) | ||
|
|
||
| _, err := provider.ListFiles(context.Background(), "batch", 10, "") | ||
| if err != nil { | ||
| t.Fatalf("ListFiles() error = %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestGetFile_Delegates(t *testing.T) { | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = w.Write([]byte(`{"id":"file-1","object":"file","purpose":"batch"}`)) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| provider := NewWithHTTPClient("key", server.Client(), llmclient.Hooks{}) | ||
| provider.SetBaseURL(server.URL) | ||
|
|
||
| resp, err := provider.GetFile(context.Background(), "file-1") | ||
| if err != nil { | ||
| t.Fatalf("GetFile() error = %v", err) | ||
| } | ||
| if resp.Provider != "bailian" { | ||
| t.Fatalf("provider = %q, want bailian", resp.Provider) | ||
| } | ||
| } | ||
|
|
||
| func TestGetFileContent_Delegates(t *testing.T) { | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = w.Write([]byte(`{"text":"content"}`)) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| provider := NewWithHTTPClient("key", server.Client(), llmclient.Hooks{}) | ||
| provider.SetBaseURL(server.URL) | ||
|
|
||
| _, err := provider.GetFileContent(context.Background(), "file-1") | ||
| if err != nil { | ||
| t.Fatalf("GetFileContent() error = %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestGetBatchResults_Delegates(t *testing.T) { | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = w.Write([]byte(`{"id":"batch-1","output_file_id":"file-out-1"}`)) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| provider := NewWithHTTPClient("key", server.Client(), llmclient.Hooks{}) | ||
| provider.SetBaseURL(server.URL) | ||
|
|
||
| _, err := provider.GetBatchResults(context.Background(), "batch-1") | ||
| if err != nil { | ||
| t.Fatalf("GetBatchResults() error = %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Assert method and endpoint in batch/file delegation tests.
These tests currently validate decoded responses but not request routing. They can still pass if delegation hits the wrong path or verb. Add r.Method/r.URL.Path assertions in each handler (or capture and assert after call) to lock the provider contract.
Minimal example pattern
func TestCreateBatch_Delegates(t *testing.T) {
+ var gotMethod, gotPath string
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ gotMethod = r.Method
+ gotPath = r.URL.Path
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{"id":"batch-bailian-1","object":"batch","status":"validating"}`))
}))
@@
if resp.ID != "batch-bailian-1" {
t.Fatalf("batch id = %q", resp.ID)
}
+ if gotMethod != http.MethodPost {
+ t.Fatalf("method = %q, want %q", gotMethod, http.MethodPost)
+ }
+ if gotPath != "/batches" {
+ t.Fatalf("path = %q, want /batches", gotPath)
+ }
}As per coding guidelines, **/*_test.go should cover behavior changes including request translation/provider-specific mapping, and these delegation tests are the contract lock for that behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/providers/bailian/bailian_test.go` around lines 644 - 826, Add HTTP
method and endpoint path assertions to each batch and file delegation test
handler to verify correct request routing. In each test function
(TestCreateBatch_Delegates, TestGetBatch_Delegates, TestListBatches_Delegates,
TestCancelBatch_Delegates, TestCreateFile_Delegates, TestDeleteFile_Delegates,
TestListFiles_Delegates, TestGetFile_Delegates, TestGetFileContent_Delegates,
TestGetBatchResults_Delegates), add assertions inside the http.HandlerFunc to
check that r.Method matches the expected HTTP verb (POST for create operations,
GET for retrieve operations, DELETE for delete operations, etc.) and that
r.URL.Path matches the expected endpoint path for that operation. This ensures
the provider is correctly routing requests to the right methods and endpoints,
not just that the response decoding works.
Source: Coding guidelines
| func (p *Provider) Passthrough(ctx context.Context, req *core.PassthroughRequest) (*core.PassthroughResponse, error) { | ||
| adapted, err := adaptPassthroughBody(req.Body) | ||
| if err != nil { | ||
| slog.Warn("bailian: passthrough body adaptation failed, forwarding original body", | ||
| "error", err) | ||
| // Read the original body back so we can still forward it | ||
| req.Body, err = rewindBody(req.Body, nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return p.compatible.Passthrough(ctx, req) | ||
| } | ||
| if adapted != nil { | ||
| req.Body = adapted | ||
| } | ||
| return p.compatible.Passthrough(ctx, req) | ||
| } |
There was a problem hiding this comment.
Passthrough no-op adaptation currently drops the request payload.
Line 242 consumes/closes req.Body, but when max_tokens is absent Line 260 returns nil; then Line 125 forwards req without restoring the body. This can send empty bodies for normal passthrough requests that don’t use token remapping.
Suggested minimal fix
func adaptPassthroughBody(body io.ReadCloser) (io.ReadCloser, error) {
@@
- // No max_tokens present — no adaptation needed, rewind original.
+ // No max_tokens present — no adaptation needed, but return a rewound body.
if _, hasMaxTokens := obj["max_tokens"]; !hasMaxTokens {
- return nil, nil
+ return io.NopCloser(bytes.NewReader(raw)), nil
}Also applies to: 237-261
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/providers/bailian/bailian.go` around lines 110 - 126, The
Passthrough method calls adaptPassthroughBody which consumes the request body.
When adaptPassthroughBody returns nil (indicating no adaptation was needed), the
body has already been read/closed but is not restored before being forwarded at
line 124. This results in an empty body being sent for passthrough requests. Fix
this by rewinding the body using the rewindBody function when adapted is nil,
similar to how the error case is handled at lines 115-116, ensuring the original
body is available when forwarding the request to p.compatible.Passthrough.
|
Merging! I'll adjust it a little bit in the upcoming PR. |
Summary
Adds Alibaba Cloud Bailian (百炼) (
dashscope.aliyuncs.com) as a primary provider. SettingBAILIAN_API_KEYisthe whole setup (
BAILIAN_BASE_URLoptional for region switching).Provider
internal/providers/bailianpackage, typebailian, default base URLhttps://dashscope.aliyuncs.com/compatible-mode/v1, Bearer auth.Passthrough all delegate to
openai.CompatibleProvider.text-embedding-v3,text-embedding-v4) are served by the compatible-mode API but are notauto-discovered via
GET /v1/models-- configure viaBAILIAN_MODELS.max_tokenscompatibility shimBailian deprecated
max_tokenson 2026-04-20 in favor ofmax_completion_tokens. The provider transparently mapsmax_tokens->max_completion_tokensfor every request. If the caller explicitly setsmax_completion_tokens, thatvalue is preserved and
max_tokensis only stripped (not overridden).Multi-region
Bailian is available in Beijing (default), Singapore, Frankfurt, and Hong Kong.
BAILIAN_BASE_URLselects the region.Docs
New
docs/providers/bailian.mdx(max_tokenscompatibility, multi-region base URLs, model IDs), wired intonav/overview; README table,
.env.template,config.example.yaml, CLAUDE.md, Swagger@descriptionupdated.Testing
ListBatches, CancelBatch, CreateFile, ListFiles, GetFile, DeleteFile, GetFileContent, GetBatchResults
Passthrough; transport failure
max_tokens->max_completion_tokensmapping, explicitmax_completion_tokenspreservation,nil/empty edge cases, request cloning, field preservation, Registration type and default URL
dashscope.aliyuncs.com): chat completions (non-streaming +SSE),
/v1/responses, and/v1/embeddingsall confirmed workingThis PR replaces #390 and addresses the feedback from @SantiagoDePolonia (lint fix + merge main).
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
max_tokenstomax_completion_tokensfor compatibility.Documentation
Chores