add minimax coding plan support by anthropic api like#765
add minimax coding plan support by anthropic api like#765JoeEmp wants to merge 3 commits intosipeed:mainfrom
Conversation
|
@JoeEmp please fix Linter bug |
Bug SummaryProblem: Using anthropic-compatible APIs (like minimax coding plan) fails after the first request with error: Root Cause: In This was introduced in commit Fix: Add assistantMsg.ToolCalls = append(assistantMsg.ToolCalls, providers.ToolCall{
ID: tc.ID,
Type: "function",
Name: tc.Name,
Arguments: tc.Arguments, // ADD THIS
Function: &providers.FunctionCall{
Name: tc.Name,
Arguments: string(argumentsJSON),
ThoughtSignature: thoughtSignature,
},
// ...
})Note: This bug existed before PR-765 but was only exposed by the new anthropic_compat provider which directly uses the |
i got it |
nikolasdehor
left a comment
There was a problem hiding this comment.
This is a substantial new provider implementation with good test coverage (464 lines of tests). Several observations:
Positives:
- Clean separation via
httpDelegateinterface inhttp_provider.go - Thorough test suite covering happy path, tool calls, system messages, options, error handling, and proxy config
- Proper context propagation with
http.NewRequestWithContext
Issues to address:
-
systemfield type mismatch:buildRequestBodysetsrequestBody["system"] = systemwheresystemis[]string. The Anthropic Messages API expectssystemto be either a single string or an array of content blocks ([{"type":"text","text":"..."}]), not a[]string. This will likely cause 400 errors from Anthropic-compatible endpoints. Should be eitherstrings.Join(system, "\n")or structured content blocks. -
Duplicate user messages for tool results: When
msg.Role == "user"andmsg.ToolCallID != "", the code creates a message withtool_resultcontent — but this case is already handled by thecase "tool"branch. The"user"case should probably not handleToolCallIDat all, or the"tool"branch is redundant. As-is, if something sends a message withRole:"user"and aToolCallID, it gets double-mapped. -
Silent proxy error: In
NewProvider, if proxy URL parsing fails, it logs and continues without a proxy. This means a misconfigured proxy silently falls back to direct connection — which could be a security issue if the proxy is required (e.g., for network isolation). Consider returning an error instead. -
max_tokenstype assertion:options["max_tokens"].(int)will fail if the value comes from JSON unmarshaling (where numbers arefloat64). The test forAcceptsOptionspassesintdirectly, but in production the config is likely JSON-sourced. Should handle bothintandfloat64. -
Breaking change in
factory_provider.go: Theanthropicprotocol case changed fromNewHTTPProviderWithMaxTokensFieldtoNewHTTPProviderWithProtocol, which drops theMaxTokensFieldconfiguration. If any existing anthropic-protocol users relied on custommax_tokens_fieldnames, this is a regression. -
No streaming support: The provider does synchronous request/response. If picoclaw supports streaming for other providers, this will be a feature gap for MiniMax users. Not a blocker but worth noting.
|
Thanks for adding the anthropic_compat provider! The code structure is clean and test coverage is good. However, there are several issues that need to be addressed before merging: 🔴 Critical Issues1. if len(system) > 0 {
requestBody["system"] = system // system is []string
}The Anthropic Messages API expects Fix: requestBody["system"] = strings.Join(system, "\n")2. Duplicate tool_result handling 🟡 Medium Priority Issues3. Proxy config fails silently (security concern) if err != nil {
log.Printf("anthropic_compat: invalid proxy URL %q: %v", proxy, err)
// continues without proxy
}If the proxy is required (e.g., for network isolation), silent fallback to direct connection is a security issue. Consider returning an error instead. 4. if mt, ok := options["max_tokens"].(int); ok {When 5. Breaking change - - return NewHTTPProviderWithMaxTokensField(cfg.APIKey, apiBase, cfg.Proxy, cfg.MaxTokensField)
+ return NewHTTPProviderWithProtocol(cfg.APIKey, apiBase, cfg.Proxy, "anthropic")If any existing users relied on custom Please address at least the critical issues (#1, #2) before merging. Issues #3 and #4 would also be good to fix for production readiness. |
📝 Description
🗣️ Type of Change
🤖 AI Code Generation
🔗 Related Issue
📚 Technical Context (Skip for Docs)
{ "agents": { "defaults": { "workspace": "~/.picoclaw/workspace", "restrict_to_workspace": true, "provider": "", "model": "MiniMax-M2.5-CodingPlan", "max_tokens": 8192, "max_tool_iterations": 20 } "model_list": [ { "model_name": "MiniMax-M2.5-CodingPlan", "model": "anthropic/MiniMax-M2.5", "api_base": "https://api.minimaxi.com/anthropic/v1", "api_key": "your-api-key" } ] }🧪 Test Environment
📸 Evidence (Optional)
Click to view Logs/Screenshots
☑️ Checklist