refactor:TUI 架构渐进式整理并补充单元测试#43
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
minorcell
left a comment
minorcell
left a comment
There was a problem hiding this comment.
整体重构方向清晰，分层目标明确，测试覆盖也补充得比较扎实。以下是几处值得关注的具体问题，请评估是否需要在本 PR 或后续跟进修复。
| import ( | ||
| "time" | ||
|
|
||
| "go-llm-demo/internal/tui/services" |
There was a problem hiding this comment.
[架构分层违反] state 层导入了 services 包，这与架构文档中「纯状态定义，只存放纯粹的 Go 结构体」的原则相矛盾。
state 是最底层的数据容器，不应依赖 services（后者本身依赖 internal/server/infra/...）。
建议：将 MemoryStats 在 state 包内自行定义（或复制所需字段），或抽到独立的 types 包中，切断 state → services → server/infra 这条依赖链。
| HistoryTurns int | ||
| Generating bool | ||
| ActiveModel string | ||
| MemoryStats services.MemoryStats |
There was a problem hiding this comment.
[同上] MemoryStats services.MemoryStats 是 state 引入 services 依赖的根因。若将此字段的类型改为在 state 包内定义，上面的 import 可以一并去掉。
|
|
||
| const ( | ||
| ModeChat Mode = iota | ||
| ModeCodeInput |
There was a problem hiding this comment.
[未使用的常量] ModeCodeInput 和 ModeMemory 在整个代码库中未见任何引用（含测试）。如果这两个模式是将来规划的占位符，建议加注释说明；如果当前不需要，可以先删除，避免 iota 序号被无谓占用导致潜在混乱。
| } | ||
|
|
||
| func (i InputBox) Render() string { | ||
| helpText := "[Enter换行 F5/F8发送 PgUp/PgDn滚动]" |
There was a problem hiding this comment.
[逻辑可读性] 默认值设置后立即在常规分支（!Generating）中被覆盖，使读者需要二次反应才能理解真正的默认行为。
建议改写为：
var helpText string
if i.Generating {
helpText = "[Enter换行 F5/F8发送 PgUp/PgDn滚动]"
} else {
helpText = "[Enter换行 F5/F8发送 Ctrl+V粘贴 PgUp/PgDn滚动]"
}这样 Generating=true/false 的分支一目了然，也更容易在未来单独扩展两种状态。
| workspaceRoot, err := tools.ResolveWorkspaceRoot(*workspaceFlag) | ||
| workspaceRoot, err := bootstrap.PrepareWorkspace(*workspaceFlag) | ||
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "解析工作区失败: %v\n", err) |
There was a problem hiding this comment.
[错误信息不准确] PrepareWorkspace 内部合并了 ResolveWorkspaceRoot 和 SetWorkspaceRoot 两步，但调用方只有一条错误信息「解析工作区失败」。如果是 SetWorkspaceRoot 失败，这条消息会产生误导。
可以让 PrepareWorkspace 对两种失败返回带前缀的错误（fmt.Errorf("解析工作区: %w", err) / fmt.Errorf("设置工作区: %w", err)），调用方统一用 "初始化工作区失败: %v" 即可，信息更完整。
| * 将这些服务注入到 `app` 层中。 | ||
| * **关键作用**：通过在这一层注入不同的实现，可以轻松实现“离线测试模式”或“Mock 测试”。 | ||
|
|
||
| ### 应用逻辑层 (App/Core) - `internal/tui/app/` |
There was a problem hiding this comment.
[文档与实现不一致] 这里写的目录路径是 internal/tui/app/，但实际代码中保留的是 internal/tui/core/，PR 描述中也说明有意不强行重命名。
建议将此处（及下方目录结构示意图中的 app/ 引用）同步修改为 core/，避免文档成为误导性参考。
| fs := flag.NewFlagSet("tui", flag.ContinueOnError) | ||
| fs.SetOutput(stderr) | ||
|
|
||
| workspaceFlag := fs.String("workspace", "", "鎸囧畾宸ヤ綔鍖烘牴鐩綍") |
|
@creatang 这个PR先合并了，后续你们组应该讨论一下“把喂给 llm 的信息换成英文”的事情，包括这些测试用例。 |
好的，我们今天中午确定一下~ |
今天完成测试之后，待其他相关pr合并后进行修复 |
test: improve coverage for provider defaults and migration script
test: improve coverage for verification and acceptance
2 participants
背景
本次 PR 继续基于#39，完成
docs/TUI_REFINED_ARCHITECTURE.md，并依据文档对 TUI 做渐进式整理。目标不是重写功能，也不是强行复制文档结构，而是在不改变现有启动方式、交互行为、流式输出和命令语义的前提下，逐步收敛 TUI 的分层边界，并补齐一批稳定的单元测试。
本次完成内容
core下沉到componentsstate层MessageChatState与UIStatecore.Model中可独立的纯状态聚合到statebootstrap层Program装配逻辑cmd/tui/main.go更接近纯入口services边界internal/tui/services/internal/tui/infra路径与空目录core/msg.go中未使用的消息辅助符号cmd/tui/main_test.gointernal/tui/bootstrap/setup_test.gointernal/tui/components/layout_helpers_test.gointernal/tui/core/view_test.gointernal/tui/core/update_test.go架构结果
当前 TUI 已基本形成以下职责边界：
cmd/tui/internal/tui/bootstrap/internal/tui/core/internal/tui/state/internal/tui/components/internal/tui/services/保持不变的内容
本次 PR 不改变以下行为：
测试
已执行：
go test ./cmd/tui ./internal/tui/... 结果通过。 说明 这次改动是“渐进式贴近文档”，不是强行完全照搬文档结构。 有意避免了以下高风险或低收益动作： - 不必要的新接口层 - 过薄且无价值的抽象堆叠 - 仅为了命名一致而强行将 core 直接重命名为 app - 对 TUI 以外模块的侵入式修改