lzh/picoclaw
1
0
Fork 0
mirror of https://github.com/sipeed/picoclaw.git synced 2026-06-12 18:08:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: reorganize commands and provider architecture

Browse Source 
Refactor command handlers into separate files to improve code organization
and maintainability. Each command (agent, auth, cron, gateway, migrate,
onboard, skills, status) now has its own dedicated file.

Restructure provider creation to support new model_list configuration
system that enables zero-code addition of OpenAI-compatible providers.
Move legacy provider logic to separate file for backward compatibility.

Move configuration functions from config.go to separate files
(defaults.go, migration.go) for better organization.
This commit is contained in:
yinwm
2026-02-19 01:03:34 +08:00
parent a73d8e1a16
commit ef7078a356
23 changed files with 3429 additions and 2241 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/design/provider-refactoring-tests.md
+179
View File
@@ -0,0 +1,179 @@
# Provider Architecture Refactoring - Test Suite Summary
> PRD: `tasks/prd-provider-refactoring.md`
This document summarizes the complete test suite designed for the Provider architecture refactoring.
## Test File Structure
```
pkg/
├── config/
│ ├── model_config_test.go # US-001, US-002: ModelConfig struct and GetModelConfig tests
│ └── migration_test.go # US-003: Backward compatibility and migration tests
├── providers/
│ ├── registry_test.go # US-006: Load balancing tests
│ ├── integration_test.go # E2E integration tests
│ └── factory/
│ └── factory_test.go # US-004, US-005: Provider factory tests
```
---
## Test Case Checklist
### 1. `pkg/config/model_config_test.go` - Configuration Parsing Tests
| Test Name | Purpose | PRD Reference |
|-----------|---------|---------------|
| `TestModelConfig_Parsing` | Verify ModelConfig JSON parsing | US-001 |
| `TestModelConfig_ModelListInConfig` | Verify model_list parsing in Config | US-001 |
| `TestModelConfig_Validation` | Verify required field validation | US-001 |
| `TestConfig_GetModelConfig_Found` | Verify GetModelConfig finds model | US-002 |
| `TestConfig_GetModelConfig_NotFound` | Verify GetModelConfig returns error | US-002 |
| `TestConfig_GetModelConfig_EmptyModelList` | Verify empty model_list handling | US-002 |
| `TestConfig_BackwardCompatibility_ProvidersToModelList` | Verify old config conversion | US-003 |
| `TestConfig_DeprecationWarning` | Verify deprecation warning | US-003 |
| `TestModelConfig_ProtocolExtraction` | Verify protocol prefix extraction | US-004 |
| `TestConfig_ModelNameUniqueness` | Verify model_name uniqueness | US-001 |
### 2. `pkg/config/migration_test.go` - Migration Tests
| Test Name | Purpose | PRD Reference |
|-----------|---------|---------------|
| `TestConvertProvidersToModelList_OpenAI` | OpenAI config conversion | US-003 |
| `TestConvertProvidersToModelList_Anthropic` | Anthropic config conversion | US-003 |
| `TestConvertProvidersToModelList_MultipleProviders` | Multiple provider conversion | US-003 |
| `TestConvertProvidersToModelList_EmptyProviders` | Empty providers handling | US-003 |
| `TestConvertProvidersToModelList_GitHubCopilot` | GitHub Copilot conversion | US-003 |
| `TestConvertProvidersToModelList_Antigravity` | Antigravity conversion | US-003 |
| `TestGenerateModelName_*` | Model name generation | US-003 |
| `TestHasProvidersConfig_*` | Detect old config existence | US-003 |
| `TestValidateMigration_*` | Migration validation | US-003 |
| `TestMigrateConfig_DryRun` | Dry run migration | US-003 |
| `TestMigrateConfig_Actual` | Actual migration | US-003 |
### 3. `pkg/providers/registry_test.go` - Load Balancing Tests
| Test Name | Purpose | PRD Reference |
|-----------|---------|---------------|
| `TestModelRegistry_SingleConfig` | Single config returns same result | US-006 |
| `TestModelRegistry_RoundRobinSelection` | 3-config round-robin selection | US-006 |
| `TestModelRegistry_RoundRobinTwoConfigs` | 2-config round-robin selection | US-006 |
| `TestModelRegistry_ConcurrentAccess` | Concurrent access thread safety | US-006 |
| `TestModelRegistry_RaceDetection` | Data race detection | US-006 |
| `TestModelRegistry_ModelNotFound` | Model not found error | US-006 |
| `TestModelRegistry_EmptyRegistry` | Empty registry handling | US-006 |
| `TestModelRegistry_MultipleModels` | Multiple model registration | US-006 |
| `TestModelRegistry_MixedSingleAndMultiple` | Single/multiple config mix | US-006 |
| `TestModelRegistry_CaseSensitiveModelNames` | Case sensitivity | US-006 |
### 4. `pkg/providers/factory/factory_test.go` - Provider Factory Tests
| Test Name | Purpose | PRD Reference |
|-----------|---------|---------------|
| `TestCreateProviderFromConfig_OpenAI` | Create OpenAI provider | US-004 |
| `TestCreateProviderFromConfig_OpenAIDefault` | Default openai protocol | US-004 |
| `TestCreateProviderFromConfig_Anthropic` | Create Anthropic provider | US-004 |
| `TestCreateProviderFromConfig_Antigravity` | Create Antigravity provider | US-004 |
| `TestCreateProviderFromConfig_ClaudeCLI` | Create Claude CLI provider | US-004 |
| `TestCreateProviderFromConfig_CodexCLI` | Create Codex CLI provider | US-004 |
| `TestCreateProviderFromConfig_GitHubCopilot` | Create GitHub Copilot provider | US-004 |
| `TestCreateProviderFromConfig_UnknownProtocol` | Unknown protocol error handling | US-004 |
| `TestCreateProviderFromConfig_MissingAPIKey` | Missing API key error | US-004 |
| `TestExtractProtocol` | Protocol prefix extraction | US-004 |
| `TestCreateProvider_UsesModelList` | Create using model_list | US-005 |
| `TestCreateProvider_FallbackToProviders` | Fallback to providers | US-005 |
| `TestCreateProvider_PriorityModelListOverProviders` | model_list priority | US-005 |
### 5. `pkg/providers/integration_test.go` - E2E Integration Tests
| Test Name | Purpose | PRD Reference |
|-----------|---------|---------------|
| `TestE2E_OpenAICompatibleProvider_NoCodeChange` | Zero-code provider addition | Goal |
| `TestE2E_LoadBalancing_RoundRobin` | Load balancing actual effect | US-006 |
| `TestE2E_BackwardCompatibility_OldProvidersConfig` | Old config compatibility | US-003 |
| `TestE2E_ErrorHandling_ModelNotFound` | Model not found | FR-30 |
| `TestE2E_ErrorHandling_MissingAPIKey` | Missing API key | FR-31 |
| `TestE2E_ErrorHandling_InvalidAPIBase` | Invalid API base | FR-30 |
| `TestE2E_ToolCalls_OpenAICompatible` | Tool call support | - |
| `TestE2E_AntigravityProvider` | Antigravity provider | US-004 |
| `TestE2E_ClaudeCLIProvider` | Claude CLI provider | US-004 |
### 6. Performance Tests
| Test Name | Purpose |
|-----------|---------|
| `BenchmarkCreateProviderFromConfig` | Provider creation performance |
| `BenchmarkGetModelConfig` | Model lookup performance |
| `BenchmarkGetModelConfigParallel` | Concurrent lookup performance |
---
## Running Tests
```bash
# Run all tests
go test ./pkg/... -v
# Run with data race detection
go test ./pkg/... -race
# Run specific package tests
go test ./pkg/config -v
go test ./pkg/providers -v
go test ./pkg/providers/factory -v
# Run E2E tests
go test ./pkg/providers -run TestE2E -v
# Run performance tests
go test ./pkg/providers -bench=. -benchmem
```
---
## PRD Acceptance Criteria Mapping
| PRD Acceptance Criteria | Test Cases |
|------------------------|------------|
| US-001: Add ModelConfig struct | `TestModelConfig_Parsing`, `TestModelConfig_Validation` |
| US-001: model_name unique | `TestConfig_ModelNameUniqueness` |
| US-002: GetModelConfig method | `TestConfig_GetModelConfig_*` |
| US-003: Auto-convert providers | `TestConvertProvidersToModelList_*` |
| US-003: Deprecation warning | `TestConfig_DeprecationWarning` |
| US-003: Existing tests pass | (existing test files unchanged) |
| US-004: Protocol prefix factory | `TestExtractProtocol`, `TestCreateProviderFromConfig_*` |
| US-004: Default prefix openai | `TestCreateProviderFromConfig_OpenAIDefault` |
| US-005: CreateProvider uses factory | `TestCreateProvider_*` |
| US-006: Round-robin selection | `TestModelRegistry_RoundRobin*` |
| US-006: Thread-safe atomic | `TestModelRegistry_RaceDetection` |
---
## Recommended Implementation Order
1. **Phase 1: Configuration Structure** (US-001, US-002)
- Implement `ModelConfig` struct
- Implement `GetModelConfig` method
- Run `model_config_test.go`
2. **Phase 2: Protocol Factory** (US-004)
- Implement `CreateProviderFromConfig`
- Implement `ExtractProtocol`
- Run `factory_test.go`
3. **Phase 3: Load Balancing** (US-006)
- Implement `ModelRegistry`
- Implement round-robin selection
- Run `registry_test.go` (with `-race`)
4. **Phase 4: Backward Compatibility** (US-003, US-005)
- Implement `ConvertProvidersToModelList`
- Refactor `CreateProvider`
- Run `migration_test.go`
- Verify existing tests pass
5. **Phase 5: E2E Verification**
- Run `integration_test.go`
- Manual testing with `config.example.json`
docs/design/provider-refactoring.md
+334
View File
@@ -0,0 +1,334 @@
# Provider Architecture Refactoring Design
> Issue: #283
> Discussion: #122
> Branch: feat/refactor-provider-by-protocol
## 1. Current Problems
### 1.1 Configuration Structure Issues
**Current State**: Each Provider requires a predefined field in `ProvidersConfig`
```go
type ProvidersConfig struct {
Anthropic ProviderConfig `json:"anthropic"`
OpenAI ProviderConfig `json:"openai"`
DeepSeek ProviderConfig `json:"deepseek"`
Qwen ProviderConfig `json:"qwen"`
Cerebras ProviderConfig `json:"cerebras"`
VolcEngine ProviderConfig `json:"volcengine"`
// ... every new provider requires changes here
}
```
**Problems**:
- Adding a new Provider requires modifying Go code (struct definition)
- `CreateProvider` function in `http_provider.go` has 200+ lines of switch-case
- Most Providers are OpenAI-compatible, but code is duplicated
### 1.2 Code Bloat Trend
Recent PRs demonstrate this issue:
| PR | Provider | Code Changes |
|----|----------|--------------|
| #365 | Qwen | +17 lines to http_provider.go |
| #333 | Cerebras | +17 lines to http_provider.go |
| #368 | Volcengine | +18 lines to http_provider.go |
Each OpenAI-compatible Provider requires:
1. Modify `config.go` to add configuration field
2. Modify `http_provider.go` to add switch case
3. Update documentation
### 1.3 Agent-Provider Coupling
```json
{
"agents": {
"defaults": {
"provider": "deepseek", // need to know provider name
"model": "deepseek-chat"
}
}
}
```
Problem: Agent needs to know both `provider` and `model`, adding complexity.
---
## 2. New Approach: model_list
### 2.1 Core Principles
Inspired by [LiteLLM](https://docs.litellm.ai/docs/proxy/configs) design:
1. **Model-centric**: Users care about models, not providers
2. **Protocol prefix**: Use `protocol/model_name` format, e.g., `openai/gpt-4o`, `anthropic/claude-3-sonnet`
3. **Configuration-driven**: Adding new Providers only requires config changes, no code changes
### 2.2 New Configuration Structure
```json
{
"model_list": [
{
"model_name": "deepseek-chat",
"model": "openai/deepseek-chat",
"api_base": "https://api.deepseek.com/v1",
"api_key": "sk-xxx"
},
{
"model_name": "gpt-4o",
"model": "openai/gpt-4o",
"api_key": "sk-xxx"
},
{
"model_name": "claude-3-sonnet",
"model": "anthropic/claude-3-5-sonnet-20241022",
"api_key": "sk-xxx"
},
{
"model_name": "gemini-3-flash",
"model": "antigravity/gemini-3-flash",
"auth_method": "oauth"
},
{
"model_name": "my-company-llm",
"model": "openai/company-model-v1",
"api_base": "https://llm.company.com/v1",
"api_key": "xxx"
}
],
"agents": {
"defaults": {
"model": "deepseek-chat",
"max_tokens": 8192,
"temperature": 0.7
}
}
}
```
### 2.3 Go Struct Definition
```go
type Config struct {
ModelList []ModelConfig `json:"model_list"` // new
Providers ProvidersConfig `json:"providers"` // old, deprecated
Agents AgentsConfig `json:"agents"`
Channels ChannelsConfig `json:"channels"`
// ...
}
type ModelConfig struct {
// Required
ModelName string `json:"model_name"` // user-facing name (alias)
Model string `json:"model"` // protocol/model, e.g., openai/gpt-4o
// Common config
APIBase string `json:"api_base,omitempty"`
APIKey string `json:"api_key,omitempty"`
Proxy string `json:"proxy,omitempty"`
// Special provider config
AuthMethod string `json:"auth_method,omitempty"` // oauth, token
ConnectMode string `json:"connect_mode,omitempty"` // stdio, grpc
// Optional optimizations
RPM int `json:"rpm,omitempty"` // rate limit
MaxTokensField string `json:"max_tokens_field,omitempty"` // max_tokens or max_completion_tokens
}
```
### 2.4 Protocol Recognition
Identify protocol via prefix in `model` field:
| Prefix | Protocol | Description |
|--------|----------|-------------|
| `openai/` | OpenAI-compatible | Most common, includes DeepSeek, Qwen, Groq, etc. |
| `anthropic/` | Anthropic | Claude series specific |
| `antigravity/` | Antigravity | Google Cloud Code Assist |
| `gemini/` | Gemini | Google Gemini native API (if needed) |
---
## 3. Design Rationale
### 3.1 Problems Solved
| Problem | Old Approach | New Approach |
|---------|--------------|--------------|
| Add OpenAI-compatible Provider | Change 3 code locations | Add one config entry |
| Agent specifies model | Need provider + model | Only need model |
| Code duplication | Each Provider duplicates logic | Share protocol implementation |
| Multi-Agent support | Complex | Naturally compatible |
### 3.2 Multi-Agent Compatibility
```json
{
"model_list": [...],
"agents": {
"defaults": {
"model": "deepseek-chat"
},
"coder": {
"model": "gpt-4o",
"system_prompt": "You are a coding assistant..."
},
"translator": {
"model": "claude-3-sonnet"
}
}
}
```
Each Agent only needs to specify `model` (corresponds to `model_name` in `model_list`).
### 3.3 Industry Comparison
**LiteLLM** (most mature open-source LLM Proxy) uses similar design:
```yaml
model_list:
- model_name: gpt-4o
litellm_params:
model: openai/gpt-4o
api_key: xxx
- model_name: my-custom
litellm_params:
model: openai/custom-model
api_base: https://my-api.com/v1
```
---
## 4. Migration Plan
### 4.1 Phase 1: Compatibility Period (v1.x)
Support both `providers` and `model_list`:
```go
func (c *Config) GetModelConfig(modelName string) (*ModelConfig, error) {
// Prefer new config
if len(c.ModelList) > 0 {
return c.findModelByName(modelName)
}
// Backward compatibility with old config
if !c.Providers.IsEmpty() {
logger.Warn("'providers' config is deprecated, please migrate to 'model_list'")
return c.convertFromProviders(modelName)
}
return nil, fmt.Errorf("model %s not found", modelName)
}
```
### 4.2 Phase 2: Warning Period (late v1.x)
- Print more prominent warnings at startup
- Provide automatic migration script
- Mark `providers` as deprecated in documentation
### 4.3 Phase 3: Removal Period (v2.0)
- Completely remove `providers` support
- Remove `agents.defaults.provider` field
- Only support `model_list`
### 4.4 Configuration Migration Example
**Old Config**:
```json
{
"providers": {
"deepseek": {
"api_key": "sk-xxx",
"api_base": "https://api.deepseek.com/v1"
}
},
"agents": {
"defaults": {
"provider": "deepseek",
"model": "deepseek-chat"
}
}
}
```
**New Config**:
```json
{
"model_list": [
{
"model_name": "deepseek-chat",
"model": "openai/deepseek-chat",
"api_base": "https://api.deepseek.com/v1",
"api_key": "sk-xxx"
}
],
"agents": {
"defaults": {
"model": "deepseek-chat"
}
}
}
```
---
## 5. Implementation Checklist
### 5.1 Configuration Layer
- [ ] Add `ModelConfig` struct
- [ ] Add `Config.ModelList` field
- [ ] Implement `GetModelConfig(modelName)` method
- [ ] Implement old config compatibility conversion
- [ ] Add `model_name` uniqueness validation
### 5.2 Provider Layer
- [ ] Create `pkg/providers/factory/` directory
- [ ] Implement `CreateProviderFromModelConfig()`
- [ ] Refactor `http_provider.go` to `openai/provider.go`
- [ ] Maintain backward compatibility for old `CreateProvider()`
### 5.3 Testing
- [ ] New config unit tests
- [ ] Old config compatibility tests
- [ ] Integration tests
### 5.4 Documentation
- [ ] Update README
- [ ] Update config.example.json
- [ ] Write migration guide
---
## 6. Risks and Mitigations
| Risk | Mitigation |
|------|------------|
| Breaking existing configs | Compatibility period keeps old config working |
| User migration cost | Provide automatic migration script |
| Special Provider incompatibility | Keep `auth_method` and other extension fields |
---
## 7. References
- [LiteLLM Config Documentation](https://docs.litellm.ai/docs/proxy/configs)
- [One-API GitHub](https://github.com/songquanpeng/one-api)
- Discussion #122: Refactor Provider Architecture