Skip to content

fix(provider):删除误加入模型发现逻辑问题#566

Merged
phantom5099 merged 7 commits into1024XEngineer:mainfrom
phantom5099:file-checkpoint
May 6, 2026
Merged

fix(provider):删除误加入模型发现逻辑问题#566
phantom5099 merged 7 commits into1024XEngineer:mainfrom
phantom5099:file-checkpoint

Conversation

@phantom5099
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown

@fennoai fennoai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 2 issues worth addressing. The main regression is that discovery for several non-openaicompat OpenAI-compatible drivers is now silently turned into an empty success path, while the rest of the config/selection stack still allows model_source: discover for those drivers.

Comment thread internal/provider/deepseek/driver.go Outdated
return nil, err
}
return p.DiscoverModels(ctx)
return nil, nil
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discover now returns nil, nil, which silently breaks custom providers that still use the dedicated deepseek/qwen/glm/mimo/minimax drivers with model_source: discover. The config layer still treats those drivers as discovery-capable (isOpenAICompatLike / custom-provider normalization), and provider add still defaults custom providers to discovery, so this changes a supported path into "success with zero models" instead of surfacing a config error. If discovery is intentionally unsupported now, these drivers should reject discovery explicitly; otherwise keep the old /models implementation.

@@ -371,74 +354,6 @@ func TestDiscoverContractAcrossDrivers(t *testing.T) {
expectedHeader: "x-api-key",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These discovery contract cases were removed without adding any replacement assertion for the new contract. That leaves the regression above completely untested: the suite no longer verifies either /models compatibility or that model_source: discover is rejected for these drivers. Please add a test that covers whichever behavior is now intended, otherwise this config/code drift will keep passing CI.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@phantom5099
Copy link
Copy Markdown
Collaborator Author

/code 提高测试覆盖率到100%

@fennoai
Copy link
Copy Markdown

fennoai Bot commented May 6, 2026

已完成

已补充这次 PR 缺失的覆盖用例,并推送到当前分支 file-checkpoint

本次补测聚焦 Codecov 指出的缺口:

  • internal/provider/registry_test.go:补了 DiscoverModelsDiscover == nil 时返回 discovery 配置错误、SupportsDiscovery 的大小写/缺失分支,以及无 ValidateCatalogIdentity 校验器时的通过路径。
  • internal/provider/catalog/service_test.go:补了 driver 已注册但不支持 discovery 时,discoverAndPersist 返回 discovery 配置错误的场景。

已验证:

  • go test ./internal/provider ./internal/provider/catalog
  • go test ./...

提交:47a0d9e8 test(provider): cover discovery capability branches

View job run

Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: phantom5099 <245659304+phantom5099@users.noreply.github.com>
@phantom5099 phantom5099 merged commit be46f74 into 1024XEngineer:main May 6, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants