pref(provider):删掉产品不需要的driver capabilities值#225
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
/review codex |
There was a problem hiding this comment.
Found a couple of regressions from removing the driver capability contract. The main issue is that the new code no longer preserves an explicit "no discovery / no transport" state, so some failures move from deterministic preflight checks to later provider invocations.
| if driver.Build == nil { | ||
| return fmt.Errorf("provider: driver %q build func is nil", driver.Name) | ||
| } | ||
| if driver.Discover == nil { |
There was a problem hiding this comment.
Hard-requiring Discover here removes the only way to represent a generate-only driver. After this change, callers have to register a stub discover function, and catalog.Service will treat the driver as discovery-capable on every cache miss/background refresh. That reintroduces ResolveDiscoveryConfig/API-key failures for providers that should have stayed on configured or default models, which is exactly the behavior the deleted catalog tests were protecting against.
| return nil, err | ||
| } | ||
|
|
||
| modelProvider, err := s.providerFactory.Build(ctx, runtimeCfg) |
There was a problem hiding this comment.
Removing the transport preflight here means Run now always builds the provider and calls Generate, even when the driver cannot actually stream tool calls. That turns a deterministic contract error into a later provider/network failure (or silent tool dropping) after we've already crossed the provider boundary. The deleted runtime tests covered this regression; if DriverTransportCapabilities is going away, runtime still needs another fail-fast way to reject unsupported streaming/tool transport before invocation.
| @@ -42,9 +42,6 @@ func (g *compactSummaryGenerator) Generate(ctx context.Context, input contextcom | |||
| strings.TrimSpace(g.providerConfig.APIKey) == "" { | |||
| return "", errors.New("runtime: compact summary generator provider config is incomplete") | |||
There was a problem hiding this comment.
The same regression exists in compact generation: without the old streaming capability check, we now invoke the provider and only fail after the call returns, often as a generic stream/protocol error. That makes unsupported drivers look like flaky runtime failures instead of a clear configuration/contract problem.
1 participant
变更说明
本次改动的目标是移除
provider层中已经失去实际区分意义的DriverTransportCapabilities及其配套逻辑。在当前项目范围内，正式支持的 driver 数量有限，且都应直接实现统一的
Generate与DiscoverModels合同。继续保留一组几乎总是被写成固定值的 capabilities 字段，只会形成重复真相，增加 runtime、catalog、registry 与测试层的维护负担。因此，本次改动将 capabilities 从“driver 元数据声明”收缩回“具体 driver 实现与运行时错误本身”，让系统重新以统一合同、显式错误和模型能力提示为主，而不是依赖额外的布尔字段做分流。
主要改动
internal/provider/contracts.go中的DriverTransportCapabilities定义。internal/provider/registry.go中DriverDefinition.Capabilities字段及对应查询逻辑。internal/provider/openaicompat/driver.go中的 capability 声明。internal/runtime/runtime.go与internal/runtime/compact_generator.go中基于 capability 的运行前预检查。internal/provider/catalog/service.go中基于ModelDiscovery的判断分支，统一改为仅按 driver 是否已注册决定是否进入 discovery 合同。预期收益
provider -> registry -> runtime -> catalog之间的职责边界，减少不必要的元数据耦合。openai-response、gemini、anthropic等主流 driver 保持更短、更直接的接入路径。