feat: glm coding plan && kimi coding plan - #2334
Conversation
WalkthroughUpstream URL construction and request routing were changed to consult a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
controller/channel.go(2 hunks)relay/channel/volcengine/adaptor.go(1 hunks)relay/channel/volcengine/constants.go(1 hunks)relay/channel/zhipu_4v/adaptor.go(2 hunks)relay/channel/zhipu_4v/constants.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
controller/channel.go (1)
relay/channel/zhipu_4v/constants.go (2)
GlmCodingPlan(4-4)GlmCodingPlanOpenAIBaseURL(6-6)
relay/channel/zhipu_4v/adaptor.go (4)
constant/channel.go (2)
ChannelBaseURLs(61-119)ChannelTypeZhipu_v4(30-30)types/relay_format.go (2)
RelayFormat(3-3)RelayFormatClaude(7-7)relay/channel/zhipu_4v/constants.go (3)
GlmCodingPlan(4-4)GlmCodingPlanClaudeBaseURL(5-5)GlmCodingPlanOpenAIBaseURL(6-6)relay/constant/relay_mode.go (1)
RelayModeEmbeddings(12-12)
🔇 Additional comments (6)
relay/channel/volcengine/adaptor.go (1)
26-29: LGTM! Constants properly refactored.The Doubao coding plan constants have been correctly moved to
constants.goin the same package. This improves code organization by centralizing constant definitions. The constants remain accessible throughout the package, and their usage in this file (lines 40, 246, 256, 345) continues to work correctly.relay/channel/zhipu_4v/adaptor.go (1)
47-50: Good practice: Default baseURL resolution.Adding default baseURL resolution when
info.ChannelBaseUrlis empty is a good defensive coding practice. This prevents potential nil pointer or empty string issues downstream.relay/channel/zhipu_4v/constants.go (2)
10-10: I'll help you verify the model identifier for "glm-4.6" in the Zhipu API. Let me start by examining the file and then searching for official documentation.
<function_calls>
cat relay/channel/zhipu_4v/constants.go
</function_calls>
<function_calls>
Zhipu GLM 4.6 model identifier API documentation
</function_calls>
3-7: Due to repository access issues, I cannot fully verify all concerns in the original review. However, the web search provided helpful information about the GLM Coding Plan API.From official Zhipu documentation:
- The OpenAI-compatible endpoint
https://open.bigmodel.cn/api/coding/paas/v4is confirmed correct- The recommended model for Coding Plan is GLM-4.6 (note: may have capitalization differences from code)
- The Anthropic-compatible endpoint (
https://open.bigmodel.cn/api/anthropic) is not mentioned in official documentation—this requires verificationUnable to verify without codebase access:
- How the sentinel value
GlmCodingPlan = "glm-coding-plan"is actually used in routing logic- Whether the design pattern creates the security/UX concerns mentioned
- Whether model name capitalization matches (glm-4.6 vs GLM-4.6)
Verify the Anthropic-compatible endpoint and sentinel value usage pattern.
One of the two endpoints is confirmed (OpenAI v4), but
https://open.bigmodel.cn/api/anthropicis not mentioned in official GLM documentation and needs verification. Additionally, the sentinel value pattern design concern cannot be assessed without reviewing the routing implementation.controller/channel.go (1)
196-200: Sentinel pattern implementation in URL construction.This conditional logic relies on the sentinel value pattern where
baseURL == zhipu_4v.GlmCodingPlantriggers different URL construction. While functional, this approach has several concerns:
- Fragile design: If a user configures their channel with baseURL = "glm-coding-plan" (perhaps misunderstanding the configuration), they'll get unexpected routing.
- No validation: There's no check to prevent users from accidentally setting this sentinel value.
- Code duplication: Similar pattern repeated for VolcEngine (lines 202-206).
Consider refactoring to use a helper function:
// Helper function to get models URL for a channel func getModelsURL(channel *model.Channel, baseURL string) string { switch channel.Type { case constant.ChannelTypeZhipu_v4: if baseURL == zhipu_4v.GlmCodingPlan { return fmt.Sprintf("%s/models", zhipu_4v.GlmCodingPlanOpenAIBaseURL) } return fmt.Sprintf("%s/api/paas/v4/models", baseURL) case constant.ChannelTypeVolcEngine: if baseURL == volcengine.DoubaoCodingPlan { return fmt.Sprintf("%s/v1/models", volcengine.DoubaoCodingPlanOpenAIBaseURL) } return fmt.Sprintf("%s/v1/models", baseURL) case constant.ChannelTypeGemini: return fmt.Sprintf("%s/v1beta/openai/models", baseURL) case constant.ChannelTypeAli: return fmt.Sprintf("%s/compatible-mode/v1/models", baseURL) default: return fmt.Sprintf("%s/v1/models", baseURL) } }Then use it in the switch statement:
- var url string - switch channel.Type { - case constant.ChannelTypeGemini: - // curl https://example.com/v1beta/models?key=$GEMINI_API_KEY - url = fmt.Sprintf("%s/v1beta/openai/models", baseURL) // Remove key in url since we need to use AuthHeader - case constant.ChannelTypeAli: - url = fmt.Sprintf("%s/compatible-mode/v1/models", baseURL) - case constant.ChannelTypeZhipu_v4: - if baseURL == zhipu_4v.GlmCodingPlan { - url = fmt.Sprintf("%s/models", zhipu_4v.GlmCodingPlanOpenAIBaseURL) - } else { - url = fmt.Sprintf("%s/api/paas/v4/models", baseURL) - } - case constant.ChannelTypeVolcEngine: - if baseURL == volcengine.DoubaoCodingPlan { - url = fmt.Sprintf("%s/v1/models", volcengine.DoubaoCodingPlanOpenAIBaseURL) - } else { - url = fmt.Sprintf("%s/v1/models", baseURL) - } - default: - url = fmt.Sprintf("%s/v1/models", baseURL) - } + url := getModelsURL(channel, baseURL)⛔ Skipped due to learnings
Learnt from: Sh1n3zZ Repo: QuantumNous/new-api PR: 1659 File: relay/relay_task.go:285-305 Timestamp: 2025-08-26T09:59:00.337Z Learning: In controller/task_video.go, data: URLs (containing base64 encoded video data) are prevented from being stored in task.FailReason by checking if the URL starts with "data:" before assignment. This same pattern should be applied consistently across the codebase.relay/channel/volcengine/constants.go (1)
3-7: URLs verified as official Volcengine endpoints; architectural concern requires code review for assessment.The endpoints referenced in the constants are confirmed in official Volcengine documentation:
https://ark.cn-beijing.volces.com/api/coding— Primary Coding Plan endpointhttps://ark.cn-beijing.volces.com/api/coding/v3— OpenAI-compatible variant (official for integration tools)- Model:
doubao-seed-code-preview-latestHowever, I cannot verify the core architectural concern (sentinel pattern confusion with baseURL assignment) or the comparison to the Zhipu implementation without access to the actual code usage patterns. This requires manual code review to assess whether:
- The sentinel pattern (
DoubaoCodingPlan = "doubao-coding-plan") is actually mixed with baseURL logic- The comparison to Zhipu's approach is accurate
- Defensive measures exist to prevent accidental baseURL assignment
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
relay/channel/volcengine/adaptor.go (1)
345-353: Minor inconsistency in special plan validation logic.In
GetRequestURL(line 247), the code checks bothhasSpecialPlan && specialPlan.ClaudeBaseURL != ""before using Claude routing. However, inDoResponse(line 347), the code only checks map existence without verifying thatClaudeBaseURLis non-empty.While all current entries in
ChannelSpecialBaseshave non-emptyClaudeBaseURLvalues, this could cause issues if a future entry has an emptyClaudeBaseURL. Consider aligning the checks:if info.RelayFormat == types.RelayFormatClaude { - if _, ok := channelconstant.ChannelSpecialBases[info.ChannelBaseUrl]; ok { + if specialPlan, ok := channelconstant.ChannelSpecialBases[info.ChannelBaseUrl]; ok && specialPlan.ClaudeBaseURL != "" { if info.IsStream { return claude.ClaudeStreamHandler(c, resp, info, claude.RequestModeMessage) } return claude.ClaudeHandler(c, resp, info, claude.RequestModeMessage) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
constant/channel.go(1 hunks)controller/channel.go(1 hunks)relay/channel/volcengine/adaptor.go(5 hunks)relay/channel/zhipu_4v/adaptor.go(2 hunks)relay/channel/zhipu_4v/constants.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- controller/channel.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-26T09:59:00.337Z
Learnt from: Sh1n3zZ
Repo: QuantumNous/new-api PR: 1659
File: relay/relay_task.go:285-305
Timestamp: 2025-08-26T09:59:00.337Z
Learning: In controller/task_video.go, data: URLs (containing base64 encoded video data) are prevented from being stored in task.FailReason by checking if the URL starts with "data:" before assignment. This same pattern should be applied consistently across the codebase.
Applied to files:
relay/channel/zhipu_4v/adaptor.go
🧬 Code graph analysis (2)
relay/channel/zhipu_4v/adaptor.go (3)
constant/channel.go (3)
ChannelBaseURLs(61-119)ChannelTypeZhipu_v4(30-30)ChannelSpecialBases(189-202)types/relay_format.go (2)
RelayFormat(3-3)RelayFormatClaude(7-7)relay/constant/relay_mode.go (1)
RelayModeEmbeddings(12-12)
relay/channel/volcengine/adaptor.go (1)
constant/channel.go (1)
ChannelSpecialBases(189-202)
🔇 Additional comments (5)
constant/channel.go (1)
183-202: Clean centralized configuration for special base URLs.The
ChannelSpecialBasestruct andChannelSpecialBasesmap provide a well-organized way to configure channel-specific base URLs. This approach is more maintainable than hardcoded constants scattered across adaptors.relay/channel/zhipu_4v/constants.go (1)
3-5: LGTM!The addition of
"glm-4.6"to the model list is straightforward and follows the existing naming convention.relay/channel/zhipu_4v/adaptor.go (1)
46-73: Good refactoring using centralized configuration.The
GetRequestURLlogic now cleanly uses theChannelSpecialBasesmap lookup instead of repeated sentinel pattern checks. This addresses the duplication concern from previous reviews and makes the routing logic more maintainable.relay/channel/volcengine/adaptor.go (2)
39-46: LGTM!The
ConvertClaudeRequestmethod cleanly delegates to the appropriate adaptor based on whether the channel has a special base configuration.
264-265: Inconsistent special plan handling for embeddings between adaptors.The
zhipu_4vadaptor (lines 62-65) checkshasSpecialPlan && specialPlan.OpenAIBaseURLfor embeddings and routes tospecialPlan.OpenAIBaseURL/embeddings. However, this adaptor does not apply the same special plan logic forRelayModeEmbeddings.If this is intentional (e.g., doubao-coding-plan doesn't support embeddings via the special URL), consider adding a comment to clarify. Otherwise, consider adding consistent handling:
case constant.RelayModeEmbeddings: + if hasSpecialPlan && specialPlan.OpenAIBaseURL != "" { + return fmt.Sprintf("%s/embeddings", specialPlan.OpenAIBaseURL), nil + } return fmt.Sprintf("%s/api/v3/embeddings", baseUrl), nil
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
constant/channel.go(1 hunks)controller/channel.go(1 hunks)relay/channel/moonshot/adaptor.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- controller/channel.go
🧰 Additional context used
🧬 Code graph analysis (1)
relay/channel/moonshot/adaptor.go (2)
constant/channel.go (1)
ChannelSpecialBases(189-206)types/relay_format.go (3)
RelayFormat(3-3)RelayFormatClaude(7-7)RelayFormatOpenAI(6-6)
🔇 Additional comments (2)
relay/channel/moonshot/adaptor.go (2)
9-9: LGTM!The import alias is appropriate to avoid conflicts with the existing
constantimport.
48-56: Verify that special base URL path construction is correct.The path construction for special bases differs from the default paths:
- Special base OpenAI:
/chat/completions(line 54)- Default OpenAI:
/v1/chat/completions(line 67)- Special base Claude:
/v1/messages(line 51)- Default Claude:
/anthropic/v1/messages(line 60)Confirm whether these differences are intentional. If the special base URLs (from
ChannelSpecialBases) already include version or service prefixes (e.g.,https://api.kimi.com/coding/v1or similar), then the shorter paths are correct. Otherwise, the paths may be missing required segments.
| type ChannelSpecialBase struct { | ||
| ClaudeBaseURL string | ||
| OpenAIBaseURL string | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add documentation for the public struct.
This public struct should have a godoc comment explaining its purpose, such as how it's used to define alternative base URLs for special channel plans.
+// ChannelSpecialBase defines alternative base URLs for special channel plans
+// that support both Claude and OpenAI formats with different endpoints.
type ChannelSpecialBase struct {
ClaudeBaseURL string
OpenAIBaseURL string
}📝 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.
| type ChannelSpecialBase struct { | |
| ClaudeBaseURL string | |
| OpenAIBaseURL string | |
| } | |
| // ChannelSpecialBase defines alternative base URLs for special channel plans | |
| // that support both Claude and OpenAI formats with different endpoints. | |
| type ChannelSpecialBase struct { | |
| ClaudeBaseURL string | |
| OpenAIBaseURL string | |
| } |
🤖 Prompt for AI Agents
In constant/channel.go around lines 184 to 187, the public struct
ChannelSpecialBase lacks a godoc comment; add a one-line godoc comment
immediately above the type declaration that explains its purpose (e.g., that it
defines alternative base URLs for special channel plans and is used to override
default Claude/OpenAI endpoints), keeping the comment concise and starting with
"ChannelSpecialBase" to satisfy Go documentation conventions.
| var ChannelSpecialBases = map[string]ChannelSpecialBase{ | ||
| "glm-coding-plan": { | ||
| ClaudeBaseURL: "https://open.bigmodel.cn/api/anthropic", | ||
| OpenAIBaseURL: "https://open.bigmodel.cn/api/coding/paas/v4", | ||
| }, | ||
| "glm-coding-plan-international": { | ||
| ClaudeBaseURL: "https://api.z.ai/api/anthropic", | ||
| OpenAIBaseURL: "https://api.z.ai/api/coding/paas/v4", | ||
| }, | ||
| "kimi-coding-plan": { | ||
| ClaudeBaseURL: "https://api.kimi.com/coding", | ||
| OpenAIBaseURL: "https://api.kimi.com/coding/v1", | ||
| }, | ||
| "doubao-coding-plan": { | ||
| ClaudeBaseURL: "https://ark.cn-beijing.volces.com/api/coding", | ||
| OpenAIBaseURL: "https://ark.cn-beijing.volces.com/api/coding/v3", | ||
| }, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add documentation for the public map.
This public map should have a godoc comment explaining that it maps special plan identifiers to their corresponding base URLs for different API formats.
+// ChannelSpecialBases maps special plan identifiers (e.g., "glm-coding-plan") to their
+// corresponding Claude and OpenAI base URLs. When a channel's base URL matches a key in
+// this map, the special URLs are used instead of the default channel base URL.
var ChannelSpecialBases = map[string]ChannelSpecialBase{
"glm-coding-plan": {
ClaudeBaseURL: "https://open.bigmodel.cn/api/anthropic",📝 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.
| var ChannelSpecialBases = map[string]ChannelSpecialBase{ | |
| "glm-coding-plan": { | |
| ClaudeBaseURL: "https://open.bigmodel.cn/api/anthropic", | |
| OpenAIBaseURL: "https://open.bigmodel.cn/api/coding/paas/v4", | |
| }, | |
| "glm-coding-plan-international": { | |
| ClaudeBaseURL: "https://api.z.ai/api/anthropic", | |
| OpenAIBaseURL: "https://api.z.ai/api/coding/paas/v4", | |
| }, | |
| "kimi-coding-plan": { | |
| ClaudeBaseURL: "https://api.kimi.com/coding", | |
| OpenAIBaseURL: "https://api.kimi.com/coding/v1", | |
| }, | |
| "doubao-coding-plan": { | |
| ClaudeBaseURL: "https://ark.cn-beijing.volces.com/api/coding", | |
| OpenAIBaseURL: "https://ark.cn-beijing.volces.com/api/coding/v3", | |
| }, | |
| } | |
| // ChannelSpecialBases maps special plan identifiers (e.g., "glm-coding-plan") to their | |
| // corresponding Claude and OpenAI base URLs. When a channel's base URL matches a key in | |
| // this map, the special URLs are used instead of the default channel base URL. | |
| var ChannelSpecialBases = map[string]ChannelSpecialBase{ | |
| "glm-coding-plan": { | |
| ClaudeBaseURL: "https://open.bigmodel.cn/api/anthropic", | |
| OpenAIBaseURL: "https://open.bigmodel.cn/api/coding/paas/v4", | |
| }, | |
| "glm-coding-plan-international": { | |
| ClaudeBaseURL: "https://api.z.ai/api/anthropic", | |
| OpenAIBaseURL: "https://api.z.ai/api/coding/paas/v4", | |
| }, | |
| "kimi-coding-plan": { | |
| ClaudeBaseURL: "https://api.kimi.com/coding", | |
| OpenAIBaseURL: "https://api.kimi.com/coding/v1", | |
| }, | |
| "doubao-coding-plan": { | |
| ClaudeBaseURL: "https://ark.cn-beijing.volces.com/api/coding", | |
| OpenAIBaseURL: "https://ark.cn-beijing.volces.com/api/coding/v3", | |
| }, | |
| } |
🤖 Prompt for AI Agents
In constant/channel.go around lines 189 to 206, add a Go doc comment for the
exported ChannelSpecialBases map that briefly explains its purpose: that it maps
special plan identifiers (string keys) to ChannelSpecialBase entries containing
the ClaudeBaseURL and OpenAIBaseURL for those plans, and that these base URLs
are used to override default API endpoints for specific plan identifiers; place
the comment immediately above the ChannelSpecialBases declaration, keep it
concise (one to two sentences), and mention the structure of values
(ChannelSpecialBase with ClaudeBaseURL and OpenAIBaseURL) so godoc consumers
understand the mapping.
| baseURL := info.ChannelBaseUrl | ||
| if specialPlan, ok := channelconstant.ChannelSpecialBases[baseURL]; ok { | ||
| if info.RelayFormat == types.RelayFormatClaude { | ||
| return fmt.Sprintf("%s/v1/messages", specialPlan.ClaudeBaseURL), nil | ||
| } | ||
| if info.RelayFormat == types.RelayFormatOpenAI { | ||
| return fmt.Sprintf("%s/chat/completions", specialPlan.OpenAIBaseURL), nil | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Handle unsupported relay formats when special base is matched.
When a special base is found but RelayFormat is neither Claude nor OpenAI, the code falls through to the switch statement at line 58. At that point, info.ChannelBaseUrl contains a plan identifier (e.g., "glm-coding-plan") rather than a valid URL, which would result in malformed URLs like "glm-coding-plan/v1/chat/completions".
Apply this diff to handle the case properly:
baseURL := info.ChannelBaseUrl
if specialPlan, ok := channelconstant.ChannelSpecialBases[baseURL]; ok {
if info.RelayFormat == types.RelayFormatClaude {
return fmt.Sprintf("%s/v1/messages", specialPlan.ClaudeBaseURL), nil
}
if info.RelayFormat == types.RelayFormatOpenAI {
return fmt.Sprintf("%s/chat/completions", specialPlan.OpenAIBaseURL), nil
}
+ // Special base found but format not supported
+ return "", fmt.Errorf("relay format %s not supported for special base %s", info.RelayFormat, baseURL)
}📝 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.
| baseURL := info.ChannelBaseUrl | |
| if specialPlan, ok := channelconstant.ChannelSpecialBases[baseURL]; ok { | |
| if info.RelayFormat == types.RelayFormatClaude { | |
| return fmt.Sprintf("%s/v1/messages", specialPlan.ClaudeBaseURL), nil | |
| } | |
| if info.RelayFormat == types.RelayFormatOpenAI { | |
| return fmt.Sprintf("%s/chat/completions", specialPlan.OpenAIBaseURL), nil | |
| } | |
| } | |
| baseURL := info.ChannelBaseUrl | |
| if specialPlan, ok := channelconstant.ChannelSpecialBases[baseURL]; ok { | |
| if info.RelayFormat == types.RelayFormatClaude { | |
| return fmt.Sprintf("%s/v1/messages", specialPlan.ClaudeBaseURL), nil | |
| } | |
| if info.RelayFormat == types.RelayFormatOpenAI { | |
| return fmt.Sprintf("%s/chat/completions", specialPlan.OpenAIBaseURL), nil | |
| } | |
| // Special base found but format not supported | |
| return "", fmt.Errorf("relay format %s not supported for special base %s", info.RelayFormat, baseURL) | |
| } |
🤖 Prompt for AI Agents
In relay/channel/moonshot/adaptor.go around lines 48 to 56, when
channelconstant.ChannelSpecialBases matches but info.RelayFormat is neither
RelayFormatClaude nor RelayFormatOpenAI the function currently falls through
leaving baseURL as a plan id and producing malformed URLs; detect this
unsupported format immediately and return a clear error instead of falling
through (e.g., return nil URL with fmt.Errorf("unsupported relay format %q for
special base %q", info.RelayFormat, baseURL)) so the caller can handle the
invalid combination.
seefs001
changed the title
feat: glm coding plan && kimi coding plan
feat: glm coding plan && kimi coding plan
2 participants
glm API地址写glm-coding-plan，支持OpenAI格式和Claude格式
国际版写 glm-coding-plan-international
kimi API地址写 kimi-coding-plan
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.