fix(event-sourced-state): preserve updated_at in todo-add projection - #3359
Conversation
1772059 to
e53c5d3
Compare
huangruiteng
left a comment
huangruiteng
left a comment
There was a problem hiding this comment.
结论：我在精确 head e53c5d3887b39b49274d0ce0db34b47590ff9f9b 上完成了整份 PR 的代码、测试、文档与上下游契约评审。运行时修复方向正确、范围小、回归测试有效；但本 PR 新增的公开 bug 文档仍把同一个已修复且测试已转绿的问题写成“未修复 / 已加红测试”，因此当前不能合入。
详细中文评审
动机
问题链路成立：_append_event_projected_successor 在 TODO_ADDED payload 中写入 updated_at，_todo_from_added_event 负责把事件还原为 todo 投影，而 management_projection 和 Markdown 渲染又消费该字段。reducer 漏拷贝会让新增 todo 丢失最后活动时间，破坏事件生产者—投影—消费者之间的保真不变量。
改动思路
生产代码在新增事件 reducer 的固定字段中补入 updated_at，并沿用完成事件已有的 compact_text 归一化；测试从真实 TODO_ADDED 事件走 build_state_projection，验证投影保留时间戳。这个方案不引入新抽象，也没有扩大权限、状态机或默认行为边界。
具体改动
loopx/event_sourced_state.py：1 行运行时修复，属于必要改动。tests/control_plane/test_todo_mutation_authority.py：22 行语义回归测试，覆盖真实正向路径。docs/development/bugs/event-sourced-updated-at-drop.md：150 行新文档；其当前状态叙述与本 PR 的精确 head、测试结果相矛盾，且大部分内容重复 PR 描述和测试证据。
关键代码讲解
正向路径：TODO_ADDED.payload.updated_at → _todo_from_added_event → projected todo → management/render consumers。新增的一行恢复了这一字段链路。
负向/兼容路径：事件没有 updated_at 时，compact_text(None) 得到空串；现有消费者使用 todo.get("updated_at") or todo.get("latest_event_at")，因此原有 fallback 不变。这里没有新增 substring 分类、领域特定措辞、机器义务或静默默认切换；typed-state、domain-neutrality、guidance-vs-obligation 三个镜头均无新增风险。
对主干的风险
运行时风险低：改动只恢复生产者已写入、消费者已读取的既有字段，且缺省语义不变。
阻塞问题如下：
[P1] 删除或改写仍标注“未修复 / 红测试”的新 bug 文档
触发条件：该 PR 合入后，维护者把 docs/development/bugs/event-sourced-updated-at-drop.md 当作主干当前事实阅读。
具体路径：文档状态表写着“未修复（已加红测试）”，后文也反复使用“当前代码”“修复前此测试红”等现在时；但同一精确 head 已加入修复，回归测试与 CI 都是绿色。
风险：公开主干会把已修复行为记录成未修复，把绿色回归写成红测，误导排障和后续维护；同时为 1 行修复新增 150 行临时问题叙事，违反 canonical 文档应压缩而非累积过期版本的原则。
最小修复：优先删除这份临时 bug note，让 PR 描述、提交信息和回归测试承载证据；若仓库确实需要保留 bug 记录，则压缩为短小的 resolved note，并把状态、时态和测试结果统一改成“已修复 / 绿色”，不再保留会过期的行号与“当前未修复”叙述。修订后用 rg 确认不再出现冲突状态即可。
我的整体评价
代码与测试可以接受，scope-fit 也成立：有真实生产者、真实 reducer 和真实消费者，不是为测试造出的接口。当前 REQUEST_CHANGES 只针对新增公开文档的错误状态与不必要体积；清理或准确改写后，这个 PR 应可快速通过。
验证证据：
- 本地：
python -m pytest -q tests/control_plane/test_todo_mutation_authority.py -k updated_at→ 1 passed - 本地：
ruff check与py_compile通过 - 远端：Sign-off、dependency-review、pytest、build、windows-powershell 均通过
- diff check：通过
English verdict: REQUEST_CHANGES — the runtime fix and regression test are sound, but the newly added public bug note is stale on this exact head and must be removed or made accurately resolved before merge.
_todo_from_added_event copies most persisted fields from the ADD event payload into the projected todo, but omitted updated_at. The producer (event_writeback.py) always writes updated_at into the ADD payload, and readers (render_todo_markdown, management_projection) read it back for last-activity display. Dropping it left the projection without the field until a later COMPLETED event happened to copy it. Add updated_at to the constructor dict using the same compact_text normalization the completion reducer already applies, and lock the behavior with a red->green regression test. Signed-off-by: rootkiller6788 <c8688rickowens@outlook.com>
e53c5d3 to
aa18f5e
Compare
|
Thanks for the review. You're right that the bug note was stale on this head — it I've replaced the 150-line note with a short resolved entry Please re-review when convenient. |
huangruiteng
left a comment
huangruiteng
left a comment
There was a problem hiding this comment.
结论：APPROVE。此前唯一阻塞项已经在精确 head aa18f5eb9746092c81b01dd89c0e6eb464135851 上解决；我重新阅读了完整 diff、生产者与消费者链路、回归测试和远端 checks，没有发现新的阻塞问题。
动机
TODO_ADDED 的生产路径已经把 updated_at 写入事件 payload，但 _todo_from_added_event 在恢复 todo 投影时漏掉该字段。下游管理投影以 updated_at 为首选最后活动时间，并仅在字段为空时回退到 latest_event_at，所以新增 todo 会在首次后续事件发生前展示不完整的活动时间。该 PR 恢复的是既有生产者—reducer—消费者契约，不是引入新的状态语义。
改动思路
修复放在真正丢失信息的 reducer 边界：_todo_from_added_event 使用与同模块其他时间字段一致的 compact_text 归一化，把 payload 中的 updated_at 复制进投影。回归测试通过真实 TODO_ADDED 事件和 build_state_projection 验证端到端投影结果。缺字段时仍得到空串，下游既有 updated_at or latest_event_at 回退逻辑保持不变。
具体改动
loopx/event_sourced_state.py：增加一行必要的生产代码，恢复updated_at的投影保真。tests/control_plane/test_todo_mutation_authority.py：增加语义回归测试，覆盖事件输入、reducer 和最终 agent todo 投影。docs/development/bugs/event-sourced-updated-at-drop.md：已从上一 head 的 150 行、状态矛盾的临时叙事压缩为 22 行 resolved note；状态、时态和绿色回归结果现与代码一致，也移除了易过期的行号和“未修复/红测试”表述。
关键链路是 event_writeback 写入 payload.updated_at → _todo_from_added_event 保留该字段 → build_state_projection 生成 todo → management projection 以其计算 last activity。没有新增 substring 分类规则、散落布尔状态、领域特定控制面措辞、权限边界或机器义务；typed-state、domain-neutrality、behavior-disclosure 与 guidance-vs-obligation 镜头均无新增问题。
对主干的风险
没有阻塞发现。最强回归场景是缺少 updated_at 的历史或手工事件进入 reducer；该路径仍归一化为空串，并由现有 latest_event_at fallback 接管，因此兼容语义不变。变更规模为 1 行运行时、22 行测试、22 行 resolved 文档，scope 与问题匹配。
验证证据：
python -m pytest -q tests/control_plane/test_todo_mutation_authority.py：46 passed。- 精确回归筛选：1 passed、45 deselected。
python3 -m py_compile loopx/event_sourced_state.py：通过。git diff --check origin/main...aa18f5eb9746092c81b01dd89c0e6eb464135851：通过。- resolved 文档冲突措辞扫描：无命中。
- 远端 Sign-off、dependency-review、pytest、Release Artifacts build、windows-powershell 均通过。Frontstage Pages 的 build/deploy 显示 cancelled，并非测试或构建失败；本 diff 只新增无链接的普通 Markdown 页面，因此将其记录为非阻塞的残余 CI 可见性风险。
我的整体评价
这个修复很小，但命中了正确的事件投影边界，并用真实 reducer 路径锁定了不变量。作者对上一轮反馈的处理也合适：保留有价值的短 resolved 记录，同时删除会与主干事实漂移的长篇临时证据。当前 head 可以合入。
English review
Motivation
The TODO_ADDED producer already persists updated_at, while _todo_from_added_event dropped it before management consumers computed last activity. This PR restores an existing producer–reducer–consumer invariant rather than adding new lifecycle semantics.
Approach
The fix copies the field at the information-loss boundary with the module's existing compact_text normalization. The regression test exercises a real event through build_state_projection; missing timestamps still normalize to an empty string and preserve the existing latest_event_at fallback.
Concrete changes
The exact-head diff contains one necessary runtime line, one focused 22-line regression test, and a 22-line resolved note. The previous stale “unfixed/red test” narrative has been removed. No new typed-state, domain-neutrality, default-behavior-disclosure, authority, or guidance-versus-obligation concern is introduced.
Main-branch risk
No blocking finding. The historical-event negative path retains its fallback behavior. Focused and full-file tests pass, compile and diff checks pass, and the relevant remote checks are green. The cancelled Frontstage Pages jobs are noted as a non-blocking CI-visibility risk rather than a code failure.
Overall assessment
The implementation is localized, behaviorally justified, independently validated, and the documentation now reflects the shipped truth. Approved at exact head aa18f5eb9746092c81b01dd89c0e6eb464135851.
2 participants
_todo_from_added_event copies most persisted fields from the ADD event payload into the projected todo, but omitted updated_at. The producer (event_writeback.py) always writes updated_at into the ADD payload, and readers (render_todo_markdown, management_projection) read it back for last-activity display. Dropping it left the projection without the field until a later COMPLETED event happened to copy it.
Add updated_at to the constructor dict using the same compact_text normalization the completion reducer already applies, and lock the behavior with a red->green regression test.
Summary
Issue Or Task
Validation
python3 -m py_compile loopx/*.pyloopx check --scan-root .Type of Change
LoopX Area
Technical Direction
Core control-plane hardening
Long-horizon benchmark evidence
Operator surface and IM integration
Shared Goal Authority and cross-host coordination
Architecture and research incubator
Target base branch:
Direction tracker or promotion unit:
Boundary Checklist
.loopx/,.codex/goals/, liveACTIVE_GOAL_STATE.md, credentials, private benchmark traces, verifier output, raw agent sessions, internal document links, or local machine paths.Signed-off-bytrailer (git commit -s).