feat(core): replace OpenAI SDK with native HTTP client #393
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces the OpenAI Java SDK with a native HTTP client implementation to support multiple OpenAI-compatible API providers. The change eliminates SDK dependency overhead while maintaining backward compatibility with the public API.
Key Changes:
- Removed
openai-javaSDK dependency - Implemented native HTTP client using OkHttp
- Added comprehensive OpenAI DTOs for request/response handling
- Introduced provider capability support for different API vendors
- Refactored
OpenAIChatModelto be stateless and HTTP-based
Reviewed changes
Copilot reviewed 95 out of 103 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| OpenAIClientExceptionTest.java | Tests exception handling for HTTP status codes |
| OpenAIChatModelTest.java | Integration tests with MockWebServer |
| OpenAIChatModelE2ETest.java | E2E tests against real OpenRouter API |
| GeminiChatModelE2ETest.java | E2E tests for Gemini via OpenRouter |
| GLMChatModelE2ETest.java | E2E tests for GLM API compatibility |
| GLMApiDebugTest.java | Debug tests for GLM API behavior |
| DoubaoE2ETest.java | E2E tests for Doubao provider |
| DeepSeekChatModelE2ETest.java | E2E tests for DeepSeek API |
| ResponseFormatTest.java | Tests for structured output DTOs |
| OpenAIBuilderIndependenceTest.java | Builder pattern independence tests |
| ProviderCapabilityTest.java | Tests for provider capability detection |
| OpenAIToolsHelperTest.java | Tests for tools helper functionality |
| OpenAIStreamingToolCallTest.java | Tests for streaming tool call parsing |
| OpenAIResponseParserTest.java | Tests for response parsing logic |
| OpenAIMultiAgentFormatterTest.java | Tests for multi-agent message formatting |
| OpenAIMessageConverterTest.java | Tests for message conversion |
| OpenAIConverterUtilsTest.java | Tests for converter utilities |
| OpenAIConversationMergerTest.java | Tests for conversation merging |
| OpenAIChatFormatterTest.java | Tests for chat message formatting |
| Multiple provider files | License header updates and test timeout increases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (modelName == null || modelName.isEmpty()) { | ||
| // Try common OpenRouter GLM model names | ||
| String[] openRouterGLMModels = { | ||
| "z-ai/glm-4.6", // GLM 4.6 |
There was a problem hiding this comment.
Model name format appears incorrect. Based on the OpenRouter API documentation and provider patterns used elsewhere in the codebase, the correct format should use zhipu/ prefix (e.g., zhipu/glm-4) instead of z-ai/. This inconsistency may cause model lookup failures when using OpenRouter.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 103 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class VisionToolIntegrationE2ETest { | ||
|
|
||
| private static final Duration TEST_TIMEOUT = Duration.ofSeconds(60); | ||
| private static final Duration TEST_TIMEOUT = Duration.ofSeconds(180); |
There was a problem hiding this comment.
Test timeout was increased from 60 to 180 seconds. This significant change should be documented in the PR description or code comments to explain why such a long timeout is needed. Consider if this indicates a performance issue that should be addressed separately, or if the tests genuinely require this much time for API calls.
| class ToolSystemE2ETest { | ||
|
|
||
| private static final Duration TEST_TIMEOUT = Duration.ofSeconds(45); | ||
| private static final Duration TEST_TIMEOUT = Duration.ofSeconds(180); |
There was a problem hiding this comment.
Test timeout was increased from 45 to 180 seconds. This is a 4x increase which should be justified. Document why such a long timeout is required and consider if tests can be optimized or if this indicates issues with the API provider responses.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 103 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 103 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…usage This commit adds two enhancements to the HTTP transport layer: 1. SSL Certificate Verification Control (fixes agentscope-ai#344): - Add `ignoreSsl` option to HttpTransportConfig for development/testing - Implement trust-all SSL context in OkHttpTransport - Support custom OkHttpClient injection via builder 2. Default Token Usage in Streaming (fixes agentscope-ai#355): - Automatically include `stream_options: {"include_usage": true}` for all streaming API calls - Ensures token usage is available in final response chunk - Compatible with OpenAI, DashScope, Bailian, and other providers 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Fixes #95
Description
This PR replaces the OpenAI Java SDK dependency with a native HTTP client implementation, enabling support for multiple OpenAI-compatible API providers.
Background
The previous implementation relied on the
openai-javaSDK, which:Changes Made
openai-javadependency fromagentscope-core/pom.xmlOpenAIClient.java- HTTP client wrapperOpenAIConfig.java- configuration classdto/OpenAIRequest.java,dto/OpenAIResponse.javadto/OpenAIMessage.java,dto/OpenAIToolCall.java, etc.ProviderCapability.java- defines tool_choice support per providerOpenAIChatModelto be stateless and HTTP-basedTesting
formatter.openaipackageBreaking Changes
None. The public API remains unchanged; only the internal implementation switches from SDK to HTTP.
Checklist
mvn spotless:applymvn test)