spec(project): 定义 Project Workspace MVP - #53
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
hifizz
changed the title
hifizz
left a comment
hifizz
left a comment
There was a problem hiding this comment.
⛔ 结论：Blocking review — 修复前不应合并
GitHub 不允许 PR 作者对自己的 PR 提交
REQUEST_CHANGES，因此本 review 以COMMENT事件提交；严重性和合并结论等同于 Request changes。
我按 thermo-nuclear-code-quality-review 的标准审查了当前 HEAD，而不是按 PR 标题把它当作文档变更。当前 PR 已经是 84 commits、61 files、+11,084/-513 的完整实现 rollout，但描述仍声称“不包含数据库、API、UI 或业务代码实现”。在修复下面的问题前不应合并。
合并阻塞
- Project Panel 用完整 bootstrap hydration 刷新资源，会清空乐观命令并重建正在运行的 stream state，破坏与 Panel 无关的生成状态。
- Contract 编辑没有在开始编辑时冻结
contractVersion，后台刷新后旧草稿可携带新版本提交，绕过 409 并静默覆盖并发修改。 - Project File 上传是不可恢复的多步骤客户端事务；PUT/ingest/页面中断后会留下永久
uploadingmembership，且重复实现了已有 Attachment 上传管线。 - “统一总预算”实际上只对每个唯一文件渲染一次计费，但同一 Attachment 的正文会被写回每一次历史 file part；最终送模上下文可远超 120k。
- Attachment 单 Project 归属采用 check-then-insert；不同 Project 并发绑定会把唯一约束竞争暴露成 500，而不是稳定的
STATE_CONFLICT。 - Archived Project 的只读约束没有成为服务端统一不变量；rename/updateThread/edit/retry 等写路径仍可执行。
- 当前
Agent Evals已红：smoke manifest 现在是 10 个 case，但基础契约测试仍硬编码为 9。
结构性整改要求
- 将“首次完整 hydration”和“Project Workspace 资源增量刷新”分开；后者只 merge Contract/Files/Artifacts，并使用单调版本或 revision 防止旧响应回写。
- 将 Project 编排移出通用
lib/chatAttachment resolver。底层 renderer 应接收领域无关输入并保持泛型类型；Project 的 manifest、选择、统计与 provenance 由 ThreadChat application/context compiler 负责。当前lib/chat反向依赖persistence/mappers，并通过 cast 恢复类型，ownership 已经倒置。 - 多文件 RAG 不应在循环中串行等待；先确定性分配预算，再并行执行独立 retrieval。
- Project PATCH 应使用显式 discriminant，或拆分语义端点；不要继续通过字段存在性猜命令类型。
- 每次 generation 完成后不应下载整个 Project bootstrap 只为发现新增 Artifact；用终态事件携带 artifact delta，或提供 cursor/delta endpoint。
测试与合并门禁
- 新增的
project-panel-ui-contract.test.mjs主要读取源码做正则断言，甚至把hydrateProject(bootstrap)本身当作通过条件；这不是 UI/e2e，也无法发现本 review 中的 stream、乐观状态、并发草稿、焦点和上传中断问题。 project-panel-workspace-state.test.mjs只证明 workspace 对象未变，同时明确接受 live stream 被降级成background；需要补真正的状态隔离测试。.github/workflows/project-workspace-ci.yml只监听临时 feature branch 的push，不监听pull_request，因此不是可复用、不可绕过的合并门禁。tasks.md已把 6.7、7.4 等验收项勾成完成，但 8.2–8.4 仍未完成，且当前 CI 已失败；请按真实证据回退勾选状态。- 更新 PR 标题/描述，使其准确反映迁移、API、上下文、状态管理、UI、测试与 eval 变更，并在合并前恢复全部 required checks 与预览部署。
下面的 inline comments 是需要逐项关闭的高置信度问题；我刻意没有加入纯命名、格式或偏好性评论。
|
|
||
| const refresh = useCallback(async () => { | ||
| const bootstrap = await client.getProject(projectId) | ||
| store.getState().hydrateProject(bootstrap) |
There was a problem hiding this comment.
[blocker · state corruption] 这里不能用全量 hydrateProject 做 Project 资源刷新。hydrateProject 会替换 messages/threads/artifacts/streams，并清空 optimisticByCommandId；打开 Panel 或上传后的 refresh 因此能重置无关的进行中 generation、丢失 liveMessage，也可能用较旧 bootstrap 回退本地新状态。这直接违反“打开/关闭 Panel 不改变当前生成状态”的设计约束。请增加只 merge Project Contract / Files / Artifacts 的增量 action，并按 contractVersion/revision 拒绝旧响应；完整 hydration 只用于首次加载或显式重载整个会话。
|
|
||
| const beginEdit = () => { | ||
| if (!project) return | ||
| setTargetDraft(project.target ?? "") |
There was a problem hiding this comment.
[blocker · lost update] 乐观并发 token 必须在 beginEdit 时和文字草稿一起冻结。当前这里只保存 target/instructions，提交时 commands.updateProjectContract 默认读取“此刻” store 中的 contractVersion；编辑期间任何后台 refresh 若推进了版本，旧草稿就会带新版本提交并绕过本应发生的 409，静默覆盖另一页面的修改。请把 expectedContractVersion 纳入 edit draft，并从这里原样传到底层命令，不能在 submit 时重新读取。
| uploadUrl: string | ||
| } | ||
|
|
||
| await callbacks.onAttachmentCreated?.(id) |
There was a problem hiding this comment.
[blocker · unrecoverable upload lifecycle] membership 在这里建立后，后续 PUT 网络失败、页面刷新或 ingest 未调用都不会把 Attachment 标记为 failed 或解除 membership，刷新后该文件会永久停在 uploading；反过来，如果本 callback 失败，已创建的 Attachment row 也会泄漏。这里还重复实现了 attachment-adapter.ts 的 create/PUT/ingest 管线。请收敛成一个 canonical、可恢复的上传状态机，为每个 post-create failure 提供 server-side fail/cleanup/retry reconciliation，并覆盖每一步中断的测试。
| const id = attachmentIdFromUrl(part.url) | ||
| const row = id ? rowById.get(id) : undefined | ||
| const resolved = id ? rendered.get(id) : undefined | ||
| if (resolved) return Promise.resolve({ type: "text", text: resolved.text }) |
There was a problem hiding this comment.
[blocker · budget/dedup contract is false] 候选虽然按 Attachment id 只渲染一次，但这里会把同一个 resolved.text 写回它在历史中的每一次 file part。若同一文件被引用 N 次，最终上下文注入 N 份正文，而 remainingBudget 只扣一次；此外 Project manifest 和外层 <project_file>/<project_files> 包装是在渲染预算之外追加的。因此配置的 120k 不是完整序列化后输入的实际上限。请把每个 Attachment 正文放到唯一的 context section，消息中只保留轻量 reference，并对完整序列化后的最终输入执行总预算验收。
| return toProjectFileDTO(current) | ||
| } | ||
| const now = new Date() | ||
| await tx.insert(projectFiles).values({ |
There was a problem hiding this comment.
[blocker · check-then-insert race] 两个不同 Project 同时绑定同一 Attachment 时，事务锁住的是不同 project row，两边都可能先读到 membership = null，随后其中一个触发 attachment_id 唯一约束。当前 route error mapping 会把这个数据库异常变成 500，而不是稳定的 STATE_CONFLICT。请锁定 Attachment row，或使用原子 insert/upsert 并把唯一冲突翻译为业务冲突；同时增加真正的并发 Promise.all 验收测试，而不是只测顺序调用。
| return trimmed.length > 0 ? trimmed : null | ||
| } | ||
|
|
||
| function assertWritableProject(project: { archivedAt: Date | null }): void { |
There was a problem hiding this comment.
[blocker · incomplete server invariant] 这个 writable guard 目前是本文件的局部 helper，且只在 Contract/File 路径使用。renameProject、updateThread，以及其他文件中的 editLatestTurn、retryMessage 仍可写 archived Project；UI 隐藏按钮不能替代服务端不变量。请把它提升为共享 application guard，并让除明确允许的 unarchive/delete/stop/feedback 外的所有写命令统一调用，补 API 级命令矩阵测试。
|
|
||
| async function refreshProjectArtifacts(projectId: string) { | ||
| try { | ||
| const bootstrap = await client.getProject(projectId) |
There was a problem hiding this comment.
[major · O(project history) refresh and stale writeback] 每次 generation 完成后，为了拿新增 Artifact 这里下载完整 Project bootstrap（全部 threads/messages/files/artifacts），随后还回写 bootstrap.project。成本会随项目历史线性增长；一个较慢的旧 GET 也可能把刚保存的新 Contract 回退到旧 DTO。请让 terminal SSE/command result 携带 artifact delta，或增加带 cursor/revision 的 artifacts endpoint；此路径不应读取或写回 Project 元数据。
| "memory-same-thread-fact", | ||
| "memory-cross-project-no-leak", | ||
| "multimodal-synthetic-chart", | ||
| "project-contract-target-instructions", |
There was a problem hiding this comment.
[blocker · current CI is deterministically red] 加入这个 case 后 smoke manifest 已有 10 项，但 e2e/observability/eval-foundation.test.mjs 仍断言 smoke.cases.length === 9，所以当前 Agent Evals / Evaluation contracts 必然失败。不要只把 9 改成 10；请改为断言明确的必选 case 集合、唯一性和预算上限等语义不变量，避免以后每加 case 都打破一个无意义的 magic count。
hifizz
left a comment
hifizz
left a comment
There was a problem hiding this comment.
Ponytail review（仅复杂度）
本轮严格按 Ponytail 边界：只找可删除复杂度，不重复已有的正确性、安全性或性能线程。10 条高置信度 finding 已作 inline 评论。
app/thread-chat/net/project-file-upload.ts:L20-68: delete: 第二套 Attachment→R2→ingest 管线。复用 lib/chat/attachment-adapter.ts 的 canonical uploader。（已有未解决线程，未重复留言。）
lib/thread-chat/contracts/dto.ts:L35: delete: suggestedQuestions 没有本期消费者。删除 DTO、mapper 和 fixtures 的透传。
app/thread-chat/core/types.ts:L45: delete: projectPanel 尺寸从未读写。真正支持 resize/persistence 时再加。
app/thread-chat/chat/chat-view.tsx:L95-117: shrink: data 属性不需要额外 div。挂到 MessageScroller.Item，并用精确 querySelector 定位。
Verdict: not lean enough; simplify before merge.
net: -400 lines possible.
| @@ -0,0 +1,135 @@ | |||
| name: Project Workspace CI | |||
|
|
|||
| on: | |||
There was a problem hiding this comment.
L3-7: delete: 这份 135 行 CI 只监听当前 feature branch，合并后立即失效，并重复 Agent Evals / OpenSpec。把一个 Project Workspace 测试脚本接入现有 pull_request workflow；删除本文件。
| } | ||
|
|
||
| /** | ||
| * Gate 3 旧 harness 的薄适配层。生产入口只使用 StoreBoundProjectPanel；这里仅把 |
There was a problem hiding this comment.
L27-99: delete: 为单个 Gate 3 旧 harness 伪造 ProjectDTO、1970 时间、状态映射和四个 no-op command。Harness 直接挂 StoreBoundProjectPanel；删除这个适配组件。
| sameValue(current.project, patch.after.project) | ||
| ? structuredClone(patch.before.project) | ||
| : current.project, | ||
| projectFilesById: rollbackRecord( |
There was a problem hiding this comment.
L375-387: yagni: Project Files 被接入 optimistic snapshot/rollback，但当前没有任何 optimistic command 修改它们。删除这段快照与回滚 plumbing，等首个 optimistic File mutation 出现再加。
| projectId: text("project_id") | ||
| .notNull() | ||
| .references(() => projects.id, { onDelete: "cascade" }), | ||
| threadId: text("thread_id") |
There was a problem hiding this comment.
L348-350: delete: artifacts.thread_id 重复 source Message 的 thread_id，并扩散成回填迁移、FK、索引、DTO、写入与测试。保留 sourceMessageId，查询时 join/derive。
| name: "project_files_pk", | ||
| columns: [table.projectId, table.attachmentId], | ||
| }), | ||
| uniqueIndex("project_files_attachment_uq").on(table.attachmentId), |
There was a problem hiding this comment.
L124-128: shrink: attachment_id 已唯一，复合主键又维护一次相同唯一性。让 attachmentId 直接做 PK，保留 projectId/addedAt 查询索引。
| import assert from "node:assert/strict" | ||
| import { readFile } from "node:fs/promises" | ||
|
|
||
| const panel = await readFile( |
There was a problem hiding this comment.
L4-79: delete: 这 80 行把 hook 名、函数名、import 和 JSX 字符串当合同，测试复制实现而不是行为。保留一个真实 Panel smoke/interaction test；删除源码正则断言。
| pages?: readonly string[] | null | ||
| } | ||
|
|
||
| export interface AttachmentCandidatePlan<T extends AttachmentContextCandidate> { |
There was a problem hiding this comment.
L8-12: yagni: AttachmentCandidatePlan 暴露 explicit/project/ordered 三套视图，唯一运行时调用者只取 .ordered。直接返回一个按优先级去重后的数组。
| } | ||
|
|
||
| return Promise.all( | ||
| const resolvedMessages = await Promise.all( |
There was a problem hiding this comment.
L114-126: shrink: 嵌套 Promise.all/Promise.resolve 包裹的全是同步 Map lookup。改成两层普通 map，删除 async 包装和末尾类型断言。
| userId, | ||
| projectFiles, | ||
| }) | ||
| const withProjectContext: ThreadChatUIMessage[] = [ |
There was a problem hiding this comment.
L121-145: shrink: 为插入一段可信文本构造完整伪 ThreadChatUIMessage、metadata 和 cast。先转换真实 messages，再 prepend 一个 ModelMessage。
| export const removeProjectFileCommandSchema = z | ||
| .object({ | ||
| commandId: commandIdSchema, | ||
| attachmentId: entityIdSchema, |
There was a problem hiding this comment.
L142-147: shrink: DELETE path 已包含 attachmentId，body 再传一次只制造第二份 schema 与 mismatch 分支。Body 只解析 commandId，再用 path id 构造内部命令。
- keep bubble open on outside click / scroll / resize while draft exists - ignore new text selections with a draft (copy-paste into the composer) - Esc with a draft opens a confirm dialog and shakes the bubble - entering/shake animations moved off steady-state to avoid tc-pop replay flash - draft-guard styles co-located with the component
- focus the question textarea only after the measured bubble becomes visible - replay the entrance transition for every new selection - compensate textarea borders to avoid phantom overflow - reuse a global slim scrollbar utility with Firefox support
7e311f6
into
codex/feat-agent-observability-evaluation
1 participant
背景
基于
codex/feat-agent-observability-evaluation的 HEAD48483101ad11bc84b611b615f423577633fedacb，先研究 ThreadChat 如何把现有 Project 从“分支对话容器”扩展为长期 AI 工作空间，再将已经冻结的 MVP 范围转换成可实施的 OpenSpec。当前产品范围
本次 MVP 的目标是：
OpenSpec Change
Change：
add-project-workspace-mvp路径：
openspec/changes/add-project-workspace-mvp/包含：
proposal.md：为什么现在做最小 Project Workspace、会改变什么、影响哪些模块；design.md：Contract、Project File membership、模型上下文、Project Artifact library、统一 Project Panel、权限与迁移决策；specs/project-workspace/spec.md：Project Workspace 的规范性 Requirements 与验收 Scenarios；tasks.md：从数据库、Commands/API、上下文装配、UI 到测试与上线校验的实施清单；.openspec.yaml：使用仓库默认spec-drivenschema。关键设计决策
contractVersion防止旧页面静默覆盖新设置。明确不在本次 MVP
@Artifact、@Message、@Thread；depends_on、依赖图、Convergence / 汇总对象；以上能力必须在 MVP 使用数据证明必要后，分别建立新的 Research/OpenSpec change。
Research 文档
docs/project/01-project-workspace-research.md：总体问题空间；docs/project/02-dependent-thread-handoff-research.md：较复杂依赖方案的探索记录；docs/project/03-reference-and-outcome-preliminary-research.md：Reference + Outcome 收敛过程；docs/project/04-project-mvp-scope-and-roadmap.md：当前冻结范围和开发节奏，产品决策以此为准。验证
OpenSpec已执行validate --all --strict并通过；