Skip to content

pref(provider):收敛公共契约、配置装配与模型目录边界#209

Merged
phantom5099 merged 21 commits into
1024XEngineer:mainfrom
phantom5099:main
Apr 10, 2026
Merged

pref(provider):收敛公共契约、配置装配与模型目录边界#209
phantom5099 merged 21 commits into
1024XEngineer:mainfrom
phantom5099:main

Conversation

@phantom5099

Copy link
Copy Markdown
Collaborator

变更说明

本 PR 基于 provider-phase2-final-plan 的 Phase A / Phase B 目标,对当前项目已落地的 provider 重构工作做统一收口。 已完成了 Phase A 的主体:provider 公共契约去 OpenAI Chat 语义、driver 能力模型补齐以及流式 EOF 异常处理稳定化;Phase B 主体:builtin/custom provider 来源管理、远程模型目录缓存、连接身份、共享类型迁移、provider.RuntimeConfigcatalog` 输入边界收口,以及 selection 行为修正。

主要改动

  • 收敛 provider 公共调用契约,将旧的 Chat 语义统一调整为 Generate / GenerateRequest,让 runtime 与 driver 的交互模型脱离 OpenAI 专属命名。
  • 在 provider 根包补齐 driver transport 能力模型,并在 runtime 中接入相应的能力检查,提前识别不支持 streaming / tool transport / model discovery 的 driver。
  • 修复流式响应在异常 EOF 场景下被误当作正常完成的问题,并补齐对应回归测试。
  • ModelDescriptorModelCapabilityHints 等 discovery/catalog 共享类型迁移到 internal/provider/types,让 provider、catalog、selection、TUI 共享同一套领域模型。
  • 引入并规范化 provider.ProviderIdentity,按 driver 专属维度生成稳定缓存键,用于 provider 去重和 cache/models/*.json 远程模型目录缓存。
  • 新增 custom provider 目录加载逻辑,由 config 负责扫描 providers/*/provider.yaml、合并 builtin/custom、执行去重与校验,并明确 custom provider 的模型来源只能是远程 discovery 与缓存。
  • 新增 provider.RuntimeConfig,让 provider 构建与 discovery 使用 provider-owned 的最小运行时输入,而不是继续直接依赖 ResolvedProviderConfig
  • provider/catalog 的输入从过细的 getter 协议进一步收敛为 provider-owned 的具体输入结构,统一承载 identity、默认模型、已配置模型与 lazy discovery resolver。
  • 修正 selection 行为:启动阶段只自动修正 current_model,对于 unsupported driver 改为显式报错,不再静默改写 selected_provider
  • 将旧的 internal/provider/openai 目录语义收敛为 internal/provider/openaicompat,统一代码层对 OpenAI-compatible 协议的命名表达。

预期收益

  • provider 主链的公共抽象不再被 OpenAI Chat 语义绑死,为多协议继续演进提供更稳定的起点。
  • runtime 能更早识别 driver 基础能力不足,减少深层协议失败与错误扩散。
  • 流式 EOF 异常不再被静默吞掉,provider 主链稳定性和可观测性更好。
  • builtin/custom provider 的来源、去重、缓存与模型目录行为统一收敛,配置边界更清晰。
  • provider/typesprovider.RuntimeConfig 与 provider-owned catalog 输入结构共同构成更稳定的跨层契约,降低 config/provider/catalog/runtime 之间的缠绕。
  • 启动阶段不再偷偷改写用户的 provider 选择,selection 语义更符合“用户意图”和“当前 runtime 能力”分离的原则。
  • 为后续 Phase C 及之后的协议拆分、native driver 接入和多模态/能力治理保留了更干净的演进空间。

@phantom5099

Copy link
Copy Markdown
Collaborator Author
  • 启动校验和切换 provider 时,缓存一旦没有命中,就会同步去远端拉模型列表,网络慢时会把启动或切换过程卡住。
  • 用户切到一个不存在的模型时,接口虽然返回失败,但配置其实已经被偷偷改成默认模型了。
  • ~/.neocode/providers 下面只要多出一个没有 provider.yaml 的目录,配置加载就会直接报错。

@minorcell

Copy link
Copy Markdown
Member

/review

fennoai[bot]

This comment was marked as outdated.

@phantom5099

Copy link
Copy Markdown
Collaborator Author

@xgopilot

@minorcell

Copy link
Copy Markdown
Member

@phantom5099

其实是:/simplify 这个命令,你再试试

@1024XEngineer 1024XEngineer deleted a comment from fennoai Bot Apr 10, 2026
@phantom5099

Copy link
Copy Markdown
Collaborator Author

研究结果

  1. internal/config/selection.go:303即使快照为空,也能保持所选自定义提供者有效,这意味着启动时可以保持空。 然后将空模型直接转发到提供者调用中,并用 硬失败。实际上,没有缓存目录的自定义提供者可能会成功启动,但在第一个用户消息时失败,而不是在选择或启动时暴露问题。current_model``internal/runtime/runtime.go:272``internal/provider/openaicompat/request.go:22``model is empty
  2. 新的权限界面可能会在已经开放的拣货器后面陷入僵局。 将所有关键事件映射到当 时,而 只渲染该状态下的拣选者。但当A到达时,它不会关闭拣货机。如果在拣选器开启时提出许可请求,批准提示会被隐藏,其密钥处理永远无法访问,因此无法在界面中批准或拒绝运行。internal/tui/core/app/update.go:253``updatePicker()``ActivePicker != pickerNone``internal/tui/core/app/view.go:133``internal/tui/core/app/update.go:1041``permission_request

注释

go test ./...经过这条支线。

查看作业运行

  1. 我可以接受改成启动时报错,但不允许这个过程会杀死启动中的进程,导致用户无法在进程中改成其他的有效配置,反而要去手动更改配置文件;也不希望这个修改涉及到其他层(比如tui、app等,只允许修改provider、小幅度修改config),如果你没有合适方案,就不要应用修改
  2. 第2个问题涉及到了其他层修改,并非provider职责,不允许修改

@phantom5099

Copy link
Copy Markdown
Collaborator Author

/simplify

@fennoai

fennoai Bot commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

完成

本次只处理仍值得修、且不跨出作者约束范围的内容。

已完成:

  • 核对历史 review comment,确认 identity/api_style 不一致问题和权限弹窗问题已经在当前分支解决
  • 保留并确认 docs/session-persistence-design.md,恢复 session JSON 结构、token 累计持久化、工作区分桶等说明
  • 按子代理建议收口重复文档:
    • docs/runtime-provider-event-flow.md
    • docs/context-compact.md
      这两处现在直接引用 docs/session-persistence-design.md,不再重复展开 session/token 持久化细节
  • 运行 go test ./...,通过
  • 已将新增提交推送到 PR 头分支 phantom5099:main

未做:

  • 没有再改 tui/app 相关逻辑;历史评论里那部分已被明确限制为不在本次处理范围内
  • 没有再做额外代码层简化;子代理评估后认为当前最合适的是文档去重,不建议继续动 internal/session/store.go

本次新增提交:ca8f094 docs(session): dedupe persistence references

View job run

xgopilot and others added 2 commits April 10, 2026 05:53
Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: phantom5099 <245659304+phantom5099@users.noreply.github.com>
Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: phantom5099 <245659304+phantom5099@users.noreply.github.com>
@1024XEngineer 1024XEngineer deleted a comment from fennoai Bot Apr 10, 2026
@wynxing

wynxing commented Apr 10, 2026

Copy link
Copy Markdown
Collaborator

/review

fennoai[bot]

This comment was marked as outdated.

@phantom5099

Copy link
Copy Markdown
Collaborator Author

/review -codex

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Found 3 issues that still look actionable in the current diff.

Comment thread internal/config/model.go
return err
}
if strings.TrimSpace(c.CurrentModel) == "" {
if strings.TrimSpace(c.CurrentModel) == "" && selected.Source != ProviderSourceCustom {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Allowing current_model to stay empty for a selected custom provider leaves the config in a state that passes Load()/EnsureSelection(), but the runtime still always sends cfg.CurrentModel into provider.Generate(). Custom providers also have an empty DefaultModel, so the first user run now fails later in openaicompat.buildRequest() with model is empty. This regresses the main path from “load-time validation” to “first request blows up” for a freshly added custom provider with no cached catalog.

}
var models []providertypes.ModelDescriptor
if providerCfg.Source == ProviderSourceCustom {
models, err = s.catalogs.ListProviderModels(ctx, input)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SelectProvider() still does a synchronous ListProviderModels() for custom providers. On a cache miss that path resolves discovery config and calls remote model discovery before the provider switch completes, and the TUI invokes it with context.Background(). In practice a slow or hung endpoint can still freeze provider switching indefinitely instead of switching immediately and letting catalog refresh happen in the background.

Comment thread README.md
@@ -129,6 +129,7 @@ go run ./cmd/neocode
- **[扩展 Provider](docs/guides/adding-providers.md)** — 如何添加新的模型提供商

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This README now advertises docs/guides/adding-providers.md as the contributor guide, but that guide still describes the pre-refactor API (internal/provider/openai, driver "openai", ChatRequest/Chat). After this PR the canonical names are internal/provider/openaicompat, driver "openaicompat", and GenerateRequest/Generate. Surfacing the stale guide here will send new provider implementations down a non-compiling path.

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Found 1 regression that still looks worth fixing before merge.

Comment thread internal/config/model.go
return err
}
if strings.TrimSpace(c.CurrentModel) == "" {
if strings.TrimSpace(c.CurrentModel) == "" && selected.Source != ProviderSourceCustom {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Allowing current_model to stay empty for a selected custom provider makes config load succeed, but this PR does not guarantee that field is ever backfilled on a cold start. EnsureSelection() now explicitly accepts the empty state when the custom catalog snapshot is empty, while runtime.Run() still forwards cfg.CurrentModel straight into GenerateRequest, so a freshly added custom provider boots and then fails on the first real request with openai provider: model is empty. That leaves the main run path broken for the exact no-cache case this change is trying to unblock.

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Found 2 regressions worth fixing before merge.

Comment thread internal/config/model.go
return err
}
if strings.TrimSpace(c.CurrentModel) == "" {
if strings.TrimSpace(c.CurrentModel) == "" && selected.Source != ProviderSourceCustom {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Allowing a selected custom provider to keep current_model empty makes startup succeed, but it still leaves the main loop in an unusable state: runtime.Run() forwards cfg.CurrentModel unchanged into GenerateRequest.Model, and the OpenAI-compatible driver still rejects an empty model. Because the async catalog refresh only repopulates the picker and never auto-selects a discovered model, a freshly selected custom provider can boot successfully and then fail on the first user message with model is empty until the user manually picks one. That is still a broken primary flow for the new custom-provider path.

@@ -731,7 +762,7 @@ func (s *Service) callProviderWithRetry(
streamDone := make(chan error, 1)
go s.forwardProviderEvents(ctx, runID, sessionID, streamEvents, streamDone, acc)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This retry loop resets only the local accumulator before retrying. Any text_delta / tool_call_* events that the first attempt already emitted have already been forwarded to TUI via forwardProviderEvents, so a retry after a mid-stream retryable failure leaves stale partial output on screen and then appends the successful retry on top of it. In practice a transient disconnect after a few streamed tokens will surface duplicated/ghost assistant text even though only the final attempt is persisted. We need either buffered forwarding that is committed only on success, or an explicit rollback/reset event before retrying.

@phantom5099 phantom5099 merged commit 9010bbb into 1024XEngineer:main Apr 10, 2026
2 checks passed
@fennoai fennoai Bot mentioned this pull request Apr 15, 2026
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.

4 participants