lhl
80daadcd31
- 将 /api/sessions/{sid}/ws 端点移入 create_app（此前置于模块级导致整模块 import NameError，回归被验证拦截）
- register_loop + subscribe 调整至 accept 之前，缩小连接已开但未订阅期间的进度丢失窗口
- 新增 tests/test_verify_ws_real_flow.py：驱动真实 HTTP 聊天流程断言 WS 收到 agent 实际发射的 parse/impact 进度
- 同步 WebSocket 计划文档 Task 3 代码片段（标注端点必须位于 create_app 内）
- 全量 pytest 实测 583 passed / 99.03% 达标
5.5 KiB
5.5 KiB
WebSocket 实时进度流 — 全分支最终代码评审
- 评审对象：分支
feat/websocket-progress（相对基线014a51a） - 评审范围：5 个任务落地、工程质量、测试、计划决策遵守、风险
- 全量验证结果：
582 passed / 582、coverage 99.03%（达到fail_under=99门禁） - 计划决策：D1=单向（server→client），D2=单进程（无跨进程总线）
总体结论
APPROVED_WITH_MINORS
五个任务全部落地，规格一致性高；跨线程 emit、disconnect 清理、前端异常降级、持久化兜底均按设计实现；全量测试 582 通过、覆盖率 99.03% 达标。存在若干非阻塞的工程质量与测试覆盖小问题（见下），建议合并后于后续迭代修复。
Critical 发现
无。
Important 发现
I1. WS 端点在「空闲阻塞于 q.get()」时无法感知客户端断开，且 disconnect 分支未被测试真正覆盖
- 位置：
src/genesis/server/app.py:316-328（session_progress_ws） - 问题：
while True: await q.get()在等待事件时并不触碰 socket，因此客户端在此期间断开连接不会被检测；连接会一直挂起直到下一条emit触发send_json抛WebSocketDisconnect才清理。若会话长时间无进度事件，连接会泄漏（占用队列、FD）。- 全量覆盖率显示
app.py:326（except WebSocketDisconnect: pass）未覆盖。对应测试tests/test_progress_e2e.py::test_ws_progress_second_emit_after_disconnect的时序是：客户端读取首条后即关闭，此时服务端正阻塞在第二次q.get()，尚未走到send_json，故WebSocketDisconnect实际未被触发，pass分支为死分支/未被测到。
- 建议修复（二选一，均不阻塞本次合并）：
- 为
q.get()增加超时并周期性await ws.receive()或发送心跳 ping，使断开可及时感知； - 或将循环改为
while True: event = await q.get(); try: await ws.send_json(event) except WebSocketDisconnect: break，并在测试中显式验证断开后队列被unsubscribe（断言hub._subs.get(sid) is None）。
- 为
I2. hub.register_loop 在每次 WS 连接时重复注册
- 位置：
src/genesis/server/app.py:319 - 问题：单进程下功能正确，但语义上「事件循环注册」应属于应用生命周期，而非每个连接。若未来多 worker/多 loop，后连的连接会覆盖
_loop，造成早连连接的run_coroutine_threadsafe投递到错误 loop。 - 建议：在
create_app启动期（lifespan或首次请求钩子）注册一次 loop；WS 端点仅subscribe/unsubscribe。属加固项，单进程假设下非缺陷。
Minor 发现
- M1（风格/潜在缺陷）
src/genesis/chat/agent.py:189-192缩进过深：_run_parse的except块内reply = ...、_emit_error、_store_error、return比同块其他语句多缩进 4 空格（16 空格 vs 12 空格）。语法合法、功能正确（异常分支仍正常执行），但触发E117 over-indented类 lint 告警，且易误读。建议统一为 12 空格缩进。 - M2（项目约定）
_AI_USAGE_LOG.md未更新：AGENTS.md 规定每次代码改动须在根目录_AI_USAGE_LOG.md追加记录，但本次 6 个提交均未修改该文件。建议补登本次迭代的范式步骤与涉及文件。 - M3（文档锚点）
docs/design.md实际写入为§12.8，而计划文档 Task5 注明「§12 追加」。非错误，但命名与计划略有出入，可统一。 - M4（前端连接策略）
chat.htmlsend()每次发消息都会close旧progressWs再重新connectProgressWs：逻辑正确（避免泄漏），但每次消息重建 WS 略显浪费；可考虑仅在会话切换（loadSession / newSession）时开关，消息发送阶段复用同一连接。 - M5（竞态，已知可接受） 若用户在前端 WS 尚未完成订阅前即触发会发射进度的请求，首条事件可能丢失（队列尚未创建）。因
role='progress'/'error'持久化兜底 + 重载渲染仍可见，正确性不受影响，属已知限制，无需阻塞。
计划决策遵守核对
| 决策 | 要求 | 实现核对 | 结论 |
|---|---|---|---|
| D1 单向 | WS 仅 server→client；聊天仍走 HTTP | chat_ws.js 仅 onmessage 消费，从不 send；/api/chat/{sid}/messages 仍是 HTTP POST |
遵守 ✓ |
| D2 单进程 | 无跨进程总线；限制写入文档 | 无 Redis/消息中间件依赖；docs/design.md §12.8 已标注「多 worker 跨进程不互通」 |
遵守 ✓ |
规格一致性（5 任务）
- Task1
ProgressHub单例：hub.py新建，3 个单测通过 ✓ - Task2 agent 发射：
_emit_progress/_emit_error在 4 个流程方法的进度/错误分支接线，持久化_persist_progress/_store_error保留 ✓ - Task3 WS 端点：
/api/sessions/{sid}/ws+/chat_ws.js静态路由 +websockets>=12依赖 ✓ - Task4 前端
chat_ws.js+chat.html集成（含 try/catch 降级、会话切换关闭旧连接）✓ - Task5 e2e 冒烟 +
design.md记录 ✓
无范围蔓延、无占位 TODO、无调试残留、无裸 except:（均为 except Exception，且带 noqa: BLE001）。
是否可合并
可以合并。 功能完整、全量测试通过、覆盖率达标、决策遵守、无 Critical/阻塞性问题。建议在合并后的后续迭代中修复 I1（空闲断开检测与 disconnect 分支测试覆盖）与 I2（loop 注册时机），并补登 M2 的项目日志。